feat(database): badger plugin value threshold #1155
Conversation
📝 WalkthroughWalkthroughAdds configurable value-threshold support to the Badger blob store plugin. Introduces a Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used🧬 Code graph analysis (3)database/plugin/blob/badger/options.go (1)
database/plugin/blob/badger/plugin.go (2)
database/plugin/blob/badger/database.go (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🔇 Additional comments (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
wolf31o2
force-pushed
the
feat/badger-value-threshold
branch
from
a55051b to
7b99788
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
database/plugin/blob/badger/database.go (1)
181-191: ApplyWithValueThreshold(db.valueThreshold)to the in-memory Badger configuration.The
valueThresholdis applied to the on-disk configuration (line 214) but missing from the in-memory configuration (lines 181-191). According to Badger documentation,WithValueThresholdshould be applied to both configurations as it controls value-log placement in the LSM tree organization, even for in-memory mode. The value threshold has already been initialized at line 172, so it should be consistently applied to both paths:badgerOpts := badger.DefaultOptions(""). WithLogger(NewBadgerLogger(db.logger)). WithLoggingLevel(badger.WARNING). WithInMemory(true). WithValueThreshold(db.valueThreshold)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
database/plugin/blob/badger/database.godatabase/plugin/blob/badger/options.godatabase/plugin/blob/badger/plugin.go
🧰 Additional context used
🧬 Code graph analysis (3)
database/plugin/blob/badger/database.go (2)
database/plugin/blob/badger/plugin.go (1)
DefaultValueThreshold(29-29)database/plugin/blob/badger/options.go (1)
WithValueThreshold(84-88)
database/plugin/blob/badger/plugin.go (2)
database/plugin/option.go (1)
PluginOptionTypeUint(32-32)database/plugin/blob/badger/options.go (1)
WithValueThreshold(84-88)
database/plugin/blob/badger/options.go (1)
database/plugin/blob/badger/database.go (1)
BlobStoreBadger(147-161)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: nilaway
- GitHub Check: go-test (1.24.x, macos-latest)
- GitHub Check: go-test (1.24.x, windows-latest)
- GitHub Check: go-test (1.25.x, ubuntu-latest)
- GitHub Check: go-test (1.25.x, windows-latest)
- GitHub Check: go-test (1.25.x, macos-latest)
- GitHub Check: Analyze (go)
🔇 Additional comments (9)
database/plugin/blob/badger/options.go (1)
82-88: LGTM!The
WithValueThresholdoption function follows the established pattern used by other option functions in this file (WithMemTableSize,WithValueLogFileSize, etc.).database/plugin/blob/badger/database.go (3)
159-159: LGTM!The
valueThresholdfield is correctly added to the struct, consistent with the other size-related fields.
172-172: LGTM!Good initialization with
DefaultValueThresholdand helpful inline comment explaining the purpose.
207-215: LGTM!The
WithValueThresholdis correctly integrated into the Badger options chain for on-disk storage.database/plugin/blob/badger/plugin.go (5)
28-29: Note:DefaultMemTableSizedoubled from 64MB to 128MB.This is a behavioral change that will increase memory usage for existing deployments. The new
DefaultValueThresholdof 1MB is a reasonable default for Badger's value log separation policy.Consider documenting this default change in release notes, as it affects memory consumption for existing users upgrading to this version.
39-39: LGTM!The
valueThresholdfield is correctly added to the command-line options structure withuint64type, matching other size-related options.
53-53: LGTM!Correct initialization of
valueThresholdwith the default constant.
110-116: LGTM!The
value-thresholdcommand-line option is correctly registered following the established pattern for other size-related options.
133-150: LGTM!The bounds checking for
valueThresholdfollows the established pattern used forvalueLogFileSizeandmemTableSize. The conversion toint64is safe after the bounds check.
Signed-off-by: Chris Gianelloni <[email protected]>
wolf31o2
force-pushed
the
feat/badger-value-threshold
branch
from
7b99788 to
b02bf2f
Compare
Summary by cubic
Add a configurable Badger value threshold to control when values stay in LSM vs the value log, defaulting to 1MB. Also increases the default memtable size to 128MB for better write buffering.
Written for commit b02bf2f. Summary will update automatically on new commits.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.