fix(cubestore): stop main-node OOM from concurrent metastore scans by waralexrom · Pull Request #11082 · cube-js/cube

waralexrom · 2026-06-13T15:24:14Z

Summary

Main/router (-main-) nodes periodically spiked to 20-24GB and OOM-killed under load. Root cause: several O(all-chunks) metastore operations run out-of-queue (concurrent, ungated) with no single-flight, so a burst of callers (imports / per-node compaction jobs) each materialize the whole tenant's chunk set at once → peak memory ~ N × scan. This PR bounds and deduplicates those scans.

Based on cubestore-early-split. The 4 commits are independent and meant to be reviewed/merged one at a time.

Changes

disk-space check (get_used_disk_space_out_of_queue, hit per persisted chunk write on import + on CREATE TABLE):
- stream the partition+chunk scan via scan_all_rows() instead of materializing full Vecs → per-call memory O(partitions), not O(chunks);
- single-flight cache (double-checked lock) so concurrent cold-cache callers share one scan.
in-memory compaction discovery (get_partitions_for_in_memory_compaction):
- cache + single-flight on the discovery scan (ids only, no chunk Vecs), TTL = compaction_in_memory_chunks_schedule_period_secs; chunks re-read fresh per partition (indexed) so compaction acts on current data; public signature unchanged (no RPC/trait change);
- skip the discovery scan entirely when no streaming table is present (in_memory_ingest() = seq column), via the in-RAM table cache.
repro harness: benches/disk_space_stampede.rs + run-stampede-docker.sh + tracking_allocator getters.

Testing

cargo check -p cubestore --all-targets green on top of cubestore-early-split.
New metastore regression test in_memory_compaction_discovery_streaming_gate passes; existing in_memory_compaction test unaffected.
Bench (200k chunks): disk-space peak 1181 MB → 0.4 MB at K=16 and flat in K (streaming); with cache enabled total allocations flat in K (single-flight dedup), vs ~linear without.
Behavior preserved: in-memory compaction discovers/compacts the same partitions; per-partition chunk predicate kept identical (active && in_memory).

🤖 Generated with Claude Code

claude · 2026-06-13T15:24:29Z

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

Code Review

Verdict: LGTM with minor notes. 0 high, 0 medium, 4 low.

Details

Summary

The PR adds two cache + single-flight layers and a streaming-table gate to bound metastore-scan memory on the router. Logic is clean and the public surface is unchanged. I read the full diff and traced the hot paths; correctness looks preserved.

What's done well

Double-checked locking is correct. Both disk_space_cache and in_memory_compaction_cache use the canonical pattern: read cache → take compute mutex → re-read cache → compute → write cache. Concurrent cold-cache callers correctly collapse to one scan. (mod.rs:2830-2860, mod.rs:1545-1580)
Streaming gate via the in-RAM table cache (get_tables_with_path(false)) makes the no-streaming case essentially free, which is the common router shape. Good call.
include_inactive=true + explicit active && in_memory filter in the per-partition re-read (mod.rs:3377-3380) preserves the original predicate exactly, and the comment explains why (avoiding the active() ⇒ uploaded() shortcut).
Streaming the partition/chunk scan with scan_all_rows() is the right fix — old code materialized a full Vec<IdRow<Chunk>> before doing anything. Peak now really is O(partitions).
Stale-id handling in get_partitions_for_in_memory_compaction (re-resolve partition; skip if gone or split) is the right way to use a cached id list without holding a snapshot.

Low-severity notes (non-blocking)

