Skip to content

perf(cubestore): enable bloom filter for metastore rocksdb reads#11083

Open
waralexrom wants to merge 1 commit into
masterfrom
cubestore-metastore-rocksdb-cache-bloom
Open

perf(cubestore): enable bloom filter for metastore rocksdb reads#11083
waralexrom wants to merge 1 commit into
masterfrom
cubestore-metastore-rocksdb-cache-bloom

Conversation

@waralexrom

Copy link
Copy Markdown
Member

Summary

Speeds up CubeStore metastore RocksDB point-gets by enabling bloom filters. Under load (mass pre-aggregation imports → huge partition/chunk space), the metastore coordinator does many get_row(id) point lookups that hit disk; bloom filters let RocksDB skip SSTs that can't contain a key.

Changes

  • Add set_bloom_filter(10.0, false) + set_whole_key_filtering(true) to the metastore BlockBasedOptions in both open_db and open_readonly_db (metastore/mod.rs).
  • A fixed_prefix(13) extractor is already set, so by default this yields a prefix bloom; the hot path is whole-key point-gets (RowKey::Table(table_id, id)), so whole-key filtering is enabled too.
  • Metastore only — cachestore options are untouched.

Compatibility

Read-path / open-time options only: no on-disk key format or table/index VERSION change, so an existing store opens without rebuild/reindex/migration. Blooms are per-SST and materialize lazily on flush/compaction — existing SSTs read fine without one (a one-time online compact_range would give immediate full coverage but is not required).

Testing

  • cargo build -p cubestore — green.
  • cargo test -p cubestore metastore:: — 28 passed, 0 failed (DB open + round-trip, snapshots, cold start, log replay).
  • Local stend brought up from this branch against an existing store: opened intact (1.5M-row table served, no migration in logs), re-confirming no-rebuild compatibility.

Add whole-key + prefix bloom filters to the metastore RocksDB block-based
table options (open_db and open_readonly_db). The hot read path is whole-key
point-gets (RowKey::Table(table_id, id)), so set_whole_key_filtering is
enabled alongside the default prefix bloom from the fixed_prefix(13) extractor.

Read-path only: no on-disk key format or table/index VERSION change, so an
existing store opens without rebuild/reindex/migration. Blooms are per-SST and
materialize lazily on flush/compaction; existing SSTs read fine without one.
@waralexrom waralexrom requested a review from a team as a code owner June 14, 2026 17:48
@github-actions github-actions Bot added cube store Issues relating to Cube Store rust Pull requests that update Rust code labels Jun 14, 2026
@claude

claude Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Claude finished @waralexrom's task in 1m 12s —— View job


Review

Verdict: LGTM. The change is correctly scoped, the rationale is sound, and the compatibility story holds.

Details — analysis, notes, minor suggestions

Correctness

  • RowKey::Table encodes as 1 byte tag + 4-byte table_id + 8 zero bytes + 8-byte row_id = 21 bytes (rocks_store.rs:412-420). The existing fixed_prefix(13) extractor groups by (tag, table_id, zero pad) — i.e. per-table — so prefix bloom is useful for table-scoped iteration; the set_whole_key_filtering(true) addition is what actually accelerates the hot get_row(id) path here. Both make sense together.
  • RowKey::Sequence / TableInfo / SecondaryIndexInfo encode to keys shorter than 13 bytes (5 bytes each). With a fixed-length prefix extractor those are out-of-domain and the prefix bloom is skipped; whole_key_filtering=true means whole-key bloom still applies to them. Behavior is correct.
  • set_bloom_filter(10.0, false) → 10 bits/key, full filter (not block-based). Standard recommended setting.

Performance / operational notes (non-blocking)

  1. Filter blocks live in the block cache by default. Under cache pressure, freshly-loaded filter blocks can be evicted, blunting the win you're shipping for. Worth considering as a follow-up (not this PR):

    block_opts.set_cache_index_and_filter_blocks(true);
    block_opts.set_pin_l0_filter_and_index_blocks_in_cache(true);
    // or, more aggressive:
    // block_opts.set_pin_top_level_index_and_filter(true);

    For metastore, where the SST set is small and read-hot, pinning filters is usually a clear win.

  2. Existing SSTs. The PR body already calls this out — old SSTs have no bloom until they're rewritten by compaction. If you want immediate full coverage on an existing store after deploy, a one-time compact_range(None, None) would do it. Fine to leave out; lazy materialization is the safer default.

  3. Duplication. The block-options builder is now duplicated almost verbatim between open_db (mod.rs:1284-1302) and open_readonly_db (mod.rs:1322-1340). Pre-existing pattern, but it just got 7 more lines of drift risk. A small helper fn build_metastore_block_opts(rocksdb_config) -> Result<BlockBasedOptions, CubeError> would centralize it. Not a blocker.

Scope check

  • Metastore-only — cachestore opts untouched, matches the PR description.
  • No on-disk format change; the open-time options are additive. Compatibility claim is accurate.
  • Build + metastore tests reported green in the PR body.

Nothing else jumped out.

| [Branch](https://github.com/cube-js/cube/tree/cubestore-metastore-rocksdb-cache-bloom)

@codecov

codecov Bot commented Jun 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.50%. Comparing base (26eec6f) to head (7b870d7).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #11083   +/-   ##
=======================================
  Coverage   58.50%   58.50%           
=======================================
  Files         216      216           
  Lines       17270    17270           
  Branches     3524     3524           
=======================================
  Hits        10103    10103           
  Misses       6652     6652           
  Partials      515      515           
Flag Coverage Δ
cube-backend 58.50% <ø> (ø)

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

☔ View full report in Codecov by Harness.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cube store Issues relating to Cube Store rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant