Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Export expvar metrics of badger to the metricsFactory #1704

Merged
merged 10 commits into from
Aug 7, 2019
4 changes: 3 additions & 1 deletion plugin/storage/badger/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"time"

"github.com/dgraph-io/badger"
"github.com/dgraph-io/badger/options"

// init() badger's metrics to make them available in Initialize()
_ "github.com/dgraph-io/badger/y"
Expand Down Expand Up @@ -92,6 +93,7 @@ func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger)
f.logger = logger

opts := badger.DefaultOptions
opts.TableLoadingMode = options.MemoryMap

if f.Options.primary.Ephemeral {
opts.SyncWrites = false
Expand Down Expand Up @@ -205,7 +207,7 @@ func (f *Factory) maintenance() {
}

func (f *Factory) metricsCopier() {
metricsTicker := time.NewTicker(10 * time.Second)
metricsTicker := time.NewTicker(f.Options.primary.MetricsUpdateInterval)
defer metricsTicker.Stop()
for {
select {
Expand Down
30 changes: 30 additions & 0 deletions plugin/storage/badger/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,3 +144,33 @@ func TestMaintenanceCodecov(t *testing.T) {
_ = f.store.Close()
waiter() // This should trigger the logging of error
}

func TestBadgerMetrics(t *testing.T) {
f := NewFactory()
v, command := config.Viperize(f.AddFlags)
command.ParseFlags([]string{
"--badger.metrics-update-interval=10ms",
})
f.InitFromViper(v)
mFactory := metricstest.NewFactory(0)
f.Initialize(mFactory, zap.NewNop())
assert.NotNil(t, f.metrics.badgerMetrics)
_, found := f.metrics.badgerMetrics["badger_memtable_gets_total"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be good to see a test for the expvar.Map metrics as well.

assert.True(t, found)

waiter := func(previousValue int64) int64 {
sleeps := 0
_, gs := mFactory.Snapshot()
for gs["adger_memtable_gets_total"] == previousValue && sleeps < 8 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why "adger"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo. It always sleeps the maximum with that typo, so the test functions correctly - but might take too long. I'll fix that.

// Wait for the scheduler
time.Sleep(time.Duration(50) * time.Millisecond)
sleeps++
_, gs = mFactory.Snapshot()
}
assert.True(t, gs["badger_memtable_gets_total"] > previousValue)
return gs["badger_memtable_gets_total"]
}

vlogSize := waiter(0)
assert.True(t, vlogSize > 0)
}
44 changes: 27 additions & 17 deletions plugin/storage/badger/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,20 @@ type Options struct {

// NamespaceConfig is badger's internal configuration data
type NamespaceConfig struct {
namespace string
SpanStoreTTL time.Duration
ValueDirectory string
KeyDirectory string
Ephemeral bool // Setting this to true will ignore ValueDirectory and KeyDirectory
SyncWrites bool
MaintenanceInterval time.Duration
namespace string
SpanStoreTTL time.Duration
ValueDirectory string
KeyDirectory string
Ephemeral bool // Setting this to true will ignore ValueDirectory and KeyDirectory
SyncWrites bool
MaintenanceInterval time.Duration
MetricsUpdateInterval time.Duration
}

const (
defaultMaintenanceInterval time.Duration = 5 * time.Second
defaultTTL time.Duration = time.Hour * 72
defaultMaintenanceInterval time.Duration = 5 * time.Minute
defaultMetricsUpdateInterval time.Duration = 10 * time.Second
defaultTTL time.Duration = time.Hour * 72
)

const (
Expand All @@ -52,6 +54,7 @@ const (
suffixSpanstoreTTL = ".span-store-ttl"
suffixSyncWrite = ".consistency"
suffixMaintenanceInterval = ".maintenance-interval"
suffixMetricsInterval = ".metrics-update-interval" // Intended only for testing purposes
defaultDataDir = string(os.PathSeparator) + "data"
defaultValueDir = defaultDataDir + string(os.PathSeparator) + "values"
defaultKeysDir = defaultDataDir + string(os.PathSeparator) + "keys"
Expand All @@ -64,13 +67,14 @@ func NewOptions(primaryNamespace string, otherNamespaces ...string) *Options {

options := &Options{
primary: &NamespaceConfig{
namespace: primaryNamespace,
SpanStoreTTL: defaultTTL,
SyncWrites: false, // Performance over durability
Ephemeral: true, // Default is ephemeral storage
ValueDirectory: defaultBadgerDataDir + defaultValueDir,
KeyDirectory: defaultBadgerDataDir + defaultKeysDir,
MaintenanceInterval: defaultMaintenanceInterval,
namespace: primaryNamespace,
SpanStoreTTL: defaultTTL,
SyncWrites: false, // Performance over durability
Ephemeral: true, // Default is ephemeral storage
ValueDirectory: defaultBadgerDataDir + defaultValueDir,
KeyDirectory: defaultBadgerDataDir + defaultKeysDir,
MaintenanceInterval: defaultMaintenanceInterval,
MetricsUpdateInterval: defaultMetricsUpdateInterval,
},
}

Expand Down Expand Up @@ -112,13 +116,18 @@ func addFlags(flagSet *flag.FlagSet, nsConfig *NamespaceConfig) {
flagSet.Bool(
nsConfig.namespace+suffixSyncWrite,
nsConfig.SyncWrites,
"If all writes should be synced immediately. This will greatly reduce write performance.",
"If all writes should be synced immediately. This can impact write performance.",
)
flagSet.Duration(
nsConfig.namespace+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,
nsConfig.MetricsUpdateInterval,
"How often the badger metrics are collected by Jaeger. Format is time.Duration (https://golang.org/pkg/time/#Duration)",
)
}

// InitFromViper initializes Options with properties from viper
Expand All @@ -133,6 +142,7 @@ func initFromViper(cfg *NamespaceConfig, v *viper.Viper) {
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)
}

// GetPrimary returns the primary namespace configuration
Expand Down