Test doesn't fully prove the gate. In in_memory_compaction_discovery_streaming_gate, the regular-table chunk is chunk_uploaded(...)'d, which typically clears active until activate_chunks runs. If that chunk is inactive at assert time, the empty-result assertion holds even without the streaming gate — so the test could silently regress. Consider either activating the chunk explicitly, or asserting via a counter/log that no chunks-table scan happened in the no-streaming case. (mod.rs:5650-5685)
Streaming-gate check fires on every call. get_tables_with_path(false) is cached, but it still allocates a Vec of TablePath per call. With many tables × many discovery callers this is small but non-zero. If profiling ever shows it, gate it on a cached boolean refreshed at the same TTL.
No negative-result cache for the streaming gate. When in_memory_ingest() returns false for all tables, the function returns Vec::new() without writing to in_memory_compaction_cache. Fine today (the table-cache hit is cheap), but means the gate code path runs on every call even under steady-state no-streaming load. A trivial sentinel cache entry would dedupe it.
Map merge in get_used_disk_space_out_of_queue still has the pre-existing "lose-size-for-unknown-node" quirk at map.entry(node).and_modify(|s| *s += size).or_insert(0) — or_insert(0) drops the size for any node not in the initial workers set. Pre-existing (not introduced here), but worth fixing while you're in the area: .or_insert(size) instead of .or_insert(0).

Things I checked and didn't flag

Cache TTL choice for in-memory compaction (= compaction_in_memory_chunks_schedule_period_secs) — appropriate; freshly-written chunks are picked up within one scheduling tick.
Per-node return from a single shared by-node scan — correct; one node missing from the map maps to empty, which matches "no work for this node."
Mutex held across read_operation_out_of_queue — fine; out-of-queue path doesn't serialize against the write queue, and the compute-lock is the intended bottleneck.
No new RPC/trait change. Confirmed.

· branch `cubestore-omm-on-main-investigation`

codecov · 2026-06-13T15:49:41Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.50%. Comparing base (26eec6f) to head (3db1965).

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #11082   +/-   ##
=======================================
  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.

claude · 2026-06-13T17:36:07Z

Claude finished @waralexrom's task in 2m 28s —— View job

Code Review (update — new commits)

Verdict: LGTM. New commits since the prior review look correct. 0 high, 0 medium, 4 low (3 new, plus the prior metastore notes still stand).

Details

Scope of this update

Three new commits on top of the previously-reviewed metastore-scan fixes:

fd9066e — guard partition_rows_for_index against sort_key_size > columns.len(), deactivate table instead of panicking
b620ff2 — wait_for_job_results resilient to dropped/lagged completion events: handles Lagged/Orphaned, optional 30 s wall‑clock safety poll, broadcast channel 8192 → 32768
53b17ea — flip error/query order in MySQL error log

Plus 0b19e9f (drop bench harness) — pure removal.

The earlier metastore changes (cluster/mod.rs, metastore/mod.rs) are unchanged on disk vs. the previous review, so the 4 low‑severity notes from that pass still apply (test gate, per‑call get_tables_with_path, no negative‑result cache, .or_insert(0) quirk).

What's done well in the new commits

wait_for_job_results_with_poll shape is sound. tokio::time::interval_at with MissedTickBehavior::Delay and a biased select! is the right pattern — the comment correctly explains why a per‑iteration recv timeout would never fire on a busy router. (cluster/mod.rs:1064-1075)
Lagged path now re‑derives authoritatively from the metastore instead of returning an error. Previously a single capacity overflow killed finalize. (cluster/mod.rs:1147-1156)
Orphaned is now matched in the channel‑delivery arm (Success | Orphaned | Error). Before, channel‑delivered Orphaned never removed the job from results, so the wait would hang forever even when an event arrived. Strict improvement for all callers. (cluster/mod.rs:1119-1133)
resolve_pending_from_metastore "None ⇒ Orphaned" is the conservative choice and is documented. Callers (finalize_external_table) already handle JobEvent::Orphaned distinctly from Error.
Corrupt‑width guard mirrors the existing deactivate_table_due_to_corrupt_data pattern at mod.rs:1554-1562, and returns CubeError::corrupt_data so the upstream RepartitionChunk job no longer crash‑loops the worker.
Broadcast bump 8192 → 32768 is well‑scoped and the comment explains the orphaned‑job burst case. Memory cost is O(capacity) × event size per active receiver — negligible.

Low‑severity notes (new commits, non‑blocking)

