Skip to content

fix(server): the count-triggered self-merge takes the worker's merge slot - #289

Merged
beinan merged 2 commits into
lance-format:mainfrom
beinan:fix/sweeper-merge-takes-slot
Oct 1, 2026
Merged

beinan merged 2 commits into
lance-format:mainfrom
beinan:fix/sweeper-merge-takes-slot

Conversation

@beinan

@beinan beinan commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

Problem

ROLLOUT_MERGE_AFTER_GENERATIONS is the pending-driven merge trigger: when a worker's own shard has ≥ N flushed generations, the 30 s flush sweeper merges it. That is exactly what hot stores need (a store written every few seconds regrows hundreds of generations between the master's 600 s sweeps, and every read has to open all of them — 8–13 s per read on a 176-generation store today).

But that sweeper merge bypassed the per-worker merge slot (ROLLOUT_MERGE_CONCURRENCY, #278). Enabling the count trigger would stack sweeper merges on top of the master's merge requests with no memory bound.

Fix

flush_pass takes the worker's merge-slot semaphore and acquires a permit around merge_if_due, so sweeper-initiated and master-initiated merges share one bound. No behaviour change while the count trigger is 0 (the default and current production value).

Verification

Server suite passes (85); clippy clean. Production enablement is a config change in the deployment repo (ROLLOUT_MERGE_AFTER_GENERATIONS=64), validated on staging first.

🤖 Generated with Claude Code

beinan and others added 2 commits October 1, 2026 08:26
…slot

ROLLOUT_MERGE_AFTER_GENERATIONS merges a shard as soon as it has N
pending generations, on the 30 s flush sweeper. That merge bypassed the
per-worker merge slot (ROLLOUT_MERGE_CONCURRENCY), so turning the count
trigger on would stack sweeper merges on top of the master's requests
with no bound. It now acquires the same slot, so the bound holds no
matter who starts the merge.

Needed to enable pending-driven merging in production: hot stores get
merged when they accumulate generations, not when the master's 600 s
sweep comes around, which is what keeps reads over them fast.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review on lance-format#289: awaiting the slot before merge_if_due stalled the flush
of every store behind the first one whenever all slots were busy, even
with the count trigger off. The pass now try_acquires; when the slots
are full the self-merge is skipped (rollout_wal_self_merge_skipped_total)
and flushing continues. Regression tests: two stores flush with all
slots held and the trigger off; a store merges when a slot is free.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@beinan

beinan commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed: the pass now try_acquire_owned()s the slot and skips the self-merge when all slots are busy (counted as rollout_wal_self_merge_skipped_total), so flushing of later stores is never blocked — including with the trigger at 0. Added flush_pass_never_waits_on_a_busy_merge_slot (two stores, trigger off, slot held → both flush, nothing merges) and flush_pass_merges_when_a_slot_is_free.

@beinan
beinan merged commit 93f2455 into lance-format:main Oct 1, 2026
10 checks passed
beinan added a commit that referenced this pull request Oct 2, 2026
…292)

A stalled WAL merge can hold a master task indefinitely. Retrying after
only an HTTP timeout can overlap old and new base-table writes. This
change owns each merge independently of its HTTP connection, closes
commit admission on recovery, and fences every admitted manifest version
before handing the table to another executor. Worker fan-out remains
serial.

- **Default-off, per-table rollout:** explicit `MERGE_OWNED_TARGETS` and
`MERGE_DRAIN_TARGETS`, empty by default. Deploy capable binaries first,
drain only the selected table, then enable it. An owned request never
falls back to legacy HTTP. Drain reconciles existing owned work while
preventing new maintenance mutations on that target; other tables retain
their job pools.
- **Keep pending/timer triggers:** #289 configuration remains valid. For
enabled rollout/generic tables the worker checks its own manifest and
posts coalesced demand; the master picks it up every 15 seconds through
normal dedupe, serial ownership, and worker slot/byte limits. Unselected
tables retain direct self-merge. Scheduling latency for enabled tables
needs measurement.
- **Separate time budgets:** slot queue defaults to 600 seconds;
execution starts after slot acquisition and has a configurable
3600-second ceiling. A separate 600-second no-progress limit observes
completed batch/phase/storage work. Repeated heartbeats do not extend
it. No hard-coded 600-second execution cap remains. Timeouts cancel
executions, not pods, and handoff still requires completed writes or
proven storage fences.
- **Worker etcd connection is lazy:** outage does not fail worker
startup or disable ingestion/flush. Owned admission and further commit
authorization fail closed; already-authorized writes may finish.
- **Bound repeated failures:** durable per-target/endpoint budgets
survive task recreation and worker demand; three initial transient
attempts, then backoff, hourly probes and attention after 15 failures.
Healthy shards remain eligible; partial failures stay visible. Missing
data never triggers destructive merge repair. The read-only failure
inspection API is split into #293 (118 lines across the API, its
pagination regression, and documentation).

Validation for the final runtime changes: master **88**, worker **92**,
and shared protocol **13** tests passed, including isolated-etcd cases
for slot waiting, no-progress cancellation, HTTP disconnects, orphan
fencing, drain recovery probes, and retained retry budgets. Clippy with
warnings denied and formatting passed on the final source. Actual
delayed-commit/new-WAL/blob recovery tests passed. The compaction/merge
concurrency test now permits only Lance's typed
`RetryableCommitConflict` from index preparation, retries once after
compaction completes, and verifies the physical row count plus exact
IDs; the corrected test passed once plus **10/10** repeated runs. The
broad core run completed: **258 passed / 3 existing benchmarks
ignored**, with only the concurrency assumption described above failing;
that corrected test was then validated in the 11 runs above. CI is
running on `c0c0b5a`.

**Draft: staging soak is still required.** Legacy untracked writes
cannot be automatically fenced, and changing flags is not proof that
they drained. All replicas/helpers must follow the per-table migration
and rollback sequence in `docs/merge-recovery.md`. Progress inside
opaque Lance operations is coarse: measure realistic large-blob phase
latency, queue time, memory/OOM, etcd overhead and p95/p99 before
enabling production targets. No production deployment or
pod/configuration changes are part of this PR.

---------

Co-authored-by: Beinan Wang <>
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.

1 participant