feat(master): schedule generic-store WAL merges through the etcd-locked MergeWal task - #264
Merged
beinan merged 1 commit intoSep 23, 2026
Conversation
…ed MergeWal task
Generic stores were never registered with the master. Their MemWAL merges
ran only on each worker's own timers, so all 20 workers raced to commit
their shard's generations into one base table. Under object-store
throttling every Lance commit-conflict retry hit its 30s cap and most
merges failed with 'Too many concurrent writers'; the generations that did
not merge kept the read fan-out (and the throttling) going. The master's
MergeWal task -- per-target etcd lock, one task per store, fan-out to the
workers -- exists to serialize exactly this, and rollout stores have used
it all along.
- MasterState opens _registry.generic.lance alongside the rollout registry
and exposes generic_uri / generic_store_options.
- The stats scan enumerates both registries. Generic stores are observed
(version, base rows, fragments, pending WAL generations) and written to
the stats table under a 'generic:' prefixed name, so the WAL-merge sweep
enqueues them as generic MergeWal tasks with no change to the etcd task
schema, dedupe keys or target locks. Store names cannot contain ':'.
- run_merge_wal routes a 'generic:' target to POST
/api/v1/generic/{name}/merge-wal and a bare target to the rollout route.
- Fan-out is now serial: every worker's merge commits a new version of the
same base table, so join_all was N writers racing one commit point even
under the task lock.
- Compaction / IndexId tasks and the compaction sweep are rollout-only;
generic rows are skipped, and retirement only considers rollout rows.
Tests (etcd-backed, like their siblings): a generic target reaches the
generic route with the bare name; workers are never called concurrently;
the merge sweep enqueues over-threshold generic rows as generic targets;
the compaction sweep skips them. Scanner: a generic store's pending
generations are observed under its prefixed name and skipped when unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Sep 23, 2026
beinan
added a commit
that referenced
this pull request
Sep 25, 2026
## Problem `merge_max_bytes` bounds how much **one** merge buffers. Nothing bounded how many merges a process ran at once. Every merge entry point — the worker's flush and cleanup sweepers, the count trigger, the manual `/merge-wal` route, and the master's fan-out — reserved memory independently. Once the master began scheduling merges for every store (#264) and fanning out to every worker, a worker could be asked to hold a dozen 1 GiB merge reads at once. Production, 2026-09-25: scaling masters 3→6 OOMKilled **17 of 20 workers within 90 seconds** (each had served ~370 merges in the hour); scaling back to 3 still produced a second wave of 6 OOMs 75 minutes later. Per-merge caps were all honoured; their sum wasn't. ## Fix `MergeMemoryBudget` — one per process, measured in **bytes**, shared by every merge regardless of trigger. - Before reading anything, a merge reserves `min(merge_max_bytes, budget)` in **one** atomic acquire (`Semaphore::acquire_many`, 1 permit = 1 MiB). If the budget is exhausted it **waits** for a release; it is never rejected. A master fanning out to a busy worker sees a slower worker, not a failure. - The reservation only grows mid-read for an oversized first generation, which is folded whole (existing rule). By then the holder has at least as much as any waiter could need, so growth can't form a cycle. - A request larger than the whole budget is admitted once it is the sole holder — same lone-oversized rule as `BlobBudget` — so an oversized generation still makes progress. - The RAII `MergeReservation` travels inside `PreparedMerge` and is dropped right after `merge_prepared_batches` consumes the batches, before the manifest drain and directory deletes. **Why this can't deadlock:** the full initial reservation is taken before any I/O, so no merge ever holds part of what it needs while waiting for the rest. Server: `ROLLOUT_MERGE_MEMORY_BYTES` (default 3 GiB; `0` disables) builds the budget in `AppState` and threads it into rollout/generic/datagen/context store options. With the total bounded, `merge_max_bytes` becomes an efficiency knob (fewer, larger commits) rather than the OOM safety valve. Metric: `rollout_merge_budget_wait_seconds` (job-scale buckets). A rising tail means merges are queueing on memory — intended under load, not an error. ## Verification - `cargo clippy --workspace --all-targets -D warnings` clean. - `merge_budget::tests`: reserve/release, exhausted budget makes the next reserve wait and resume on release, oversized request takes the whole budget and proceeds, `grow_to` waits for the extra and is a downward no-op. - `generic_store::tests::concurrent_merges_queue_on_the_shared_memory_budget`: two real stores, budget sized for one initial reservation; the second `prepare_cleanup_merge` blocks until the first `commit_prepared_merge`, then completes; budget reads 0 at the end. - Will be load-tested on the staging fleet with 6 masters before production. Refs #264, #256. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Beinan Wang <> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
beinan
added a commit
that referenced
this pull request
Sep 26, 2026
…e-enqueueing them (#267) Closes #266. ## Problem A store whose base-table manifest names a fragment that no longer exists fails every `MergeWal` and `Compact` at the same point, forever: ``` task failed target=<store> error="Not found: .../<store>.rollout.lance/data/<frag>.lance" ``` The scheduler has no memory of failure. `sweep_merge_wal_inner` / `sweep_candidates_inner` re-enqueue every over-threshold store on every tick (`enqueue` only de-dupes against queued/running), so a task that just failed is back in the queue ten minutes later, and each attempt holds a `TASK_CONCURRENCY` slot through a full serial worker fan-out before failing. Production 2026-09-26: five such stores (one at **7,985** pending generations) consumed roughly half of 6 masters' merge slots; the merge queue for healthy stores went 0 → 109 in an hour. ## Fix `TaskStore` records consecutive failures per `(kind, target)` in etcd under `<prefix>/cooldown/<kind>/<target>`, leased for the cooldown duration so it ages out on its own. - After `TASK_COOLDOWN_AFTER_FAILURES` (default **3**), the sweeps skip the target for `TASK_COOLDOWN_BASE_SECS` (default **600**), doubling per further failure up to `TASK_COOLDOWN_MAX_SECS` (default **21600**). `0` disables. - A success clears the record. A manual `POST /tasks` is **not** gated — operators can always force an attempt. - Below the threshold the record only carries the count (leased for `max`, so a slow trickle of unrelated failures doesn't accumulate forever) and is not a cooldown. - Entering cooldown emits `warn!` with the last error and `master_task_cooldowns_total{kind}`; `GET /api/v1/scheduler/cooldowns` returns `Vec<TaskCooldown>` (kind, target, failures, until_ms, last_error) so a broken store is visible instead of silently eating capacity. - Bookkeeping is best-effort after the task's terminal state is committed; a failure to write the cooldown key never fails the task completion. ## Verification - `cargo clippy --workspace --all-targets -D warnings` clean. - etcd-backed suite 26/26 (`ETCD_TEST_ENDPOINTS` against local etcd 3.7), including new `repeated_failures_cool_the_target_down_and_sweeps_skip_it`: threshold 2, no worker endpoints so every MergeWal fails; asserts the target is *not* cooling after one failure, *is* after two, the sweep then enqueues 0, `list_cooldowns` reports it with `until_ms` set, and a manual enqueue succeeds. - Writing the test caught a bug in the first draft: a sub-threshold record was treated as a cooldown. Fixed (`is_cooling_down` requires `until_ms`). Refs #264, #261. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Beinan Wang <> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Generic stores were never registered with the master. Their MemWAL merges ran only on each worker's own timers (
spawn_flush_sweeper/spawn_global_sweeper), so all N workers raced to commit their shard's flushed generations into one base table.In production (20 workers, ABFS) under storage-account throttling this failed continuously:
Lance's commit-conflict retry re-runs the whole
merge_insertand gives up after 30 s wall clock. With 20 writers on one commit point and a throttled object store, the second attempt never completes in time. The generations that did not merge kept the read fan-out — every read opened ~500 generation datasets — which kept the account throttled. Serially triggering/merge-walby hand while worker timers were disabled still failed on ~30% of shards.The master's
MergeWaltask — per-target etcd lock, one task per store, fan-out toWORKER_ENDPOINTS— exists to serialize exactly this, and rollout stores have used it from the start. Generic stores just weren't wired into it.Fix
Bring generic stores onto the existing master path; no new mechanism.
MasterStateopens_registry.generic.lance(sameRolloutRegistrytype the data plane writes) alongside the rollout registry; addsgeneric_uri()/generic_store_options().GenericStore::open_existing, and their rows are written under ageneric:<name>target. Store names match[A-Za-z0-9_][A-Za-z0-9._-]*so:is unambiguous, and carrying the kind in the target string keeps the etcd task schema, dedupe keys and per-target locks unchanged.sweep_merge_wal_innertherefore picks up generic rows overmerge_wal_min_generationswith no change.run_merge_walparses the target:generic:→POST /api/v1/generic/{name}/merge-wal, bare →/api/v1/internal/merge-wal/{name}.join_all. Every worker's merge commits a new version of the same base table, so parallel fan-out was N writers racing one commit point even with the task lock held. This applies to rollout too.Compact/IndexIdtasks and the compaction sweep remain rollout-only (generic rows are skipped with a clear error if enqueued by hand); retirement only considers rollout rows.Verification
cargo clippy --workspace --all-targets -D warningsclean.ETCD_TEST_ENDPOINTSagainst a local etcd 3.7), including four new tests:merge_wal_routes_generic_targets_to_the_generic_endpoint— ageneric:gstask hits/api/v1/generic/gs/merge-walwith the bare name and never the rollout route;merge_wal_calls_workers_serially— three stub workers, shared in-flight counter, asserts max in-flight is 1;sweep_merge_wal_enqueues_over_threshold_and_dedupes— extended: a generic row over threshold is enqueued asgeneric:ghotalongside the rollout row, and de-dupes on the second sweep;compaction_sweep_skips_generic_rows;generic_store_is_observed_with_pending_wal— 3 sealed adds → row undergeneric:gwithpending_wal_generations == 3, skipped on an unchanged re-scan.Deployment note: with the master driving generic merges, the worker-side count trigger for generic should be off (
ROLLOUT_MERGE_AFTER_GENERATIONS=0) and the 300 s cleanup kept as a fallback — the same posture rollout already runs with.Refs #256, #257, #258 (supersedes the "unified maintainer" direction there — the right answer was the existing master path), #263.
🤖 Generated with Claude Code