columns[0].len() still panics if sort_key_size == 0 and columns.is_empty(). The new guard catches sort_key_size > columns.len() but not the degenerate case where both are zero — line 1490 dereferences columns[0] unconditionally. Almost certainly impossible in practice (every index has ≥1 sort‑key column), but cheap to harden: change the guard to sort_key_size == 0 || sort_key_size > columns.len(), or just check columns.is_empty() separately. (store/mod.rs:1471-1488)
Index {:?} in the corrupt‑data log message dumps the whole IdRow<Index> — columns, definitions, etc. The neighboring corrupt‑data message at line 1555 has the same shape, so this is consistent, but a log line like Index id={} for table_id={} expects ... would be easier to grep and bounded in size. Style/log‑hygiene only.
Behavior change in wait_for_job_results affects every existing caller, not just finalize_external_table. Previously Lagged propagated as an error via recv().await?; now it triggers a silent metastore re‑check on the no‑poll path too. That's the right default, but worth calling out in the PR description — streaming reconcile and test paths now also tolerate channel lag. No code change needed; verified all current callers (streaming/mod.rs, sql/mod.rs tests, table_creator.rs) handle the resulting JobEvents correctly (or are tests that just consume the vec).
get_job_by_ref is called in a tight loop on every recheck tick. For finalize jobs that's a handful per 30 s — fine. But if wait_for_job_results_with_poll ever gets used for a large wait_for vec, it'll do N point lookups per tick. Not a problem at current call sites; just flag it before promoting the polling path to other callers.

Things I checked and didn't flag

Reuse of wait_for_job_results API — only the internal mut self plumbing changed; all 16+ callers compile unchanged.
Orphaned‑then‑rescheduled race — once the listener reports Orphaned, the finalize path returns Orphaned to the caller; that's the existing contract.
needs_recheck flag flow — both arms set it before exit, and the recheck mutates results in the same async context (no concurrent mutation).
Broadcast capacity 32768 — MetaStoreEvent is a couple of IdRows on the heap; per‑listener worst case is bounded and reasonable.
MySQL log format change — the new format pulls the error to the front of the line; nothing parses this log.

· branch `cubestore-omm-on-main-investigation`

@200k

…und router memory The disk-space limit check (`get_used_disk_space_out_of_queue`, hit per persisted chunk write during CSV/location imports and on CREATE TABLE) materialized the entire partition AND chunk tables into Vecs via `get_all_partitions_and_chunks_out_of_queue`. Because it runs out-of-queue (ungated `spawn_blocking`) and the cache has no single-flight, a burst of concurrent chunk writes on a cold cache makes N callers each hold a full copy of the tenant's metastore at once — peak memory grows ~linearly with concurrency, producing the multi-GB spikes and OOM-kills on main/router nodes (observed ~20-24GB on prod tenants with millions of chunks). Rewrite the cold path to stream via `scan_all_rows()` inside a single read_operation_out_of_queue closure, accumulating only the per-partition size map (O(partitions)) and folding chunks one at a time. Fold semantics are preserved exactly (same overwrite + workers-seed behavior; scan order is identical since all_rows is scan_all_rows collected). Add a reproduction harness: `benches/disk_space_stampede.rs` (peak-memory scaling vs concurrency via TrackingAllocator), peak/current getters on the tracking allocator, and `run-stampede-docker.sh` for a faithful Linux/cgroup run. Bench @200k chunks/K=16: peak 1181 MB -> 0.4 MB, flat in K. Note: this bounds memory only; the redundant CPU of K concurrent full chunk scans remains and should be addressed with a single-flight disk_space_cache.

…etastore scans The disk-space limit check (`get_used_disk_space_out_of_queue`) cache had no single-flight: a burst of concurrent cold-cache callers (many partition writes during an import/repartition) each ran its own full metastore scan. Memory was already bounded by the streaming rewrite, but the redundant CPU/scan work scaled linearly with concurrency. Add a compute lock with double-checked caching: on a cache miss the first caller scans and populates the cache while the rest wait and reuse the fresh result, so a burst collapses to one scan per cache-TTL window instead of N. Bench (200k chunks): with cache_secs>0 total allocations stay flat across K (one scan shared); with cache_secs=0 (cache always stale) total scales ~linearly with K (126MB * K), while peak stays flat in both — confirming the dedup affects work, not the already-bounded memory. Adds a total_allocated() getter and a "total alloc" column to the repro bench.

