Repository navigation
feat: publish merged watermarks in the maintenance release transaction - #350
Merged
beinan merged 1 commit intoOct 8, 2026
Merged
Conversation
Third P1 step (docs/design/scheduler-p1-data-model.md §1.1, §2.2). A successful WAL catch-up now writes `merged_through` for every shard it merged into the same etcd transaction that releases table ownership, so a planner never observes a released table whose demand still counts the generations just merged. - core: `rollout_append::merged_watermarks(uri)` reads the base table's MemWAL index for the per-shard merged generation. - merge: `Coordinator::release_with_demand` extends `release` with compare/op pairs from `demand_release_changes`, which only advances a shard's `merged_through`, keeps the writer's sealed/epoch/bytes fields, creates a fresh executor-sourced record for an unknown shard, and emits nothing for a shard already at or past the new watermark. `release` itself is unchanged and delegates with an empty list. - master: `run_as` reads watermarks after a successful Catchup outcome and releases with them. A watermark read failure logs and releases without demand rather than stranding a completed merge. Other maintenance kinds release as before. Metric `master_demand_merged_watermarks_total`. Validation: merge-crate etcd test for advance/keep/skip/fresh-shard and a fold to the expected pending count; the resident two-master merge test now asserts two executor-sourced events per table whose merged watermarks equal the base table's and fold to zero pending. 7 demand tests and 9 master etcd tests pass; clippy -D warnings, fmt, typos clean. Refs lance-format#333. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Oct 8, 2026
beinan
added a commit
that referenced
this pull request
Oct 8, 2026
…ut (#357) Part of #333. Fixes review finding **2 [P1]** (round 4) on #350 (credit: production review). Independent of #352. ## Problems in #350 1. Watermark read after the watchdog exited and before `release`, with **no deadline** → a stalled storage read holds a completed merge's ownership. 2. Release txn compared every shard's demand key by `mod_revision` → **a normal writer flush on any shard fails the whole release**. Reproduced. 3. 129+ shards → etcd default `--max-txn-ops=128` exceeded. ## Fix - `release` is the plain ownership txn again; `release_with_demand` / `demand_release_changes` removed. - After a successful `Catchup` release: `demand_publish::spawn_merged_demand` — detached, 15 s timeout, 4 slots, drop-and-count, **one monotonic put per shard**, bounded CAS retry on a lost race. Loss/lateness cannot misstate demand (per-dimension join; scan reconciles). - Both publish paths share `join_events` + retry, so the writer path no longer silently returns `Ok(false)` after losing a race either. - `master_demand_merged_watermarks_total{result}`. ## Validation - New etcd test: writer `sealed=25` and executor `merged=18` published **concurrently** on one shard → both succeed, stored `(25, Some(18))`; lower merged no-op; unknown shard → fresh epoch-0 record. - Resident two-master merge test polls for the detached publish; both shards fold to zero pending. - 7 demand, 9 maintenance/resident, 9 sweeper tests green against etcd; clippy `-D warnings`, fmt, typos clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) 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
1 participant
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.
Part of #333 (P1.3). Spec §1.1/§2.2 (#343), on top of #344/#345.
What
A successful WAL catch-up writes
merged_throughper shard in the same etcd txn that releases ownership, so demand and ownership change together and a planner never sees a released table whose pending count still includes the generations just merged.rollout_append::merged_watermarks(uri)— per-shard merged generation from the base table's MemWAL index.Coordinator::release_with_demand(claim, execution, &[ShardMerged]).demand_release_changesonly advancesmerged_through, keeps the writer'ssealed/epoch/bytes, creates a fresh executor-sourced record for an unknown shard, and emits no compare/op for a shard already at or past the watermark.releaseis unchanged (delegates with&[]).run_asreads watermarks after a successfulCatchupand releases with them; read failure logs and releases without demand. Other kinds unchanged. Metricmaster_demand_merged_watermarks_total.Additive; no key family removed; old masters ignore
demand-events/.Validation
release_changes_only_advance_merged_and_keep_writer_fields(etcd): advance 15 keeps sealed 20 / epoch 3 / bytes; lower 12 emits nothing; unknown shard →sealed == merged; fold → expected pending.resident_replicas_merge_from_queue_with_no_workers_or_jobsextended: after both masters merge, each table has exactly two executor-sourced events whosemerged_throughequalsmerged_watermarks(uri)and which fold topending_generations == 0.-D warnings, fmt, typos clean.🤖 Generated with Claude Code