Skip to content

Commit

Permalink
Remove unused "namespace" field from Badger config (#5929)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
- NamespaceConfig had a private `namespace` field which is not necessary
- Part of #5926

## Description of the changes
- Remove the field, clean up the tests

## How was this change tested?
- CI

---------

Signed-off-by: Yuri Shkuro <[email protected]>
  • Loading branch information
yurishkuro authored Sep 4, 2024
1 parent 3f5e87c commit 7a8fe39
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 29 deletions.
2 changes: 1 addition & 1 deletion plugin/storage/badger/dependencystore/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func runFactoryTest(tb testing.TB, test func(tb testing.TB, sw spanstore.Writer,
require.NoError(tb, f.Close())
}()

opts := badger.NewOptions("badger")
opts := badger.NewOptions()
v, command := config.Viperize(opts.AddFlags)
command.ParseFlags([]string{
"--badger.ephemeral=true",
Expand Down
2 changes: 1 addition & 1 deletion plugin/storage/badger/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ type Factory struct {
// NewFactory creates a new Factory.
func NewFactory() *Factory {
return &Factory{
Options: NewOptions("badger"),
Options: NewOptions(),
maintenanceDone: make(chan bool),
}
}
Expand Down
38 changes: 19 additions & 19 deletions plugin/storage/badger/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ type Options struct {

// NamespaceConfig is badger's internal configuration data
type NamespaceConfig struct {
namespace string
SpanStoreTTL time.Duration `mapstructure:"span_store_ttl"`
ValueDirectory string `mapstructure:"directory_value"`
KeyDirectory string `mapstructure:"directory_key"`
Expand All @@ -40,6 +39,7 @@ const (
)

const (
prefix = "badger"
suffixKeyDirectory = ".directory-key"
suffixValueDirectory = ".directory-value"
suffixEphemeral = ".ephemeral"
Expand Down Expand Up @@ -67,9 +67,9 @@ func DefaultNamespaceConfig() NamespaceConfig {
}

// NewOptions creates a new Options struct.
func NewOptions(primaryNamespace string, _ ...string /* otherNamespaces */) *Options {
// @nocommit func NewOptions(primaryNamespace string, _ ...string /* otherNamespaces */) *Options {
func NewOptions() *Options {
defaultConfig := DefaultNamespaceConfig()
defaultConfig.namespace = primaryNamespace

options := &Options{
Primary: defaultConfig,
Expand All @@ -91,42 +91,42 @@ func (opt *Options) AddFlags(flagSet *flag.FlagSet) {

func addFlags(flagSet *flag.FlagSet, nsConfig NamespaceConfig) {
flagSet.Bool(
nsConfig.namespace+suffixEphemeral,
prefix+suffixEphemeral,
nsConfig.Ephemeral,
"Mark this storage ephemeral, data is stored in tmpfs.",
)
flagSet.Duration(
nsConfig.namespace+suffixSpanstoreTTL,
prefix+suffixSpanstoreTTL,
nsConfig.SpanStoreTTL,
"How long to store the data. Format is time.Duration (https://golang.org/pkg/time/#Duration)",
)
flagSet.String(
nsConfig.namespace+suffixKeyDirectory,
prefix+suffixKeyDirectory,
nsConfig.KeyDirectory,
"Path to store the keys (indexes), this directory should reside in SSD disk. Set ephemeral to false if you want to define this setting.",
)
flagSet.String(
nsConfig.namespace+suffixValueDirectory,
prefix+suffixValueDirectory,
nsConfig.ValueDirectory,
"Path to store the values (spans). Set ephemeral to false if you want to define this setting.",
)
flagSet.Bool(
nsConfig.namespace+suffixSyncWrite,
prefix+suffixSyncWrite,
nsConfig.SyncWrites,
"If all writes should be synced immediately to physical disk. This will impact write performance.",
)
flagSet.Duration(
nsConfig.namespace+suffixMaintenanceInterval,
prefix+suffixMaintenanceInterval,
nsConfig.MaintenanceInterval,
"How often the maintenance thread for values is ran. Format is time.Duration (https://golang.org/pkg/time/#Duration)",
)
flagSet.Duration(
nsConfig.namespace+suffixMetricsInterval,
prefix+suffixMetricsInterval,
nsConfig.MetricsUpdateInterval,
"How often the badger metrics are collected by Jaeger. Format is time.Duration (https://golang.org/pkg/time/#Duration)",
)
flagSet.Bool(
nsConfig.namespace+suffixReadOnly,
prefix+suffixReadOnly,
nsConfig.ReadOnly,
"Allows to open badger database in read only mode. Multiple instances can open same database in read-only mode. Values still in the write-ahead-log must be replayed before opening.",
)
Expand All @@ -138,14 +138,14 @@ func (opt *Options) InitFromViper(v *viper.Viper, logger *zap.Logger) {
}

func initFromViper(cfg *NamespaceConfig, v *viper.Viper, _ *zap.Logger) {
cfg.Ephemeral = v.GetBool(cfg.namespace + suffixEphemeral)
cfg.KeyDirectory = v.GetString(cfg.namespace + suffixKeyDirectory)
cfg.ValueDirectory = v.GetString(cfg.namespace + suffixValueDirectory)
cfg.SyncWrites = v.GetBool(cfg.namespace + suffixSyncWrite)
cfg.SpanStoreTTL = v.GetDuration(cfg.namespace + suffixSpanstoreTTL)
cfg.MaintenanceInterval = v.GetDuration(cfg.namespace + suffixMaintenanceInterval)
cfg.MetricsUpdateInterval = v.GetDuration(cfg.namespace + suffixMetricsInterval)
cfg.ReadOnly = v.GetBool(cfg.namespace + suffixReadOnly)
cfg.Ephemeral = v.GetBool(prefix + suffixEphemeral)
cfg.KeyDirectory = v.GetString(prefix + suffixKeyDirectory)
cfg.ValueDirectory = v.GetString(prefix + suffixValueDirectory)
cfg.SyncWrites = v.GetBool(prefix + suffixSyncWrite)
cfg.SpanStoreTTL = v.GetDuration(prefix + suffixSpanstoreTTL)
cfg.MaintenanceInterval = v.GetDuration(prefix + suffixMaintenanceInterval)
cfg.MetricsUpdateInterval = v.GetDuration(prefix + suffixMetricsInterval)
cfg.ReadOnly = v.GetBool(prefix + suffixReadOnly)
}

// GetPrimary returns the primary namespace configuration
Expand Down
6 changes: 3 additions & 3 deletions plugin/storage/badger/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
)

func TestDefaultOptionsParsing(t *testing.T) {
opts := NewOptions("badger")
opts := NewOptions()
v, command := config.Viperize(opts.AddFlags)
command.ParseFlags([]string{})
opts.InitFromViper(v, zap.NewNop())
Expand All @@ -25,7 +25,7 @@ func TestDefaultOptionsParsing(t *testing.T) {
}

func TestParseOptions(t *testing.T) {
opts := NewOptions("badger")
opts := NewOptions()
v, command := config.Viperize(opts.AddFlags)
command.ParseFlags([]string{
"--badger.ephemeral=false",
Expand All @@ -45,7 +45,7 @@ func TestParseOptions(t *testing.T) {
}

func TestReadOnlyOptions(t *testing.T) {
opts := NewOptions("badger")
opts := NewOptions()
v, command := config.Viperize(opts.AddFlags)
command.ParseFlags([]string{
"--badger.read-only=true",
Expand Down
6 changes: 3 additions & 3 deletions plugin/storage/badger/spanstore/read_write_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ func TestPersist(t *testing.T) {
require.NoError(t, f.Close())
}()

opts := badger.NewOptions("badger")
opts := badger.NewOptions()
v, command := config.Viperize(opts.AddFlags)

keyParam := fmt.Sprintf("--badger.directory-key=%s", dir)
Expand Down Expand Up @@ -438,7 +438,7 @@ func runFactoryTest(tb testing.TB, test func(tb testing.TB, sw spanstore.Writer,
require.NoError(tb, f.Close())
}()

opts := badger.NewOptions("badger")
opts := badger.NewOptions()
v, command := config.Viperize(opts.AddFlags)
command.ParseFlags([]string{
"--badger.ephemeral=true",
Expand Down Expand Up @@ -598,7 +598,7 @@ func BenchmarkServiceIndexLimitFetch(b *testing.B) {
func runLargeFactoryTest(tb testing.TB, test func(tb testing.TB, sw spanstore.Writer, sr spanstore.Reader)) {
assertion := require.New(tb)
f := badger.NewFactory()
opts := badger.NewOptions("badger")
opts := badger.NewOptions()
v, command := config.Viperize(opts.AddFlags)

dir := filepath.Join(tb.TempDir(), "badger-testRun")
Expand Down
2 changes: 1 addition & 1 deletion plugin/storage/badger/stats_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (

func TestDiskStatisticsUpdate(t *testing.T) {
f := NewFactory()
opts := NewOptions("badger")
opts := NewOptions()
v, command := config.Viperize(opts.AddFlags)
command.ParseFlags([]string{
"--badger.ephemeral=true",
Expand Down
2 changes: 1 addition & 1 deletion plugin/storage/badger/stats_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (

func TestDiskStatisticsUpdate(t *testing.T) {
f := NewFactory()
opts := NewOptions("badger")
opts := NewOptions()
v, command := config.Viperize(opts.AddFlags)
command.ParseFlags([]string{
"--badger.ephemeral=true",
Expand Down

0 comments on commit 7a8fe39

Please sign in to comment.