Skip to content

Conversation

@tac0turtle
Copy link
Contributor

@tac0turtle tac0turtle commented Jan 2, 2026

Overview

Tune badgerdb for our write heavy usecase, this is a small change i dont believe we need to go beyond this

closes #2605

@github-actions
Copy link
Contributor

github-actions bot commented Jan 2, 2026

The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed⏩ skipped✅ passed✅ passedJan 9, 2026, 10:46 AM

@claude
Copy link
Contributor

claude bot commented Jan 2, 2026

Claude finished @tac0turtle's task in 2m 17s —— View job


Comprehensive Code Review

I've completed a thorough review of this PR. The changes are well-structured and provide valuable performance tuning for BadgerDB. Below is my detailed feedback across all review areas:


Summary

Overall Assessment: ✅ Approve with minor suggestions

This PR successfully tunes BadgerDB for ev-node's write-heavy workload and adds a useful benchmarking tool. The changes are backward compatible, well-documented, and follow Go best practices. The tuned options are sensible for append-heavy patterns with periodic overwrites.


1. Code Quality & Architecture

Strong Points

BadgerDB Options (pkg/store/badger_options.go):

  • Clean separation of concerns with dedicated options configuration
  • Well-documented rationale for each tuning parameter
  • CPU-based scaling for compactors (4-8 range) is sensible and prevents over-saturation
  • Follows idiomatic Go style with clear function names

Integration (pkg/store/kv.go):

  • Minimal, focused changes that apply the tuned options
  • Proper reuse of BadgerOptions() in both persistent and in-memory stores
  • No breaking changes to the public API

Benchmark Tool (tools/db-bench/main.go):

  • Well-structured with clear separation: config parsing, profile execution, result presentation
  • Profile abstraction allows easy comparison between configurations
  • Reproducible results with seeded random data

🔍 Minor Observations

  1. compactorCount() function (badger_options.go:27-37):

    • The function is simple and clear, but could benefit from a brief comment explaining the 4-8 range rationale
    • Consider: Is there any scenario where you'd want to override this dynamically? (probably not needed, but worth considering for future extensibility)
  2. Benchmark tool config (main.go:21-30):

    • The config struct is well-designed, but all fields are unexported. This is fine for a standalone binary, but if you ever want to use this as a library, you'd need to export them.

2. Error Handling & Resilience

Good Practices

  • Consistent error wrapping with fmt.Errorf and %w throughout the benchmark tool
  • Proper cleanup with defer kv.Close() at main.go:136 (good fix!)
  • Input validation in parseFlags() (lines 95-106) prevents invalid configurations
  • Graceful exit with meaningful error messages

⚠️ Issues to Address

  1. Ignored error in cleanup (main.go:67):

    if cfg.reset {
        _ = os.RemoveAll(profileDir)
    }

    Issue: If RemoveAll fails (e.g., permissions), the benchmark proceeds with potentially stale data, leading to confusing results.

    Recommendation: Log a warning or fail explicitly:

    if cfg.reset {
        if err := os.RemoveAll(profileDir); err != nil {
            fmt.Fprintf(os.Stderr, "Warning: failed to remove profile directory %s: %v\n", profileDir, err)
        }
    }

    Fix this →

  2. Redundant manual close calls (main.go:157, 164, 169, 178, 184):
    The code has defer kv.Close() at line 136 but also manual _ = kv.Close() calls in error paths. While not incorrect, this is redundant since defer will handle it.

    Recommendation: Remove the manual close calls in error paths (lines 157, 164, 169, 178, 184) for cleaner code.
    Fix this →

  3. Error handling in dirSize (main.go:223-238):
    The function correctly propagates errors from filepath.WalkDir, but could be more resilient to permission errors on individual files.


3. Security

No Security Issues Detected

  • No credentials or secrets exposed
  • Input validation prevents injection attacks
  • File operations use safe patterns (no path traversal vulnerabilities)
  • Deterministic RNG usage with rand.New(rand.NewSource(1)) is appropriate for benchmarking (no cryptographic requirements)
  • The //nolint:gosec at line 118 is correctly placed since this is non-crypto usage

ℹ️ Notes

  • The benchmark tool is for local testing only, not exposed as a service, so security surface is minimal
  • File permissions 0o755 (line 69) are appropriate for DB directories

4. Performance & Resource Efficiency

Excellent Performance Design

BadgerDB Tuning Rationale:

  1. WithDetectConflicts(false): Excellent choice for single-writer scenarios (ev-node's pattern). Removes unnecessary overhead.

  2. WithNumLevelZeroTables(10) & WithNumLevelZeroTablesStall(20):

    • Badger defaults are 5 and 15 respectively
    • Increasing these allows more bursty writes before compaction/stalling
    • Well-suited for block production patterns
    • Trade-off: Slightly higher read latency during bursts (acceptable for this use case)
  3. Dynamic compactor scaling (4-8):

    • Prevents thrashing on low-core machines (minimum 4)
    • Caps at 8 to avoid excessive background CPU usage
    • Good balance for write-heavy workloads

Benchmark Tool Performance:

  • Batch writes (default 1000) minimize disk I/O overhead
  • Pre-allocated value buffer (line 119) avoids per-write allocations
  • Context reuse is efficient
  • Sync() call (line 183) ensures data is flushed before timing stops

🔍 Minor Considerations

  1. Memory usage: The tuned options will use more memory (more L0 tables in memory). This is intentional and acceptable, but worth documenting if you have memory-constrained deployments.

  2. Benchmark value size: Default 1024 bytes might not match real-world block/state sizes. Consider testing with realistic value sizes (e.g., typical block header size, transaction size).


5. Testing & Reliability

⚠️ Coverage Concerns

Per Codecov: 7.10% patch coverage with 170 lines missing coverage (mostly in tools/db-bench/main.go).

Analysis:

  • The benchmark tool (166 missing lines) is a standalone utility, not production code
  • The core changes in badger_options.go have 71.42% coverage (2 missing + 2 partials)

Recommendations:

  1. Add unit tests for BadgerOptions() in pkg/store/badger_options_test.go:

    func TestBadgerOptions(t *testing.T) {
        opts := BadgerOptions()
        require.NotNil(t, opts)
        // Verify specific settings
    }
    
    func TestCompactorCount(t *testing.T) {
        // Test boundary conditions: low CPU, high CPU
    }

    Fix this →

  2. Integration test: Consider adding a test that verifies the tuned options don't break existing functionality:

    func TestTunedOptionsCompatibility(t *testing.T) {
        // Create store with tuned options
        // Perform writes
        // Reopen with same options
        // Verify data integrity
    }
  3. Benchmark tool testing: While 0% coverage for the benchmark tool is acceptable (it's a dev utility), you could add basic smoke tests to prevent regressions.

Reliability Features

  • Deterministic benchmarking with seeded RNG
  • Explicit sync before timing stops
  • DB size measurement for compression analysis

6. Documentation & Developer Experience

Strong Documentation

  1. README updates:

    • pkg/store/README.md now mentions the tuned options and benchmark tool
    • tools/db-bench/README.md provides clear usage examples
  2. Inline comments:

    • Each BadgerDB option has a clear rationale comment
    • Benchmark tool has helpful comments for complex logic
  3. Examples:

    • The benchmark README shows practical usage patterns

📝 Suggestions for Enhancement

  1. Document performance impact: Consider adding a section in pkg/store/README.md or tools/db-bench/README.md with actual benchmark results showing the improvement over defaults. This would help justify the tuning choices.

  2. Configuration override: Document how users could override these defaults if needed (e.g., for memory-constrained environments). Currently, there's no way to opt-out of the tuned options.

  3. Migration guide: Although this is backward compatible (as I confirmed earlier), a brief note in the README reassuring users that upgrading doesn't require data migration would be helpful.


7. Addressing Existing Review Feedback

Gemini Code Assist provided two suggestions:

  1. Error handling for os.RemoveAll (line 67): I agree with this suggestion and have included it in my recommendations above.

  2. Redundant kv.Close() calls: I agree with this suggestion. The defer kv.Close() at line 136 makes the manual close calls redundant.


8. Additional Observations

Positive Patterns

  1. Modular design: The profile abstraction allows easy extension to test other configurations
  2. Human-readable output: The humanBytes() function makes results easy to interpret
  3. Cleanup handling: The ensureBaseDir() pattern with cleanup function is elegant

Nitpicks (not blocking)

  1. Flag defaults: The default -bytes 512<<20 is a bit opaque. Consider using 512*1024*1024 or documenting that it's 512 MiB in a comment.

  2. Profile selection error message (line 60): Good error message, but could suggest go run ./tools/db-bench -h for help.


Conclusion

This is a well-executed PR that delivers meaningful performance improvements with minimal risk. The changes are:

  • ✅ Backward compatible
  • ✅ Well-documented
  • ✅ Following Go best practices
  • ✅ Addressing a real performance concern

Required Changes: None (blocking issues)

Recommended Changes (prioritized):

  1. Handle os.RemoveAll error at line 67 (easy fix, prevents confusing benchmark results)
  2. Remove redundant kv.Close() calls (cleanup)
  3. Add basic unit tests for BadgerOptions() and compactorCount() (improves coverage)

Optional Enhancements:

  • Document actual benchmark results
  • Add integration test for tuned options
  • Document configuration override mechanism

Final Verdict: ✅ LGTM with minor improvements suggested

The core changes to BadgerDB tuning are solid and ready to merge. The suggestions above would further improve code quality and testing, but are not blocking.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @tac0turtle, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the performance of the BadgerDB key-value store by introducing a set of finely-tuned default options tailored for the ev-node's specific write workload. To support validation and comparison, a new benchmarking utility has also been added, allowing developers to assess the impact of these optimizations.

Highlights

  • BadgerDB Option Tuning: Introduced a new BadgerOptions() function in pkg/store/badger_options.go to provide a set of tuned configurations for BadgerDB, specifically optimized for ev-node's append-heavy write patterns with periodic overwrites.
  • Configuration Details: The tuning includes disabling conflict detection, increasing NumLevelZeroTables to 10 and NumLevelZeroTablesStall to 20 to handle bursty writes, and dynamically scaling NumCompactors based on CPU count (between 4 and 8).
  • Integration of Tuned Options: The NewDefaultKVStore and NewTestInMemoryKVStore functions in pkg/store/kv.go were updated to utilize these new BadgerOptions() by default.
  • New Benchmarking Tool: A new command-line tool, db-bench, has been added under tools/db-bench. This tool allows for local benchmarking of BadgerDB performance, comparing the ev-node tuned options against Badger's default settings.
  • Documentation Updates: The pkg/store/README.md was updated to mention the tuned Badger options and the existence of the db-bench tool for validation. A new tools/db-bench/README.md was also added to document the usage of the benchmarking tool.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces performance tuning for BadgerDB by providing a set of default options tailored for the application's write patterns. It also adds a new benchmark tool, db-bench, to validate these performance improvements. The changes are well-structured and the new options are correctly applied. The benchmark tool is a valuable addition. I have provided a couple of suggestions to improve the implementation of the benchmark tool for better error handling and code clarity.

for _, p := range selected {
profileDir := filepath.Join(baseDir, p.name)
if cfg.reset {
_ = os.RemoveAll(profileDir)
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The error returned by os.RemoveAll is ignored. If removing the directory fails (e.g., due to permissions), the benchmark might run with old data or fail at a later stage, leading to confusing results. It would be better to handle this error, for instance by logging a warning to the user.

Suggested change
_ = os.RemoveAll(profileDir)
if err := os.RemoveAll(profileDir); err != nil {
fmt.Fprintf(os.Stderr, "Warning: failed to remove profile directory %s: %v\n", profileDir, err)
}

@codecov
Copy link

codecov bot commented Jan 2, 2026

Codecov Report

❌ Patch coverage is 7.10383% with 170 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.61%. Comparing base (453a8a4) to head (473ac84).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
tools/db-bench/main.go 0.00% 166 Missing ⚠️
pkg/store/badger_options.go 71.42% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2950      +/-   ##
==========================================
- Coverage   58.74%   57.61%   -1.14%     
==========================================
  Files          93       95       +2     
  Lines        8863     9043     +180     
==========================================
+ Hits         5207     5210       +3     
- Misses       3067     3242     +175     
- Partials      589      591       +2     
Flag Coverage Δ
combined 57.61% <7.10%> (-1.14%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tac0turtle tac0turtle requested a review from julienrbrt January 7, 2026 09:36
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Copy link
Member

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

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

utACK. @claude is this backward compatible?

@claude
Copy link
Contributor

claude bot commented Jan 9, 2026

Claude finished @tac0turtle's task in 55s —— View job


Analyzing Backward Compatibility

  • Read the BadgerDB options changes
  • Analyze compatibility impact
  • Provide detailed answer

Answer: Yes, this is backward compatible ✅

The changes in this PR are fully backward compatible with existing deployments. Here's why:

What Changed

The PR modifies runtime behavior only, not data format:

Before: badger4.NewDatastore(path, nil) → used BadgerDB's default options
After: badger4.NewDatastore(path, BadgerOptions()) → uses tuned options

The tuned options are:

  • WithDetectConflicts(false) - disables conflict detection (runtime only)
  • WithNumLevelZeroTables(10) - allows more L0 tables before compaction (runtime compaction policy)
  • WithNumLevelZeroTablesStall(20) - increases stall threshold (runtime write throttling)
  • WithNumCompactors(4-8) - scales compactor workers (runtime thread pool size)

Why It's Compatible

  1. No data format changes: These options only affect how BadgerDB processes data at runtime. They don't change:

    • Key encoding
    • Value encoding
    • SSTable format
    • Log structure
    • File layout
  2. Existing databases open cleanly: When you open an existing BadgerDB directory with these new options, BadgerDB will:

    • Read all existing data without issues
    • Apply the new compaction policies going forward
    • Continue writing in the same format
  3. Configuration-level changes: These are runtime configuration parameters, similar to changing cache sizes or thread counts. They don't require database migration or version upgrades.

  4. Safe option choices:

    • Disabling conflict detection is safe for single-writer scenarios (ev-node's pattern)
    • L0 table tuning only affects when compaction triggers, not how it works
    • Compactor count just scales parallelism

Migration Path

No migration needed! Nodes can upgrade by:

  1. Stop the node
  2. Deploy the new binary with this code
  3. Start the node

The existing BadgerDB data will be read normally, and the new tuning will apply to subsequent operations.

Rollback Safety

If needed, you can roll back to the old code (nil options) and the database will continue working. The only difference is the runtime behavior would revert to BadgerDB defaults.


Bottom line: This is a safe, non-breaking performance optimization. Existing deployments can upgrade without any migration steps or data conversion.

@tac0turtle tac0turtle enabled auto-merge January 9, 2026 10:45
@tac0turtle tac0turtle added this pull request to the merge queue Jan 9, 2026
Merged via the queue into main with commit b1377b8 Jan 9, 2026
25 of 27 checks passed
@tac0turtle tac0turtle deleted the perf-badger-options-bench branch January 9, 2026 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf: configure db options

3 participants