Repository navigation
feat: publish per-shard demand watermarks on every writer flush - #345
Merged
Merged
Conversation
Second P1 step (docs/design/scheduler-p1-data-model.md §1.1, §1.3).
Writers now publish their shard watermark to
`P/demand-events/<hex>/<shard>` on every flush, beside the existing
`merge-requests` put, so a planner can see WAL demand without a sweep:
below the count threshold, on tables this master does not own, and for
low-count tails.
- core: `own_shard_watermark()` reads the writer's own MemWAL manifest
for (shard_id, writer_epoch, highest flushed generation, pending
count). Metadata only; no batches, no memory reservation.
- merge: `demand.rs` moves here so server and master share it;
`Coordinator::publish_demand_event` is a monotonic put ordered by
(writer_epoch, sealed_through, merged_through) under a mod-revision
CAS. An unchanged watermark is not rewritten. Never deletes.
- server: `route_merge` publishes before the ownership gate and after
the drain check; failure to publish never blocks the merge request.
Metric `rollout_wal_demand_events_total{result}`.
Additive. `merge-requests`, the count trigger and self-merge are
unchanged; older masters ignore the new key family.
Validation: etcd test for publish monotonicity (duplicate, lower
sealed, lower epoch, merged regress all rejected; sealed/merged/epoch
advances and separate shards accepted); server test that a flush below
threshold on a non-owned target writes exactly one event per shard,
folds to the right pending count, is not rewritten on an unchanged
flush, and advances after a new generation. All 8 sweeper tests and
6 demand tests pass against real etcd. Clippy -D warnings, fmt, typos
clean across core/merge/master/server.
Refs lance-format#333.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
17 of 18 tasks
This was referenced Oct 8, 2026
beinan
added a commit
that referenced
this pull request
Oct 8, 2026
#350) Part of #333 (P1.3). Spec §1.1/§2.2 (#343), on top of #344/#345. ## What A successful WAL catch-up writes `merged_through` per 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. - **core**: `rollout_append::merged_watermarks(uri)` — per-shard merged generation from the base table's MemWAL index. - **merge**: `Coordinator::release_with_demand(claim, execution, &[ShardMerged])`. `demand_release_changes` only advances `merged_through`, keeps the writer's `sealed`/`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. `release` is unchanged (delegates with `&[]`). - **master**: `run_as` reads watermarks after a successful `Catchup` and releases with them; read failure logs and releases without demand. Other kinds unchanged. Metric `master_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_jobs` extended: after both masters merge, each table has exactly two executor-sourced events whose `merged_through` equals `merged_watermarks(uri)` and which fold to `pending_generations == 0`. - 7 demand tests, 9 master etcd tests (resident + maintenance_execution) green; clippy `-D warnings`, fmt, typos clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
beinan
added a commit
that referenced
this pull request
Oct 8, 2026
…ath (#351) Part of #333. Fixes review finding **1 [P1]** on #345 (credit: production review). ## Problem #345 awaited the manifest read + etcd put inline in `route_merge`, *before* the merge request. A hung publish ate the flush pass timeout, skipped that table's merge request, and delayed every later table in the pass — up to 150 s/table with defaults. Telemetry was on the scheduling path, which P1 forbids. ## Fix Publish is spawned and never awaited: 5 s timeout, process-wide semaphore of 4 in-flight publishes, drop-and-count when full (the next flush republishes an absolute watermark, so nothing is lost). All outcomes on `rollout_wal_demand_events_total{result=written|unchanged|no_manifest|failed|timeout|dropped}`. `Sweepable: Clone` (all implementors are `Arc<RwLock<_>>`). ## Validation - **New** `hung_demand_publish_never_delays_flush_or_merge_request`: a store whose watermark read is `pending()` forever → flush pass returns in < 2 s, merge request still issued, 8 further passes while still hung return in < 2 s total. - `flush_publishes_shard_demand_…` adapted to poll for the detached write (10 s bound). - 9/9 sweeper tests against real 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.2). Implements spec §1.1/§1.3 (#343) on top of the fold from #344.
What
Writers publish their shard watermark to
P/demand-events/<hex>/<shard>on every flush, beside the existingmerge-requestsput. A planner can then see demand below the count threshold, on tables this master does not own, and for low-count tails — without any sweep.own_shard_watermark()reads the writer's own MemWAL manifest →(shard_id, writer_epoch, max flushed generation, pending count). Metadata only.demand.rsmoves here (shared by server and master).Coordinator::publish_demand_eventis a monotonic put ordered by(writer_epoch, sealed_through, merged_through)under a mod-revision CAS; unchanged watermarks are not rewritten; never deletes.route_mergepublishes after the drain check and before the ownership gate; publish failure never blocks the merge request. New metricrollout_wal_demand_events_total{result=written|unchanged|failed}.Additive.
merge-requests, count trigger and self-merge paths are untouched; older masters ignore the key family (review point 5).Validation
publish_is_monotonic_by_epoch_then_sealed_then_merged(etcd): duplicate / lower sealed / lower epoch / merged regress rejected; sealed, merged, epoch advances and separate shards accepted.flush_publishes_shard_demand_below_threshold_and_without_ownership(etcd): threshold 8, 2 pending, non-owned target → no merge request, exactly one event per shard, folds to the right pending count, unchanged flush does not bumpmod_revision, new generation advancessealed_throughat the same epoch.-D warnings, fmt, typos clean.🤖 Generated with Claude Code