-
Notifications
You must be signed in to change notification settings - Fork 453
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
[dbnode] Set default values for BootstrapPeersConfiguration #3420
[dbnode] Set default values for BootstrapPeersConfiguration #3420
Conversation
if pCfg.StreamShardConcurrency != nil { | ||
pOpts = pOpts.SetDefaultShardConcurrency(*pCfg.StreamShardConcurrency) | ||
} | ||
if pCfg.StreamPersistShardConcurrency != nil { | ||
pOpts = pOpts.SetShardPersistenceConcurrency(*pCfg.StreamPersistShardConcurrency) | ||
} | ||
if pCfg.StreamPersistShardFlushConcurrency != nil { | ||
pOpts = pOpts.SetShardPersistenceFlushConcurrency(*pCfg.StreamPersistShardFlushConcurrency) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default values are already being set in peers.NewOptions()
:
m3/src/dbnode/storage/bootstrap/bootstrapper/peers/options.go
Lines 88 to 101 in 37e8c9b
func NewOptions() Options { | |
return &options{ | |
resultOpts: result.NewOptions(), | |
defaultShardConcurrency: DefaultShardConcurrency, | |
shardPersistenceConcurrency: DefaultShardPersistenceConcurrency, | |
shardPersistenceFlushConcurrency: DefaultShardPersistenceFlushConcurrency, | |
indexSegmentConcurrency: fsbootstrapper.DefaultIndexSegmentConcurrency, | |
persistenceMaxQueueSize: defaultPersistenceMaxQueueSize, | |
// Use a zero pool, this should be overridden at config time. | |
contextPool: context.NewPool(context.NewOptions(). | |
SetContextPoolOptions(pool.NewObjectPoolOptions().SetSize(0)). | |
SetFinalizerPoolOptions(pool.NewObjectPoolOptions().SetSize(0))), | |
} | |
} |
Codecov Report
@@ Coverage Diff @@
## master #3420 +/- ##
=======================================
Coverage 72.4% 72.4%
=======================================
Files 1100 1100
Lines 102602 102602
=======================================
Hits 74347 74347
Misses 23151 23151
Partials 5104 5104
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* master: [dbnode] Set default values for BootstrapPeersConfiguration (#3420) [integration-tests] Use explicit version for quay.io/m3db/prometheus_remote_client_golang (#3422) [dtest] Fix dtest docker compose config: env => environment (#3421) Fix broken links to edit pages (#3419) [dbnode] Fix races in source_data_test.go (#3418) [coordinator] add more information to processed count metric (#3415) [dbnode] Avoid use of grow on demand worker pool for fetchTagged and aggregate (#3416) [docs] Fix m3aggregagtor typo (#3417) [x/log] Bump zap version and add logging encoder configuration (#3377) Do not use buffer channels if growOnDemand is true (#3414) [dbnode] Fix TestSeriesWriteReadParallel datapoints too far in past with -race flag (#3413) [docs] Update m3db operator docs with v0.13.0 features (#3397) [aggregator] Fix followerFlushManager metrics (#3411) [query] Restore optimization to skip initial fetch for timeShift and unary fns (#3408)
What this PR does / why we need it:
Fixes #3286: when only a subset of
BootstrapPeersConfiguration
parameters were supplied in config, others defaulted to 0 instead of default values mentioned in parameter description.m3/src/cmd/services/m3dbnode/config/bootstrap.go
Lines 250 to 263 in 37e8c9b
Special notes for your reviewer:
Does this PR introduce a user-facing and/or backwards incompatible change?:
Does this PR require updating code package or user-facing documentation?: