-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
add bolt compaction sleep interval #13018
add bolt compaction sleep interval #13018
Conversation
b6f3aaa
to
ad42eef
Compare
1f399d4
to
9e765fb
Compare
server/embed/config.go
Outdated
ExperimentalCompactionBatchLimit int `json:"experimental-compaction-batch-limit"` | ||
ExperimentalEnableLeaseCheckpoint bool `json:"experimental-enable-lease-checkpoint"` | ||
ExperimentalCompactionBatchLimit int `json:"experimental-compaction-batch-limit"` | ||
// ExperimentalCompactionSleepInterval is the sleep interval between every bolt compaction loop. |
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.
bolt -> etcd. Compaction happens in etcd data-model. defrag
would be bbolt
operation.
When user should enlarge that time ?
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.
done. have change to 'etcd' from 'bolt'.
Q: When user should enlarge that time ?
A: In more then 5,000 nodes k8s cluster.
9e765fb
to
184b0e5
Compare
Codecov Report
@@ Coverage Diff @@
## main #13018 +/- ##
==========================================
- Coverage 68.20% 64.41% -3.80%
==========================================
Files 435 422 -13
Lines 33186 33597 +411
==========================================
- Hits 22636 21641 -995
- Misses 8615 9764 +1149
- Partials 1935 2192 +257
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
approved workflow run |
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 Thanks @AlexStocks
@@ -52,9 +52,11 @@ const ( | |||
|
|||
var restoreChunkKeys = 10000 // non-const for testing | |||
var defaultCompactBatchLimit = 1000 | |||
var minimumBatchInterval = 10 * time.Millisecond |
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.
Should rename to defaultBatchInterval
.
By 'minimum,' we mean that the interval should be adjusted to 10ms if users set a smaller value, but clearly, the current implementation does not reflect this.
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.
In mvcc/kvstore_compaction.go:store.scheduleCompaction() func, users can set the compaction number by "experimental-compaction-sleep-interval". However, the sleep interval is a constant btw every compaction loop which is 10ms.
I wanna test how long the interval is perfect in my huge k8s cluster if I can set this param by "experimental-compaction-sleep-interval".