… scan The in-memory compaction job (NodeInMemoryChunksCompaction → compact_node_in_memory_chunks) calls get_partitions_for_in_memory_compaction, which scanned the entire chunks table to find partitions with active in-memory chunks. This runs for every node on each scheduling tick (~5s), so a fan-out of concurrent jobs each scanned the whole table on the metastore node — the same N-concurrent-full-scans pattern that drives main-node CPU/memory pressure. Memoize the discovery: a short-lived (TTL = compaction_in_memory_chunks_schedule_period_secs) single-flight cache holds only the per-node partition ids (no chunk data), so concurrent jobs in a tick share one scan instead of each running it. The chunks are re-read fresh per partition (indexed by partition id) inside the same method, so compaction always acts on current data and the public signature is unchanged (no RPC/trait change). Cached ids are re-validated against a fresh snapshot (partition still active / not multi-partition, chunks non-empty) so stale ids are harmless. The per-partition chunk predicate is kept identical to the original (active && in_memory).

…g tables In-memory chunks are only produced by streaming ingestion — tables with in_memory_ingest() (a seq column, set when a unique key is defined). A regular INSERT/import writes persisted chunks (in_memory=false). So on a cluster with no such table the in-memory compaction discovery scan can never find anything, yet it re-scanned the whole chunks table on every node every scheduling tick. Short-circuit in_memory_compaction_partition_ids: if no ready table has in_memory_ingest(), return empty immediately and skip the full chunks scan. The check reads the in-RAM table cache (get_tables_with_path(false), kept fresh on table create/ready/drop), so it costs ~nothing in the common no-streaming case. With a streaming table present, behavior is unchanged (cached + single-flight discovery, fresh per-partition chunk re-read). Adds a metastore regression test (regular table => empty discovery; unique-key/streaming table => partition found). Existing in_memory_compaction test is unaffected (per-partition path).

…ness These were investigation-only (peak-memory / single-flight repro). The actual fixes live in metastore/mod.rs; the bench, its Cargo entry, the docker runner, and the tracking-allocator getters added for it are not worth keeping permanently.

…nicking partition_rows_for_index sliced `columns[0..sort_key_size]` unguarded. When a chunk's stored column count is below the index's sort key (schema/version skew), this panicked the RepartitionChunk job and crash-looped it, flooding the metastore event bus. Guard it: if sort_key_size > columns.len(), deactivate the table via the corrupt-data mechanism and return CorruptData — mirroring the existing corrupt-data handling already in this function.

…are lost The metastore broadcast channel can drop completion events under load (Lagged), and a busy router can otherwise miss/never-deliver an event, leaving CREATE TABLE finalize blocked in wait_for_job_results forever (table stuck "processing"). - wait_for_job_results: on Lagged, re-derive pending job outcomes authoritatively from the metastore instead of erroring; add an optional wall-clock safety poll (independent tokio::time::interval + biased select, so it fires on schedule regardless of unrelated channel traffic — a per-iteration recv timeout would be reset by every event and never fire on a busy node) that re-checks pending jobs. - Handle Orphaned symmetrically in the event and poll paths (a channel-delivered Orphaned was previously dropped, so the job was never removed from the wait set). - finalize_external_table opts into a 30s poll for the import job wait. - Raise the metastore event channel capacity 8192 -> 32768 to reduce Lagged.

"Error during processing <query>: <error>" buried the error after a potentially huge query. Print the error first, then the query on its own "Query:" line.

claude · 2026-06-14T14:42:35Z

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

Code Review (sync update)

Verdict: LGTM. No new substantive issues since the previous two passes. PR is already approved by @paveltiunov.

Details

Scope of this update

I re-checked the current tip against the prior reviews. The 8 commits on the branch match the changes already covered:

13b1f9c stream partition+chunk scan in disk-space check
c66b856 single-flight disk-space cache
6339c1e cache + single-flight in-memory compaction discovery
02354b6 skip discovery when no streaming tables
bd52291 drop bench harness
6992edd deactivate table on corrupt chunk width
5dd2628 keep import finalize live when job-completion events are lost
3db1965 log query-processing error before the query

No code changes since the last pass — same diff content, same file SHAs as the second review covered.

Carry-over notes from prior passes (still applicable, all low / non-blocking)

store/mod.rs:1471 — guard catches sort_key_size > columns.len() but columns[0].len() at line 1490 still panics if columns.is_empty() and sort_key_size == 0. Trivial to harden (columns.is_empty() || sort_key_size > columns.len()).
metastore/mod.rs — in_memory_compaction_discovery_streaming_gate test does not prove the gate (regular-table chunk is chunk_uploaded only, so it's inactive at assert time and the empty result holds even without the gate).
metastore/mod.rs — pre-existing or_insert(0) in get_used_disk_space_out_of_queue drops size for nodes outside the initial workers set; should be or_insert(size).
metastore/mod.rs — streaming gate has no negative-result cache, so the table-cache lookup runs on every discovery call under steady-state no-streaming load. Trivial sentinel would dedupe.

None of these block merging.

· `cubestore-omm-on-main-investigation`

waralexrom requested a review from a team as a code owner June 13, 2026 15:24

github-actions Bot added cube store Issues relating to Cube Store rust Pull requests that update Rust code labels Jun 13, 2026

paveltiunov approved these changes Jun 14, 2026

View reviewed changes

waralexrom force-pushed the cubestore-early-split branch from 0c0b1dc to 6742a90 Compare June 14, 2026 06:37

Base automatically changed from cubestore-early-split to master June 14, 2026 14:31

waralexrom added 8 commits June 14, 2026 16:37

fix(cubestore): log query-processing error before the query

3db1965

"Error during processing <query>: <error>" buried the error after a potentially huge query. Print the error first, then the query on its own "Query:" line.

waralexrom force-pushed the cubestore-omm-on-main-investigation branch from 53b17ea to 3db1965 Compare June 14, 2026 14:42

vercel Bot deployed to Preview June 14, 2026 14:44 View deployment

waralexrom merged commit ee86b13 into master Jun 14, 2026
74 of 76 checks passed

waralexrom deleted the cubestore-omm-on-main-investigation branch June 14, 2026 17:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(cubestore): stop main-node OOM from concurrent metastore scans#11082

fix(cubestore): stop main-node OOM from concurrent metastore scans#11082
waralexrom merged 8 commits into
masterfrom
cubestore-omm-on-main-investigation

waralexrom commented Jun 13, 2026

Uh oh!

claude Bot commented Jun 13, 2026 •

edited

Loading

Summary

What's done well

Low-severity notes (non-blocking)

Things I checked and didn't flag

Uh oh!

codecov Bot commented Jun 13, 2026 •

edited

Loading

Uh oh!

claude Bot commented Jun 13, 2026 •

edited

Loading

Scope of this update

What's done well in the new commits

Low‑severity notes (new commits, non‑blocking)

Things I checked and didn't flag

Uh oh!

claude Bot commented Jun 14, 2026 •

edited

Loading

Scope of this update

Carry-over notes from prior passes (still applicable, all low / non-blocking)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

waralexrom commented Jun 13, 2026

Summary

Changes

Testing

Uh oh!

claude Bot commented Jun 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review

Summary

What's done well

Low-severity notes (non-blocking)

Things I checked and didn't flag

Uh oh!

codecov Bot commented Jun 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

claude Bot commented Jun 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review (update — new commits)

Scope of this update

What's done well in the new commits

Low‑severity notes (new commits, non‑blocking)

Things I checked and didn't flag

Uh oh!

claude Bot commented Jun 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review (sync update)

Scope of this update

Carry-over notes from prior passes (still applicable, all low / non-blocking)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

claude Bot commented Jun 13, 2026 •

edited

Loading

codecov Bot commented Jun 13, 2026 •

edited

Loading

claude Bot commented Jun 13, 2026 •

edited

Loading

claude Bot commented Jun 14, 2026 •

edited

Loading