From a6ba9ddc6dad437fff02b987bba46d36c6f3e2ae Mon Sep 17 00:00:00 2001 From: Migorithm Date: Sat, 18 Jul 2026 14:19:40 +0400 Subject: [PATCH 01/14] plan doc --- .../event-driven-range-load.md | 274 ++++++++++++++++++ .../metadata_management_roadmap.md | 2 + 2 files changed, 276 insertions(+) create mode 100644 docs/metadata-management/event-driven-range-load.md diff --git a/docs/metadata-management/event-driven-range-load.md b/docs/metadata-management/event-driven-range-load.md new file mode 100644 index 00000000..221c3613 --- /dev/null +++ b/docs/metadata-management/event-driven-range-load.md @@ -0,0 +1,274 @@ +# Event-Driven Range Load Classification + +**Goal:** Make automatic range split and merge decisions a deterministic result of committed segment-roll causes, while preserving physical seal time for retention and operational visibility. + +**Depends on:** segment lifecycle coordination, Raft-backed metadata commands, automatic range split/merge, and segment retention. + +**Status:** Proposed by GitHub issue #192. The current implementation still classifies load from a wall-clock seal history; this document describes its replacement. + +--- + +## Why the current signal is insufficient + +EastGuard already records every segment roll through Raft. The original range-load policy reused those rolls by storing their proposal-time Unix timestamps: + +- several recent seals make a range eligible to split; +- no recent seals make adjacent ranges eligible to merge; +- a split timestamp prevents the children from immediately changing topology again. + +This is attractive because it needs no metrics pipeline, but it asks physical time to answer two different questions: + +1. **When was this segment sealed?** Physical time is meaningful for retention and operations. +2. **What does this roll say about range load?** This is an ordered policy signal. + +Raft commit order is authoritative for the second question. A proposal captures its timestamp before commit, so two proposals may commit in a different order from their timestamps. Sorting the timestamps keeps the history ordered, but the decision still depends on wall time and still treats every roll as evidence of load—even when a roll was caused by failure or recovery. + +The replacement keeps the useful “segment rolls are free load observations” idea, but records why each roll happened and applies that cause in commit order. + +--- + +## Range seals and segment seals are different operations + +The word “seal” appears at two levels. Conflating them would make topology changes look like traffic. + +### Segment roll + +A segment roll seals the current active segment, creates its successor, and leaves the owning range active. It can reveal something about the range's workload: + +- the segment filled to its size limit; +- the segment reached its age limit without filling; +- replication failed; +- recovery required a new active segment. + +Only a successful roll of the current active segment may update range-load state. + +### Range seal + +A split or merge seals a range itself. The operation also seals the range's active segment, removes its write head, and records lineage. This is a topology transition, not a segment roll and not a load observation. + +After a parent range is split, its data leader may still need to report the exact end of the segment that the range seal closed. That message travels through existing segment lifecycle coordination, but metadata applies it only as a sealed-segment boundary correction. It creates no successor and contributes no load signal. + +```text +active range sealed range + | | + | segment roll | boundary correction + v v +seal active segment fill exact segment end +create successor no successor +range stays active range stays sealed +may update load state never updates load state +``` + +This boundary is structural: split and merge remain their own Raft commands. There is no “topology” segment-roll cause. + +--- + +## Roll causes + +Every real roll of an active segment carries an explicit cause from its origin through Raft commit. + +| Cause | Observation | Effect on range load | +|---|---|---| +| **Data pressure** | The segment reached its configured size limit | Advance consecutive pressure | +| **Idle maintenance** | The segment reached its configured age limit before filling | Mark idle and clear pressure | +| **Replication failure** | The write path could not replicate to the current set | No change | +| **Recovery** | Membership loss or boundary recovery required a new active segment | No change | + +“Idle” does not mean that the range received zero writes. It means the active segment survived for the configured age without reaching its size limit: the range is cold enough to be considered for consolidation under this policy. + +Failure and recovery are deliberately neutral. They describe cluster health, not demand. Allowing them to advance pressure would split ranges during outages; allowing them to mark idle would merge ranges because replicas failed. + +### Retries preserve intent + +A retry is another delivery attempt for the same logical roll, not a new reason to roll. + +```text +size limit reached + | + v +request(Data pressure) -- lost --> retry(Data pressure) --> Raft commit + pressure + 1 +``` + +The pending request therefore retains its original cause. If a size-triggered request is dropped, its eventual retry still counts as pressure. Network loss and retry timing cannot change classification. + +Only the first command that successfully rolls the active segment applies the signal. A later duplicate finds that segment already sealed and becomes an idempotent no-op or, where applicable, a boundary correction; it cannot count the cause twice. + +--- + +## Committed range-load state + +Each active range stores one compact classification: + +- **Unclassified** — no usable load observation since the range was created. +- **Idle** — the most recent classifying observation was an age-limit roll. +- **Pressure(N)** — `N` consecutive size-limit rolls have committed since the last age-limit roll. + +The transition function is: + +```text +Unclassified + | age-limit roll + v + Idle <------------------------- age-limit roll + | | + | size-limit roll | + v | +Pressure(1) --size--> Pressure(2) --size--> Pressure(threshold) + +replication failure / recovery: remain in the current state +range split / merge / boundary correction: do not enter this state machine +``` + +Pressure is consecutive rather than windowed. An age-limit roll breaks the streak because the active range has demonstrated a full low-pressure interval. Failure and recovery leave the previous observation intact because neither says whether demand increased or decreased. + +The pressure count is capped at the split threshold. A split proposal can be delayed, rejected as stale, or temporarily impossible, so further size rolls must not make bookkeeping grow or overflow. + +--- + +## Automatic split + +An active range becomes split-eligible when: + +1. its committed load state reaches the pressure threshold; and +2. the topic's partition strategy permits splitting. + +The leader proposes the split through Raft. The load transition itself happens during deterministic metadata apply, so every replica sees the same state after the same committed log. Only the leader originates the follow-up proposal. + +```text +three committed size-limit rolls + | + v + Pressure(threshold) + | + v + leader proposes split + | + v + Raft commits split +``` + +Existing apply-time checks continue to protect against stale proposals. If the range was already sealed or otherwise changed before the split commits, the proposal cannot corrupt current topology. + +--- + +## Automated merge + +The existing leader-side merge timer remains, but its role becomes smaller: it prompts a scan of committed metadata. It does not calculate idleness and its wall-clock value does not affect eligibility. + +Two adjacent active ranges are merge-eligible only when both are explicitly idle: + +| Left | Right | Merge eligible? | +|---|---|---| +| Unclassified | Idle | No | +| Idle | Pressure | No | +| Pressure | Pressure | No | +| Idle | Idle | Yes | + +When an eligible pair is found, the leader proposes the merge through Raft. Existing adjacency, active-state, and partition-strategy checks remain authoritative when the proposal applies. + +### Why split children start unclassified + +Both children created by a split begin unclassified, not idle. An empty history is absence of evidence; it is not evidence that a child is cold. + +This prevents an immediate split-merge loop: + +```text +hot parent splits + | + +--> left child: Unclassified --age roll--> Idle + | + +--> right child: Unclassified --age roll--> Idle + | + v + merge may become eligible +``` + +Each child must independently survive an age interval without filling before the pair can merge. The explicit idle gate replaces the old wall-clock split cooldown for merge protection. A merged range also starts unclassified because its combined workload has not yet been observed. + +--- + +## Physical time remains for retention + +Removing wall time from load classification does not remove segment seal timestamps. + +Every sealed segment keeps its physical `sealed_at` value. Retention continues to expire an oldest-first prefix when: + +```text +sealed_at + retention period < current time +``` + +Split and merge timestamps also continue to describe when their segments and ranges were created or sealed. The separation is about use, not collection: + +| Metadata | Ordering authority | Purpose | +|---|---|---| +| Segment seal time | Physical clock | Retention and operational visibility | +| Range-load state | Raft commit order | Automatic split and merge policy | + +No load-classification decision reads seal time, creation time, current time, or retry time. + +--- + +## Failure and edge cases + +### Size and age become eligible together + +The data plane assigns the cause at the source. If the segment is already at or above its size limit, data pressure wins; otherwise an age-triggered roll carries idle maintenance. Request deduplication preserves the first logical request that was emitted. + +### A request is retried + +The retry copies the original cause. Classification therefore depends on why the operation began, not how many packets were lost before it committed. + +### Failure rolls an active segment + +The successor is created normally, but the range keeps its previous classification. Failure is not demand. + +### A range seal needs its exact segment end later + +The report is explicitly identified as boundary correction rather than an active segment roll. It can update the already-sealed segment's end and restore offset continuity, but it cannot create a successor or touch range-load state. + +### Leadership changes + +No load history must be reconstructed. Classification is committed metadata, identical on every replica. The new leader can resume split and merge evaluation from the state it applied through Raft. + +--- + +## Design rules + +1. **Only a committed roll of the current active segment may classify range load.** This prevents retries, stale commands, range seals, and boundary correction from counting the same physical transition as new demand. + +2. **Roll cause survives every delivery attempt.** Otherwise packet loss could turn pressure into a neutral event—or turn retry timing into a false load signal. + +3. **Range topology operations never masquerade as segment load observations.** Split and merge seal ranges for lineage and routing; treating those seals as load would create self-reinforcing topology churn. + +4. **Both merge inputs must provide positive idle evidence.** Unclassified means unknown, not cold. Requiring two committed age-limit rolls prevents newly split children from merging immediately. + +5. **Physical time is retained but policy-neutral.** Retention needs wall time; deterministic split and merge decisions need committed event order. Keeping those responsibilities separate avoids weakening either one. + +--- + +## Implementation and verification outline + +Implementation crosses four conceptual boundaries: + +1. Carry the cause from size, age, failure, and recovery triggers through request retries and the committed segment roll. +2. Distinguish active-segment rolls from post-range-seal boundary corrections in lifecycle messages and metadata apply. +3. Replace timestamp history and split cooldown state with the committed unclassified/idle/pressure classification. +4. Make split and merge eligibility read only that committed classification while leaving retention untouched. + +Coverage must demonstrate: + +- pressure streaks split exactly at the configured threshold; +- an age-limit roll resets pressure and marks the range idle; +- failure and recovery rolls do not change classification; +- reversed physical timestamps cannot change results determined by commit order; +- retries preserve their original cause and duplicate commits cannot count twice; +- split children and merged ranges start unclassified; +- two explicit idle children can merge, while either child being unclassified or pressured blocks merge; +- a range seal and its later boundary correction never update classification; +- segment retention remains based on physical seal time; +- repeated deterministic turmoil scenarios cover size, age, failure, recovery, retry, split, merge, and boundary-correction paths. + +The persisted metadata and Raft command formats are serialized. Before implementation, compatibility expectations for existing stores must be decided: migrate old state to unclassified, or explicitly require a fresh store. Historical timestamps cannot reliably be converted into causes. + +For the broader architecture, see [Metadata Management — Mental Model](mental-model.md). For the phase history, see [Metadata Management Roadmap](metadata_management_roadmap.md). Operational contracts live in `.agents/rules/metadata-state-machine.md`, `.agents/rules/raft-actor.md`, and `.agents/rules/ds-rsm.md`. diff --git a/docs/metadata-management/metadata_management_roadmap.md b/docs/metadata-management/metadata_management_roadmap.md index 886c98bb..ce2cb9a7 100644 --- a/docs/metadata-management/metadata_management_roadmap.md +++ b/docs/metadata-management/metadata_management_roadmap.md @@ -100,6 +100,8 @@ Commit-completion tracking lives in the consensus actor: when a proposal is acce **Decision logic.** Each range tracks its recent seal timestamps within a sliding window. When the seal count exceeds the split threshold (and the range isn't in a cooldown window from a recent split), the leader proposes a split at the keyspace midpoint. Adjacent buddy ranges — children of the same parent split — with no recent seals can merge back together. Hysteresis between split and merge thresholds prevents oscillation. +**Planned replacement.** Issue #192 replaces proposal-time timestamp history with committed, event-driven roll causes. Size-limit rolls advance pressure, age-limit rolls provide explicit idle evidence, and failure/recovery rolls remain neutral. Range-level split/merge seals stay outside load classification. See [Event-Driven Range Load Classification](event-driven-range-load.md). + **Anti-recursion.** The apply path never proposes. Auto-proposals are buffered during apply and drained at the start of the next flush cycle, so a split apply can't recursively trigger another split. Stale auto-proposals (e.g., a merge proposed but the range was already split) are rejected by apply-time precondition checks. Midpoint splitting is good enough for now. Percentile-based split points are in the backlog. From d88789e41238ee058651450ea56b4fdaac104e1c Mon Sep 17 00:00:00 2001 From: Migorithm Date: Sat, 18 Jul 2026 16:17:58 +0400 Subject: [PATCH 02/14] docs --- .claude/rules/metadata-state-machine.md | 4 ++-- docs/data-plane/d3_segment_roll_integration.md | 10 +++++----- docs/metadata-management/event-driven-range-load.md | 6 +++--- .../metadata-management/metadata_management_roadmap.md | 10 ++++------ 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/.claude/rules/metadata-state-machine.md b/.claude/rules/metadata-state-machine.md index e25d338e..c34e528e 100644 --- a/.claude/rules/metadata-state-machine.md +++ b/.claude/rules/metadata-state-machine.md @@ -67,9 +67,9 @@ MetadataStateMachine (one per shard group) 15. **Merges require adjacency.** Only ranges with contiguous keyspaces (`r1.keyspace_end == r2.keyspace_start` or vice versa) can merge. Otherwise the merged range's keyspace would not be a contiguous slice. -16. **Seal history is deterministic.** `seal_history` on `RangeMeta` is updated only inside `apply_roll_segment()`, which runs on ALL replicas with the same input. No leader-only state leaks into `RangeSealHistory`. The split/merge *decision* is leader-only, but the data the decision reads is identical on every replica. +16. **Range load classification is deterministic.** `load_state` on `RangeMeta` changes only when `apply_roll_segment()` successfully rolls the current active segment, which runs on ALL replicas with the same committed `SegmentRollIntent`. Data-pressure rolls advance the capped consecutive-pressure streak; idle-maintenance rolls reset it to Idle; replication-failure and recovery rolls leave it unchanged. Boundary correction, idempotent replay, and range-level split/merge seals never classify load. The split/merge *decision* is leader-only, but the data the decision reads is identical on every replica. -17. **Children of a split carry a cooldown anchor.** Child ranges created by `apply_split_range()` always have `seal_history.created_by_split_at` set to the split's `created_at` timestamp. Enforces `SPLIT_COOLDOWN_MS` before children can be re-split — prevents split storms from transient load. +17. **Pressure streaks are bounded and nonzero.** `RangeLoadState::Pressure.consecutive_rolls` is always in `1..=SPLIT_SEAL_THRESHOLD`. The pressure counter stops growing once it hits the split limit, preventing it from increasing endlessly if a split is delayed. Newly created ranges (from splits or merges) start as 'Unclassified'. We don't enforce this starting state as a strict invariant, since normal segment rolls will soon reclassify them anyway. 18. **`correct_end_offset` is write-once.** Updates the sealed segment's `end_offset` only when the current value is `None` AND the incoming `end_entry_id` is `Some`. Simultaneously sets the active segment's `start_offset` to `end_entry_id + 1`. Re-application is a no-op. Restores invariant 8 after a death-triggered roll. This is now the **fallback** correction — for the follower-death race (the live leader's write-path seal arrives after a reconcile `None`-roll) and the unknown-end fallback. The leader-crash path no longer relies on it: it recovers the committed end *before* rolling (`raft-actor.md` #6), so the successor opens at `min+1` directly rather than at 0-then-corrected. diff --git a/docs/data-plane/d3_segment_roll_integration.md b/docs/data-plane/d3_segment_roll_integration.md index d66be975..d0f07659 100644 --- a/docs/data-plane/d3_segment_roll_integration.md +++ b/docs/data-plane/d3_segment_roll_integration.md @@ -124,9 +124,9 @@ Segment Leader (node D) Coordinator (node A) Raft [A, B, C] The flow moves three pieces of information, each scoped to where it is needed: -- **Seal request** (segment leader → coordinator): the requester's identity, which segment to seal, which nodes failed (empty for size/age seals), and the last committed entry id. The requester identity is carried **explicitly** in the request — it is not inferred from the transport connection. Node-death-triggered rolls do not use a seal request at all and therefore have no requester. +- **Seal request** (segment leader → coordinator): the requester's identity, which segment to seal, which nodes failed (empty for size/age seals), the last committed entry id, and the roll intent. The intent distinguishes size pressure, idle maintenance, replication failure, and post-range-seal boundary correction. The requester identity is carried **explicitly** in the request — it is not inferred from the transport connection. Node-death-triggered recovery rolls do not use a seal request at all and therefore have no requester. -- **Roll command** (proposed through Raft): which segment, the seal timestamp, the new replica set, and an *optional* end entry id. The end entry id is present for segment-leader-initiated seals (it came from the seal request) and absent for node-death seals, where the coordinator does not know the actual committed offset. +- **Roll command** (proposed through Raft): which segment, the seal timestamp, the new replica set, an *optional* end entry id, and the intent. The end entry id is present for segment-leader-initiated seals (it came from the seal request) and absent when recovery does not yet know the committed boundary. Only a successful roll of the active segment applies a load signal; boundary correction fills an already-sealed segment's end without creating a successor. - **Pending context** (held by the coordinator between propose and commit): the requester and the old segment identity, keyed by the proposal's log position. The coordinator also keeps a dedup set of in-flight seal keys so duplicate seal requests for the same segment are skipped. @@ -162,10 +162,10 @@ On an inbound seal request from the data port, the data plane converts it to a c ### Coordinator: Seal-Request Handling ``` -on a seal request (requester, segment, failed nodes, end entry id): +on a seal request (requester, segment, failed nodes, end entry id, intent): if this segment already has an in-flight seal → skip (dedup) compute new replica set = current replica set − failed nodes + healthy nodes - build the roll (segment, seal timestamp, new replica set, end entry id) + build the roll (segment, seal timestamp, new replica set, end entry id, intent) propose through Raft: accepted → record the in-flight seal key + pending context rejected (not leader) → no reply; the requester retries on timeout @@ -175,7 +175,7 @@ on a seal request (requester, segment, failed nodes, end entry id): **Dispatch:** on a replication timeout, after a flush size check, and on the periodic age ticker, the data plane resolves the coordinator from the local topology view and sends a seal request to it over the data port. -**Timeout (~5s):** when a seal request is sent, the data plane records it. If no seal response arrives before the timeout, it refreshes the coordinator from the topology and retries. For size- and age-triggered seals this is the only recovery path — the replication timeout won't fire, since replication is healthy. +**Timeout (~5s):** when a seal request is sent, the data plane records its failed nodes and intent. If no seal response arrives before the timeout, it refreshes the coordinator from the topology and retries the same logical request with the same intent. For size- and age-triggered seals this is the only recovery path — the replication timeout won't fire, since replication is healthy. ### Applying a Roll diff --git a/docs/metadata-management/event-driven-range-load.md b/docs/metadata-management/event-driven-range-load.md index 221c3613..6db93071 100644 --- a/docs/metadata-management/event-driven-range-load.md +++ b/docs/metadata-management/event-driven-range-load.md @@ -4,7 +4,7 @@ **Depends on:** segment lifecycle coordination, Raft-backed metadata commands, automatic range split/merge, and segment retention. -**Status:** Proposed by GitHub issue #192. The current implementation still classifies load from a wall-clock seal history; this document describes its replacement. +**Status:** Implemented for GitHub issue #192. --- @@ -154,7 +154,7 @@ Existing apply-time checks continue to protect against stale proposals. If the r ## Automated merge -The existing leader-side merge timer remains, but its role becomes smaller: it prompts a scan of committed metadata. It does not calculate idleness and its wall-clock value does not affect eligibility. +Applying an idle signal immediately checks the newly committed metadata and queues a merge when both adjacent ranges are idle. The existing leader-side merge timer remains as a recovery scan, but it does not calculate idleness and its wall-clock value does not affect eligibility. Two adjacent active ranges are merge-eligible only when both are explicitly idle: @@ -269,6 +269,6 @@ Coverage must demonstrate: - segment retention remains based on physical seal time; - repeated deterministic turmoil scenarios cover size, age, failure, recovery, retry, split, merge, and boundary-correction paths. -The persisted metadata and Raft command formats are serialized. Before implementation, compatibility expectations for existing stores must be decided: migrate old state to unclassified, or explicitly require a fresh store. Historical timestamps cannot reliably be converted into causes. +This change is intentionally format-breaking for persisted metadata and Raft commands. Existing stores must be recreated; there is no timestamp-to-cause migration because historical timestamps cannot reliably distinguish pressure, maintenance, failure, or recovery. For the broader architecture, see [Metadata Management — Mental Model](mental-model.md). For the phase history, see [Metadata Management Roadmap](metadata_management_roadmap.md). Operational contracts live in `.agents/rules/metadata-state-machine.md`, `.agents/rules/raft-actor.md`, and `.agents/rules/ds-rsm.md`. diff --git a/docs/metadata-management/metadata_management_roadmap.md b/docs/metadata-management/metadata_management_roadmap.md index ce2cb9a7..598d26ee 100644 --- a/docs/metadata-management/metadata_management_roadmap.md +++ b/docs/metadata-management/metadata_management_roadmap.md @@ -96,11 +96,9 @@ Commit-completion tracking lives in the consensus actor: when a proposal is acce **Goal.** The system detects ranges with disproportionate load and rebalances them through splits and merges, without external metrics infrastructure. -**Core insight.** Segment seals are already in the Raft log. A range whose segments seal frequently is a hot range. Counting seal frequency gives a load signal for free — no probe protocol, no data plane → metadata reporting pipeline, no key histograms. +**Core insight.** Segment rolls are already in the Raft log. Carrying their cause turns existing lifecycle traffic into an ordered load signal: size means pressure, age means idle, and failure/recovery say nothing about demand. No metrics pipeline, probe protocol, or key histogram is required. -**Decision logic.** Each range tracks its recent seal timestamps within a sliding window. When the seal count exceeds the split threshold (and the range isn't in a cooldown window from a recent split), the leader proposes a split at the keyspace midpoint. Adjacent buddy ranges — children of the same parent split — with no recent seals can merge back together. Hysteresis between split and merge thresholds prevents oscillation. - -**Planned replacement.** Issue #192 replaces proposal-time timestamp history with committed, event-driven roll causes. Size-limit rolls advance pressure, age-limit rolls provide explicit idle evidence, and failure/recovery rolls remain neutral. Range-level split/merge seals stay outside load classification. See [Event-Driven Range Load Classification](event-driven-range-load.md). +**Decision logic.** Each committed active-segment roll carries its cause. Size-limit rolls advance a consecutive pressure streak; age-limit rolls reset pressure and mark the range idle; failure and recovery rolls are neutral. At the pressure threshold the leader proposes a split at the keyspace midpoint. Committing the second idle signal for adjacent ranges queues a merge immediately; a periodic scan remains as a recovery fallback. New split children and merged ranges start unclassified, so absence of observations cannot trigger a merge. Range-level split/merge seals and their boundary-correction plumbing stay outside load classification. See [Event-Driven Range Load Classification](event-driven-range-load.md). **Anti-recursion.** The apply path never proposes. Auto-proposals are buffered during apply and drained at the start of the next flush cycle, so a split apply can't recursively trigger another split. Stale auto-proposals (e.g., a merge proposed but the range was already split) are rejected by apply-time precondition checks. @@ -182,7 +180,7 @@ The two layers should share one replacement-selection policy. Otherwise the same 7. **Segment offsets enable consumer position tracking.** Each sealed segment knows where it ended; consumers know they are done with a range when their position reaches the last segment's end. On split or merge, children start at offset 0 — a clean break from the parent's offset space. -8. **Seal frequency is the hot-range signal.** Already in the log, costs nothing additional, accurate enough for midpoint splitting. +8. **Committed segment-roll causes are the range-load signal.** Size and age rolls already pass through the log, so explicit causes distinguish pressure from idle without a metrics pipeline; failure and recovery remain policy-neutral. 9. **Membership changes go through the log, never via direct mutation.** Detailed in Phase 3d and `mental-model.md`. @@ -196,7 +194,7 @@ The two layers should share one replacement-selection policy. Otherwise the same | Hash ring divergence between client and server | Server resolves routing from SWIM (fast convergence). Leader forwarding handles misrouted proposals. | | Log entry format changes on payload variants | A version byte on log entries when bincode-incompatible changes are needed. | | Midpoint split does not balance skewed workloads | Acceptable for now; percentile-based split is in the backlog. | -| Seal tracker grows unbounded on long-lived ranges | Sliding window keeps it bounded; cleaned up on range seal or delete. | +| Pressure streak grows unbounded on long-lived ranges | The committed streak saturates at the split threshold. | | Two leaders coexisting briefly with divergent peer sets | Membership goes through the Raft log — quorum size advances deterministically with the log index. Split-brain commit is structurally impossible. See Phase 3d. | | New leader inherits dead peers because the predecessor died before converting the event | Reference-back step on becoming leader (Phase 3d; detailed in `mental-model.md`). | | Reconciliation shrinks the group's failure budget | Pair every removal with an addition — at both the Raft-peer and segment-replica-set layers. Not yet built. | From 90b7bf616202654554dd5018945dcd9ec0ab2e69 Mon Sep 17 00:00:00 2001 From: Migorithm Date: Sat, 18 Jul 2026 16:18:54 +0400 Subject: [PATCH 03/14] pressure results in split / idle results in merge --- src/it/e2e/client_protocol.rs | 128 ++++++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) diff --git a/src/it/e2e/client_protocol.rs b/src/it/e2e/client_protocol.rs index b9b6b790..aea22ede 100644 --- a/src/it/e2e/client_protocol.rs +++ b/src/it/e2e/client_protocol.rs @@ -582,6 +582,108 @@ fn age_seal_rolls_the_active_segment() -> turmoil::Result { sim.run() } +/// Three committed size-driven rolls classify the range as pressured and cause +/// the metadata leader to split it through Raft. The simulation pins every +/// placement/scheduling input and disables age interference, so only data +/// pressure can produce the split signal. +#[test] +#[serial_test::serial] +fn pressure_rolls_automatically_split_the_range() -> turmoil::Result { + let mut sim = Builder::new() + .tick_duration(Duration::from_millis(100)) + .simulation_duration(Duration::from_secs(240)) + .tcp_capacity(4096) + .rng_seed(192) + .build(); + + let _ = tracing_subscriber::fmt() + .with_env_filter(tracing_subscriber::EnvFilter::from_default_env()) + .try_init(); + + host_cluster(&mut sim, &NODES_3, |env| { + env.node_id_suffix = Some("sim".to_string()); + env.vnodes_per_node = 16; + env.segment_size_limit_bytes = 2048; + env.max_segment_age_secs = 3600; + }); + + sim.client("test-client", async { + const NODES: [(&str, u16); 3] = [("node-1", 8081), ("node-2", 8082), ("node-3", 8083)]; + const TOPIC: &str = "pressure-auto-split"; + + wait_for_cluster(&NODES, 3).await; + create_topic_anywhere(TOPIC, &NODES, 3).await; + produce_until_topic_splits(TOPIC, &NODES).await; + + Ok(()) + }); + + sim.run() +} + +/// After the pressure-driven split, both children independently age-roll and +/// become idle. Applying the second idle signal queues and commits a `MergeRange`; +/// the periodic scan remains only a recovery fallback. Age zero avoids `std::time` +/// dependence, while the first age check is delayed until after the pressure rolls. +#[test] +#[serial_test::serial] +fn idle_split_children_automatically_merge() -> turmoil::Result { + let mut sim = Builder::new() + .tick_duration(Duration::from_millis(100)) + .simulation_duration(Duration::from_secs(420)) + .tcp_capacity(4096) + .rng_seed(193) + .build(); + + let _ = tracing_subscriber::fmt() + .with_env_filter(tracing_subscriber::EnvFilter::from_default_env()) + .try_init(); + + host_cluster(&mut sim, &NODES_3, |env| { + env.node_id_suffix = Some("sim".to_string()); + env.vnodes_per_node = 16; + env.segment_size_limit_bytes = 2048; + env.max_segment_age_secs = 0; + env.segment_age_check_interval_secs = 300; + }); + + sim.client("test-client", async { + const NODES: [(&str, u16); 3] = [("node-1", 8081), ("node-2", 8082), ("node-3", 8083)]; + const TOPIC: &str = "idle-auto-merge"; + + wait_for_cluster(&NODES, 3).await; + create_topic_anywhere(TOPIC, &NODES, 3).await; + produce_until_topic_splits(TOPIC, &NODES).await; + + // Pressure production completes well before the first age check. Cross + // that single virtual deadline only after observing the split, then poll + // briefly for the event-driven merge commit. + tokio::time::sleep(Duration::from_secs(310)).await; + for _ in 0..30 { + tokio::time::sleep(Duration::from_secs(1)).await; + let Some(detail) = describe_topic(TOPIC, &NODES).await else { + continue; + }; + let active_count = detail + .ranges + .iter() + .filter(|range| range.state == RangeState::Active) + .count(); + let merged_range_is_active = detail + .ranges + .iter() + .any(|range| range.state == RangeState::Active && range.merged_from.is_some()); + if active_count == 1 && merged_range_is_active { + return Ok(()); + } + } + + panic!("idle split children never merged through Raft"); + }); + + sim.run() +} + /// End-to-end sealed-segment repair. Produce + age-seal a segment, kill one of its /// replicas, and confirm the coordinator reassigns the segment to the spare node, /// which catches up and serves the old data on a cold fetch. Exercises the whole @@ -1646,6 +1748,32 @@ async fn produce_until_acked<'a>( None } +async fn produce_until_topic_splits(topic: &str, nodes: &[(&str, u16)]) { + for record in 0..16 { + let mut payload = format!("pressure-{record}").into_bytes(); + payload.resize(1024, b'-'); + produce_until_acked(topic, &payload, nodes) + .await + .unwrap_or_else(|| panic!("pressure record {record} never acked")); + + for _ in 0..10 { + if let Some(detail) = describe_topic(topic, nodes).await + && detail + .ranges + .iter() + .filter(|range| range.state == RangeState::Active) + .count() + == 2 + { + return; + } + tokio::time::sleep(Duration::from_millis(200)).await; + } + } + + panic!("three committed pressure rolls never split topic {topic}"); +} + enum ProduceOutcome { Acked, /// Redirect to the write leader at this address. From 7cf8fb961c1e07844badf9197bde65f803e44c4a Mon Sep 17 00:00:00 2001 From: Migorithm Date: Sat, 18 Jul 2026 16:38:06 +0400 Subject: [PATCH 04/14] terminal state oriented refactoring --- .claude/rules/raft-actor.md | 6 +- src/config.rs | 12 +- src/control_plane/consensus/actor.rs | 2 +- ...{seal_recovery.rs => boundary_recovery.rs} | 86 +++--- .../consensus/messages/command.rs | 7 +- src/control_plane/consensus/messages/event.rs | 10 +- src/control_plane/consensus/mod.rs | 2 +- src/control_plane/consensus/multi_raft.rs | 177 ++++++------ .../consensus/raft/states/metadata_state.rs | 252 +++++++++++------- src/control_plane/metadata/event.rs | 12 +- src/control_plane/metadata/topic.rs | 24 +- src/data_plane/messages/command.rs | 2 + src/data_plane/states/mod.rs | 2 +- src/data_plane/states/seal_request.rs | 107 -------- src/data_plane/states/segment_store.rs | 2 +- src/it/helpers.rs | 2 +- 16 files changed, 331 insertions(+), 374 deletions(-) rename src/control_plane/consensus/{seal_recovery.rs => boundary_recovery.rs} (84%) delete mode 100644 src/data_plane/states/seal_request.rs diff --git a/.claude/rules/raft-actor.md b/.claude/rules/raft-actor.md index 6a42f14a..795c1e1a 100644 --- a/.claude/rules/raft-actor.md +++ b/.claude/rules/raft-actor.md @@ -130,9 +130,9 @@ RaftTransportActor (TCP I/O) 2. **On `LeaderChange → self`, the new leader reconciles against SWIM.** Actor snapshots SWIM's live member set and proposes `RemovePeer` for every current peer not in that set. Without this, SWIM facts about deaths that occurred while no leader existed (typically: the previous leader was one of the dead) never become Raft truth — the events have already been disseminated by SWIM and won't re-fire. -3. **`MetadataCommitted` is dispatched only by the current leader.** During event drain, if the replica is no longer leader, its `MetadataCommitted` events are discarded (the entry still applies — only the outbound dispatch is suppressed). Otherwise every replica would emit duplicate `SegmentAssignment` / `SealResponse` to the data plane, and a former leader would dispatch entries it can no longer be authoritative about. +3. **`MetadataCommitted` is dispatched only by the current leader.** During event drain, if the replica is no longer leader, its `MetadataCommitted` events are discarded (the entry still applies — only the outbound dispatch is suppressed). Otherwise every replica would emit duplicate `SegmentAssignment` / `SegmentRollCommitted` to the data plane, and a former leader would dispatch entries it can no longer be authoritative about. -4. **`pending_seals` is bounded.** Cleared in two places: (a) on observed `LogMutation::TruncateFrom(idx)`, entries with `log_index ≥ idx` are dropped — the new leader may have truncated those proposals; (b) when `!raft.is_leader()` at drain time, all entries for that group are dropped — the `SealResponse` will not come from this replica. Without this, unbounded growth as leaders churn. +4. **`pending_rolls` is bounded.** Cleared in two places: (a) on observed `LogMutation::TruncateFrom(idx)`, entries with `log_index ≥ idx` are dropped — the new leader may have truncated those proposals; (b) when `!raft.is_leader()` at drain time, all entries for that group are dropped — the `SegmentRollCommitted` will not come from this replica. Without this, unbounded growth as leaders churn. 5. **`flush` terminates within a bounded number of rounds.** `MultiRaftActor::flush` loops `store.flush()` up to `MAX_FLUSH_ROUNDS = 8` because `route_event` may call back into the store (reconciliation proposes new `RemovePeer` entries). Bounded so a feedback bug cannot deadlock the actor. Steady-state shape is ≤ 2 rounds. @@ -146,7 +146,7 @@ RaftTransportActor (TCP I/O) 7. **The recovered segment is led by the most-complete survivor.** The new `replica_set[0]` is the survivor with the highest durable extent (`argmax`, ties broken by lowest `NodeId`) — a clean "most-caught-up replica wins" election, read from the same gather. The choice is the leader's and is committed via `RollSegment.new_replica_set` ordering, so apply stays deterministic (a leader-only *decision*, identical *apply* on every replica — cf. `metadata-state-machine.md` #16). -8. **Boundary recovery reads durable extents, never the notified commit cursor.** A `SealBoundaryReport` carries `next_entry_id - 1` (`SegmentTracker::durable_end_entry_id`), advanced only after `wal.flush_batch()` succeeds — distinct from and `≥` the *notified* `committed_entry_id`. A survivor's notified commit lags its durable extent (the leader acks the producer before propagating the commit notification), so trusting the cursor could drop an already-acked entry. `None` means the survivor holds nothing ⇒ nothing was committed ⇒ the segment seals empty. +8. **Boundary recovery reads durable extents, never the notified commit cursor.** A `DurableSegmentEndReported` carries `next_entry_id - 1` (`SegmentTracker::durable_end_entry_id`), advanced only after `wal.flush_batch()` succeeds — distinct from and `≥` the *notified* `committed_entry_id`. A survivor's notified commit lags its durable extent (the leader acks the producer before propagating the commit notification), so trusting the cursor could drop an already-acked entry. `None` means the survivor holds nothing ⇒ nothing was committed ⇒ the segment seals empty. 9. **Sealed-segment catch-up is re-driven until confirmed.** The `CatchUpAssignment` a committed `ReassignSegment` dispatches (via the #3 drain) is one-shot and fire-and-forget. So on applying the reassignment the leader seeds a per-segment in-repair entry (the `CatchUpRepairs` tracker, leader-only — gated on role at apply like `peer_states` in `apply_add_peer`) and re-drives `CatchUpAssignment` to every not-yet-confirmed member on each heartbeat sweep (`maybe_redrive_catch_ups`, beside the active-segment `maybe_redrive_segment_assignments`). A member clears itself with a `CatchUpAck` once it holds the segment through `sealed_end`; the entry is pruned when all members confirm, superseded when the segment is re-reassigned, and dropped on step-down (leader-volatile, cleared with `confirmed_assignment`). `CatchUpAck` is idempotent and a full-match member re-confirms on re-drive (`data_plane/state.rs`), so no single dropped message strands a repair. A re-drive also rescues an *in-flight* receive that has stalled (dead source or dropped chunk): the receiver re-requests the remainder from a freshly-picked source rather than no-op'ing on the duplicate assignment, and its chunk handler appends only the strictly-next entry so a resume overlap can't inflate the verified prefix past `sealed_end`. On takeover the new leader re-seeds the tracker for every known-end sealed segment it leads (`reseed_catch_up`, from `reconcile_on_leadership_change`), since the tracker is leader-volatile — so a repair in flight at a leadership change is re-driven, not stranded (already-complete members full-match-ack cheaply; the re-drive of all sealed segments is the cost). This is a *rule*, not an invariant (it is a liveness guarantee over time, not a snapshot predicate): no `assert_invariants` entry. diff --git a/src/config.rs b/src/config.rs index d8c6a871..b2b2e046 100644 --- a/src/config.rs +++ b/src/config.rs @@ -129,8 +129,8 @@ pub struct Environment { #[arg(long, env = "HOT_CACHE_PRESSURE_WATERMARK", default_value_t = 0.9)] pub hot_cache_pressure_watermark: f64, - #[arg(long, env = "SEAL_REQUEST_TIMEOUT_SECS", default_value_t = 5)] - pub seal_request_timeout_secs: u64, + #[arg(long, env = "SEGMENT_ROLL_REQUEST_TIMEOUT_SECS", default_value_t = 5)] + pub segment_roll_request_timeout_secs: u64, /// Orphan-GC sweep interval, and the grace before a recovered segment is eligible for /// reclaim: a restarted node keeps its on-disk segments for this long so re-fill can @@ -379,7 +379,9 @@ impl Environment { batch_max_bytes: self.batch_max_bytes, hot_cache_budget_bytes: self.hot_cache_budget_bytes, hot_cache_pressure_watermark: self.hot_cache_pressure_watermark, - seal_request_timeout: std::time::Duration::from_secs(self.seal_request_timeout_secs), + segment_roll_request_timeout: std::time::Duration::from_secs( + self.segment_roll_request_timeout_secs, + ), orphan_gc_interval: std::time::Duration::from_secs(self.orphan_gc_interval_secs), data_dir: self.data_dir_path(), } @@ -394,7 +396,7 @@ pub struct DataNodeConfig { pub batch_max_bytes: usize, pub hot_cache_budget_bytes: u64, pub hot_cache_pressure_watermark: f64, - pub seal_request_timeout: std::time::Duration, + pub segment_roll_request_timeout: std::time::Duration, pub orphan_gc_interval: std::time::Duration, pub data_dir: PathBuf, } @@ -444,7 +446,7 @@ mod tests { batch_max_bytes: 10 * 1024 * 1024, hot_cache_budget_bytes: 4 * 1024 * 1024 * 1024, hot_cache_pressure_watermark: 0.9, - seal_request_timeout_secs: 5, + segment_roll_request_timeout_secs: 5, orphan_gc_interval_secs: 300, } } diff --git a/src/control_plane/consensus/actor.rs b/src/control_plane/consensus/actor.rs index 4f244671..b2165835 100644 --- a/src/control_plane/consensus/actor.rs +++ b/src/control_plane/consensus/actor.rs @@ -173,7 +173,7 @@ impl MultiRaftActor { RaftEvent::RedriveAssignments(cmds) => { self.data_transport_cmds.extend(cmds); } - RaftEvent::SealBoundaryQueries(cmds) => { + RaftEvent::DurableEndQueries(cmds) => { self.data_transport_cmds.extend(cmds); } } diff --git a/src/control_plane/consensus/seal_recovery.rs b/src/control_plane/consensus/boundary_recovery.rs similarity index 84% rename from src/control_plane/consensus/seal_recovery.rs rename to src/control_plane/consensus/boundary_recovery.rs index f5f8beaf..0e83323e 100644 --- a/src/control_plane/consensus/seal_recovery.rs +++ b/src/control_plane/consensus/boundary_recovery.rs @@ -1,12 +1,12 @@ -//! Leader-crash seal-end recovery — a Raft-independent tracker. +//! Leader-crash segment-boundary recovery — a Raft-independent tracker. //! //! When a segment's write leader crashes, the coordinator must recover the //! committed end before sealing it (see `docs/data-plane/leader_crash_seal_boundary.md`). -//! This module owns that bookkeeping as a pure state machine: one [`SealEndGather`] +//! This module owns that bookkeeping as a pure state machine: one [`BoundaryRecoveryRound`] //! per leader-crashed segment, polling the survivors for their durable extents //! and sealing at their `min`. It holds no Raft/transport/topology state — it //! consumes the current leaderless set plus survivor reports, and emits -//! [`SealEndStep`]s for the owner (`MultiRaft`) to execute. +//! [`BoundaryRecoveryAction`]s for the owner (`MultiRaft`) to execute. use std::collections::{HashMap, HashSet}; @@ -32,16 +32,16 @@ pub(crate) enum DurableSegmentEndReportedError { pub(crate) const GATHER_ATTEMPTS: u32 = 3; /// A side effect for the owner to carry out after driving recovery. -pub(crate) enum SealEndStep { +pub(crate) enum BoundaryRecoveryAction { /// (Re)send a boundary query to these survivors. - Query { + QueryDurableEnds { segment_key: SegmentKey, targets: Vec, }, /// Propose the recovery roll: seal `segment_key` at `end`, led by `leader`. /// `end = None` is the unknown-end fallback (timeout, or a survivor holds /// nothing). - Seal { + ProposeRoll { shard_group_id: ShardGroupId, segment_key: SegmentKey, end: Option, @@ -49,9 +49,9 @@ pub(crate) enum SealEndStep { }, } -/// One in-flight seal-end recovery: poll the survivors for their durable +/// One in-flight boundary recovery: poll the survivors for their durable /// extents, seal at the `min` (the committed end), let the most-recent lead. -struct SealEndGather { +struct BoundaryRecoveryRound { shard_group_id: ShardGroupId, /// Survivors we await, in replica-set order. nodes: Vec, @@ -64,7 +64,7 @@ struct SealEndGather { proposed: bool, } -impl SealEndGather { +impl BoundaryRecoveryRound { fn new(shard_group_id: ShardGroupId, nodes: Vec) -> Self { Self { shard_group_id, @@ -84,7 +84,7 @@ impl SealEndGather { segment_key: SegmentKey, from: NodeId, durable_end: Option, - ) -> Result, DurableSegmentEndReportedError> { + ) -> Result, DurableSegmentEndReportedError> { if self.proposed { return Ok(None); } @@ -106,7 +106,7 @@ impl SealEndGather { return Ok(None); } self.proposed = true; - Ok(Some(self.seal(segment_key))) + Ok(Some(self.propose_roll(segment_key))) } fn is_complete(&self) -> bool { @@ -151,9 +151,9 @@ impl SealEndGather { ranked.first().map(|(_, node)| (*node).clone()) } - /// The seal step for a completed gather. - fn seal(&self, segment_key: SegmentKey) -> SealEndStep { - SealEndStep::Seal { + /// The roll proposal for a completed gather. + fn propose_roll(&self, segment_key: SegmentKey) -> BoundaryRecoveryAction { + BoundaryRecoveryAction::ProposeRoll { shard_group_id: self.shard_group_id, segment_key, end: self.recovered_end(), @@ -162,15 +162,15 @@ impl SealEndGather { } } -/// Tracks every in-flight seal-end recovery (one per leader-crashed segment). -/// Pure: it decides the next [`SealEndStep`]s and the owner executes them, +/// Tracks every in-flight boundary recovery (one per leader-crashed segment). +/// Pure: it decides the next [`BoundaryRecoveryAction`]s and the owner executes them, /// feeding back survivor reports and the current leaderless set. #[derive(Default)] -pub(crate) struct SealEndRecovery { - gathers: HashMap, +pub(crate) struct SegmentBoundaryRecovery { + gathers: HashMap, } -impl SealEndRecovery { +impl SegmentBoundaryRecovery { /// Start / re-query / expire gathers for `group`'s current leaderless /// `segments`. Returns the queries to send and rolls to propose. Callers /// prune stale gathers (segment no longer leaderless) *before* this — see @@ -179,13 +179,15 @@ impl SealEndRecovery { &mut self, group: ShardGroupId, segments: LeaderlessSegments, - ) -> Vec { + ) -> Vec { let mut steps = Vec::with_capacity(segments.len()); for (segment_key, survivors) in segments { let Some(g) = self.gathers.get_mut(&segment_key) else { - self.gathers - .insert(segment_key, SealEndGather::new(group, survivors.clone())); - steps.push(SealEndStep::Query { + self.gathers.insert( + segment_key, + BoundaryRecoveryRound::new(group, survivors.clone()), + ); + steps.push(BoundaryRecoveryAction::QueryDurableEnds { segment_key, targets: survivors, }); @@ -198,7 +200,7 @@ impl SealEndRecovery { // applied, reconciliation no longer includes it and prunes us. if g.proposed { if g.is_complete() { - steps.push(g.seal(segment_key)); + steps.push(g.propose_roll(segment_key)); } else { // The unknown-end fallback was proposed after an incomplete // gather. It may have applied while a replica was down, but @@ -207,7 +209,7 @@ impl SealEndRecovery { g.proposed = false; g.reports.clear(); g.attempts_left = GATHER_ATTEMPTS; - steps.push(SealEndStep::Query { + steps.push(BoundaryRecoveryAction::QueryDurableEnds { segment_key, targets: g.nodes.clone(), }); @@ -218,10 +220,10 @@ impl SealEndRecovery { if g.attempts_left == 0 { g.proposed = true; tracing::warn!( - "seal-end recovery timed out for {:?}; sealing with unknown end", + "boundary recovery timed out for {:?}; proposing a roll with an unknown end", segment_key ); - steps.push(SealEndStep::Seal { + steps.push(BoundaryRecoveryAction::ProposeRoll { shard_group_id: group, segment_key, end: None, @@ -231,7 +233,7 @@ impl SealEndRecovery { } g.attempts_left -= 1; - steps.push(SealEndStep::Query { + steps.push(BoundaryRecoveryAction::QueryDurableEnds { segment_key, targets: g.pending(), }); @@ -247,7 +249,7 @@ impl SealEndRecovery { segment_key: SegmentKey, from: NodeId, durable_end: Option, - ) -> Result, DurableSegmentEndReportedError> { + ) -> Result, DurableSegmentEndReportedError> { self.gathers .get_mut(&segment_key) .ok_or(DurableSegmentEndReportedError::UnknownRecovery)? @@ -308,9 +310,9 @@ mod tests { NodeId::new(id) } - fn gather_with(reports: &[(&str, Option)]) -> SealEndGather { + fn gather_with(reports: &[(&str, Option)]) -> BoundaryRecoveryRound { let nodes = reports.iter().map(|(n, _)| node(n)).collect(); - let mut g = SealEndGather::new(ShardGroupId(0), nodes); + let mut g = BoundaryRecoveryRound::new(ShardGroupId(0), nodes); let key = SegmentKey::new(TopicId(0), RangeId(0), SegmentId(0)); for (n, end) in reports { let _ = g.record(key, node(n), end.map(EntryId)).unwrap(); @@ -354,14 +356,14 @@ mod tests { #[test] fn advance_starts_then_expires_with_unknown_end() { let seg = SegmentKey::new(TopicId(0), RangeId(0), SegmentId(0)); - let mut recovery = SealEndRecovery::default(); + let mut recovery = SegmentBoundaryRecovery::default(); let segments = || vec![(seg, vec![node("y"), node("z")])]; // First pass starts the gather and queries both survivors. let steps = recovery.advance(ShardGroupId(1), segments()); assert!(matches!( steps.as_slice(), - [SealEndStep::Query { targets, .. }] if targets == &[node("y"), node("z")] + [BoundaryRecoveryAction::QueryDurableEnds { targets, .. }] if targets == &[node("y"), node("z")] )); assert!(recovery.contains(&seg)); @@ -369,23 +371,23 @@ mod tests { for _ in 0..GATHER_ATTEMPTS { assert!(matches!( recovery.advance(ShardGroupId(1), segments()).as_slice(), - [SealEndStep::Query { .. }] + [BoundaryRecoveryAction::QueryDurableEnds { .. }] )); } assert!(matches!( recovery.advance(ShardGroupId(1), segments()).as_slice(), - [SealEndStep::Seal { end: None, .. }] + [BoundaryRecoveryAction::ProposeRoll { end: None, .. }] )); assert!(matches!( recovery.advance(ShardGroupId(1), segments()).as_slice(), - [SealEndStep::Query { targets, .. }] if targets == &[node("y"), node("z")] + [BoundaryRecoveryAction::QueryDurableEnds { targets, .. }] if targets == &[node("y"), node("z")] )); } #[test] - fn record_seals_at_min_with_recency_leader_once() { + fn record_proposes_roll_at_min_with_recency_leader_once() { let seg = SegmentKey::new(TopicId(0), RangeId(0), SegmentId(0)); - let mut recovery = SealEndRecovery::default(); + let mut recovery = SegmentBoundaryRecovery::default(); recovery.advance(ShardGroupId(1), vec![(seg, vec![node("y"), node("z")])]); assert!(matches!( @@ -398,7 +400,7 @@ mod tests { .expect("completes"); assert!(matches!( step, - SealEndStep::Seal { end: Some(EntryId(40)), leader: Some(l), .. } if l == node("y") + BoundaryRecoveryAction::ProposeRoll { end: Some(EntryId(40)), leader: Some(l), .. } if l == node("y") )); // A late/duplicate report after the roll is proposed is ignored. assert!(matches!( @@ -412,7 +414,7 @@ mod tests { recovery .advance(ShardGroupId(1), vec![(seg, vec![node("y"), node("z")])]) .as_slice(), - [SealEndStep::Seal { + [BoundaryRecoveryAction::ProposeRoll { end: Some(EntryId(40)), .. }] @@ -422,7 +424,7 @@ mod tests { #[test] fn conflicting_and_unexpected_reports_are_rejected() { let seg = SegmentKey::new(TopicId(0), RangeId(0), SegmentId(0)); - let mut recovery = SealEndRecovery::default(); + let mut recovery = SegmentBoundaryRecovery::default(); recovery.advance(ShardGroupId(1), vec![(seg, vec![node("y"), node("z")])]); assert!(matches!( @@ -447,7 +449,7 @@ mod tests { #[test] fn drop_stale_reconciled_drops_segments_no_longer_leaderless() { let seg = SegmentKey::new(TopicId(0), RangeId(0), SegmentId(0)); - let mut recovery = SealEndRecovery::default(); + let mut recovery = SegmentBoundaryRecovery::default(); recovery.advance(ShardGroupId(1), vec![(seg, vec![node("y")])]); assert!(recovery.contains(&seg)); diff --git a/src/control_plane/consensus/messages/command.rs b/src/control_plane/consensus/messages/command.rs index 0429d343..dc0c2f02 100644 --- a/src/control_plane/consensus/messages/command.rs +++ b/src/control_plane/consensus/messages/command.rs @@ -4,7 +4,7 @@ use crate::control_plane::consensus::messages::timer::RaftTimeoutCallback; use crate::control_plane::membership::{ShardGroup, ShardGroupId}; use crate::control_plane::metadata::command::MetadataCommand; use crate::data_plane::messages::command::RequestSegmentRoll; -use crate::impl_from_variant; +use crate::{impl_from_variant, impl_new_struct_wrapper}; pub struct InboundRaftRpc { pub shard_group_id: ShardGroupId, @@ -50,6 +50,5 @@ pub enum RaftTransportCommand { } #[derive(Debug)] -pub struct ProposeSegmentRoll { - pub request: RequestSegmentRoll, -} +pub struct ProposeSegmentRoll(pub RequestSegmentRoll); +impl_new_struct_wrapper!(ProposeSegmentRoll, RequestSegmentRoll); diff --git a/src/control_plane/consensus/messages/event.rs b/src/control_plane/consensus/messages/event.rs index 8ae59bc2..63835475 100644 --- a/src/control_plane/consensus/messages/event.rs +++ b/src/control_plane/consensus/messages/event.rs @@ -1,7 +1,7 @@ use crate::control_plane::NodeId; use crate::control_plane::consensus::messages::rpc::OutboundRaftPacket; use crate::control_plane::consensus::messages::timer::RaftTimer; -use crate::control_plane::consensus::multi_raft::SealContext; +use crate::control_plane::consensus::multi_raft::RollRequestContext; use crate::control_plane::consensus::raft::log::LogEntry; use crate::control_plane::membership::ShardGroupId; use crate::control_plane::metadata::event::ApplyResult; @@ -21,7 +21,7 @@ pub struct MetadataCommitted { pub shard_group_id: ShardGroupId, pub result: ApplyResult, pub log_index: u64, - pub seal_context: Option, + pub roll_context: Option, } #[derive(Debug, Clone)] @@ -55,7 +55,7 @@ pub enum RaftEvent { /// Leader-crash `RequestDurableSegmentEnd` fan-out to a segment's survivors. /// A coordinator-initiated data-plane send not tied to a committed entry. /// The actor just forwards these to the data transport. - SealBoundaryQueries(Vec), + DurableEndQueries(Vec), } impl MetadataCommitted { @@ -65,14 +65,14 @@ impl MetadataCommitted { ApplyResult::TopicCreated(tc) => { vec![tc.into_command(sgid)] } - ApplyResult::SegmentRolled(sr) => sr.into_command(self.seal_context, sgid), + ApplyResult::SegmentRolled(sr) => sr.into_command(self.roll_context, sgid), ApplyResult::RangeSplit(rs) => rs.into_command(sgid), ApplyResult::RangeMerged(rm) => rm.into_commands(sgid), ApplyResult::TopicDeleted(deleted) => deleted.into_commands(), ApplyResult::Noop => vec![], ApplyResult::SegmentReassigned(r) => r.into_catch_up_commands(sgid), ApplyResult::SegmentsDeleted(d) => d.into_commands(), - ApplyResult::SegmentSealCorrected(ssc) => ssc.into_commands(), + ApplyResult::SegmentBoundaryCorrected(ssc) => ssc.into_commands(), ApplyResult::ConsumerGroupChanged(epoch) => epoch.into_commands(), } } diff --git a/src/control_plane/consensus/mod.rs b/src/control_plane/consensus/mod.rs index 8b462de6..27628a3a 100644 --- a/src/control_plane/consensus/mod.rs +++ b/src/control_plane/consensus/mod.rs @@ -1,6 +1,6 @@ pub(crate) mod actor; +pub(crate) mod boundary_recovery; pub(crate) mod messages; pub(crate) mod multi_raft; pub(crate) mod raft; -pub(crate) mod seal_recovery; pub(crate) mod transport; diff --git a/src/control_plane/consensus/multi_raft.rs b/src/control_plane/consensus/multi_raft.rs index a18d5212..683fa67a 100644 --- a/src/control_plane/consensus/multi_raft.rs +++ b/src/control_plane/consensus/multi_raft.rs @@ -1,4 +1,7 @@ use crate::control_plane::NodeId; +use crate::control_plane::consensus::boundary_recovery::{ + BoundaryRecoveryAction, SegmentBoundaryRecovery, +}; use crate::control_plane::consensus::messages::{ DeferredConsumerGroupAssignment, DeferredReply, InboundRaftRpc, LogMutation, MetadataProposal, MultiRaftActorCommand, ProposeSegmentRoll, RaftEvent, RaftProtocolMessage, RaftTimeoutCallback, @@ -8,11 +11,10 @@ use crate::control_plane::consensus::raft::state::{Raft, TimerSeqs}; use crate::control_plane::consensus::raft::states::consensus::LeaderlessSegments; use crate::control_plane::consensus::raft::storage::RaftStorage; use crate::control_plane::consensus::raft::{compute_replacement_replica_set, now_ms}; -use crate::control_plane::consensus::seal_recovery::{SealEndRecovery, SealEndStep}; use crate::control_plane::membership::{ShardGroup, ShardGroupId, TopologyReader}; use crate::control_plane::metadata::command::RollSegment; use crate::control_plane::metadata::{ - ConsumerGroupAssignment, EntryId, TopicId, TopicMeta, TopicStats, + ConsumerGroupAssignment, EntryId, SegmentRollIntent, TopicId, TopicMeta, TopicStats, }; use crate::data_plane::SegmentKey; use crate::data_plane::messages::command::{ @@ -37,62 +39,66 @@ pub(crate) struct MultiRaft { /// Senders waiting for a specific (shard, log index) to be committed and applied. pending_proposes: BTreeMap<(ShardGroupId, u64), oneshot::Sender>>, - pending_seals: PendingSealTracker, - /// Leader-crash seal-end recoveries (one per leader-crashed segment). - seal_recovery: SealEndRecovery, + pending_rolls: PendingRollTracker, + /// Leader-crash boundary recoveries (one per leader-crashed segment). + boundary_recovery: SegmentBoundaryRecovery, topology: TopologyReader, } #[derive(Debug)] -pub(crate) struct SealContext { +pub(crate) struct RollRequestContext { pub(crate) requester: NodeId, pub(crate) segment_key: SegmentKey, } -// Bookkeeping for in-flight seal proposals. +// Bookkeeping for in-flight roll proposals. #[derive(Default)] -struct PendingSealTracker { - by_group: HashMap>, +struct PendingRollTracker { + by_group: HashMap>, by_key: HashSet, } -impl PendingSealTracker { +impl PendingRollTracker { fn contains(&self, key: &SegmentKey) -> bool { self.by_key.contains(key) } - fn insert(&mut self, group_id: ShardGroupId, log_index: u64, seal: SealContext) { - self.by_key.insert(seal.segment_key); + fn insert(&mut self, group_id: ShardGroupId, log_index: u64, roll: RollRequestContext) { + self.by_key.insert(roll.segment_key); self.by_group .entry(group_id) .or_default() - .insert(log_index, seal); + .insert(log_index, roll); } - fn remove(&mut self, group_id: ShardGroupId, log_index: u64) -> Option { + fn remove(&mut self, group_id: ShardGroupId, log_index: u64) -> Option { let group_map = self.by_group.get_mut(&group_id)?; - let seal = group_map.remove(&log_index)?; + let roll = group_map.remove(&log_index)?; // Clean up the empty map to prevent memory leaks over time if group_map.is_empty() { self.by_group.remove(&group_id); } - self.by_key.remove(&seal.segment_key); - Some(seal) + self.by_key.remove(&roll.segment_key); + Some(roll) } - fn pop_seal_context(&mut self, group_id: ShardGroupId, log_index: u64) -> Option { + fn pop_roll_context( + &mut self, + group_id: ShardGroupId, + log_index: u64, + ) -> Option { self.remove(group_id, log_index) } - /// Drop pending seal contexts for `group_id` whose log indices were + /// Drop pending roll contexts for `group_id` whose log indices were /// truncated by a new leader. Prevents unbounded growth of `by_key` / /// `by_index` when proposals are rejected by truncation. fn drop_from(&mut self, group_id: ShardGroupId, from_index: u64) { if let Some(group_map) = self.by_group.get_mut(&group_id) { let stale = group_map.split_off(&from_index); - for seal in stale.into_values() { - self.by_key.remove(&seal.segment_key); + for roll in stale.into_values() { + self.by_key.remove(&roll.segment_key); } if group_map.is_empty() { @@ -101,14 +107,14 @@ impl PendingSealTracker { } } - /// Drop every pending seal context for `group_id`. Called when a leader + /// Drop every pending roll context for `group_id`. Called when a leader /// steps down — any not-yet-committed proposals will either be truncated /// by the new leader or commit there, but this replica will no longer be /// the one to dispatch `SegmentRollCommitted`. fn drop_group(&mut self, group_id: ShardGroupId) { if let Some(stale_group) = self.by_group.remove(&group_id) { - for seal in stale_group.into_values() { - self.by_key.remove(&seal.segment_key); + for roll in stale_group.into_values() { + self.by_key.remove(&roll.segment_key); } } } @@ -131,8 +137,8 @@ impl MultiRaft { pending_events: Vec::new(), deferred: Vec::new(), pending_proposes: BTreeMap::new(), - pending_seals: PendingSealTracker::default(), - seal_recovery: SealEndRecovery::default(), + pending_rolls: PendingRollTracker::default(), + boundary_recovery: SegmentBoundaryRecovery::default(), topology, } } @@ -215,9 +221,9 @@ impl MultiRaft { let leaderless = raft.take_leaderless_segments(); - self.seal_recovery + self.boundary_recovery .drop_stale_in_group(shard_group_id, &leaderless); - self.drive_seal_end_recovery(shard_group_id, leaderless); + self.drive_boundary_recovery(shard_group_id, leaderless); } pub(crate) fn process(&mut self, cmd: MultiRaftActorCommand) { @@ -272,7 +278,7 @@ impl MultiRaft { self.handle_segment_placed(ack); } MultiRaftActorCommand::DurableSegmentEndReported(report) => { - self.handle_seal_boundary_report(report); + self.handle_durable_end_report(report); } MultiRaftActorCommand::SegmentCaughtUp(ack) => { self.handle_catch_up_ack(ack); @@ -504,24 +510,23 @@ impl MultiRaft { // Across every group we just reconciled, drop gathers whose segment is no // longer leaderless — a group left with none drops all of its gathers. - self.seal_recovery.drop_stale_reconciled(&reconciled); + self.boundary_recovery.drop_stale_reconciled(&reconciled); for (shard_group_id, leaderless) in reconciled { if !leaderless.is_empty() { - self.drive_seal_end_recovery(shard_group_id, leaderless); + self.drive_boundary_recovery(shard_group_id, leaderless); } } } fn propose_segment_roll(&mut self, req: ProposeSegmentRoll) { - let seal = &req.request; - if self.pending_seals.contains(&seal.segment_key) { + if self.pending_rolls.contains(&req.segment_key) { return; } - let Some(group_id) = self.find_group_for_topic(&seal.segment_key.topic_id) else { + let Some(group_id) = self.find_group_for_topic(&req.segment_key.topic_id) else { tracing::warn!( "segment roll requested for unknown topic {:?}", - seal.segment_key.topic_id + req.segment_key.topic_id ); return; }; @@ -529,10 +534,10 @@ impl MultiRaft { return; }; - let Some(old_replica_set) = raft.get_replica_set(&seal.segment_key) else { + let Some(old_replica_set) = raft.get_replica_set(&req.segment_key) else { tracing::warn!( "segment roll requested for unknown segment {:?}", - seal.segment_key + req.segment_key ); return; }; @@ -540,23 +545,24 @@ impl MultiRaft { let live_nodes = self.topology.live_nodes(); let new_replica_set = - compute_replacement_replica_set(&old_replica_set, &seal.failed_nodes, &live_nodes); + compute_replacement_replica_set(&old_replica_set, &req.failed_nodes, &live_nodes); let cmd = RollSegment { - segment_key: seal.segment_key, + segment_key: req.segment_key, sealed_at: now_ms(), new_replica_set, - end_entry_id: Some(seal.end_entry_id), + end_entry_id: Some(req.end_entry_id), + intent: req.intent, }; match raft.propose(cmd.into()) { Ok(log_index) => { - self.pending_seals.insert( + self.pending_rolls.insert( group_id, log_index, - SealContext { - requester: seal.from.clone(), - segment_key: seal.segment_key, + RollRequestContext { + requester: req.from.clone(), + segment_key: req.segment_key, }, ); self.dirty.insert(group_id); @@ -567,27 +573,27 @@ impl MultiRaft { } } - /// Drive the seal-end recovery subsystem for `group`'s leaderless `segments` + /// Drive boundary recovery for `group`'s leaderless `segments` /// (callers prune stale gathers first), executing the steps it returns. - fn drive_seal_end_recovery(&mut self, group: ShardGroupId, segments: LeaderlessSegments) { - for step in self.seal_recovery.advance(group, segments) { - self.execute_seal_step(step); + fn drive_boundary_recovery(&mut self, group: ShardGroupId, segments: LeaderlessSegments) { + for step in self.boundary_recovery.advance(group, segments) { + self.execute_boundary_recovery_action(step); } } /// Feed a survivor's durable extent into the subsystem; if it completes a - /// gather, execute the resulting seal. - fn handle_seal_boundary_report(&mut self, report: DurableSegmentEndReported) { + /// round, execute the resulting action. + fn handle_durable_end_report(&mut self, report: DurableSegmentEndReported) { match self - .seal_recovery + .boundary_recovery .record(report.segment_key, report.from, report.durable_end) { - Ok(Some(step)) => self.execute_seal_step(step), + Ok(Some(step)) => self.execute_boundary_recovery_action(step), Ok(None) => {} Err(error) => tracing::warn!( segment = ?report.segment_key, ?error, - "rejected seal-boundary report", + "rejected durable-end report", ), } } @@ -601,13 +607,13 @@ impl MultiRaft { raft.handle_catch_up_ack(ack); } - fn execute_seal_step(&mut self, step: SealEndStep) { + fn execute_boundary_recovery_action(&mut self, step: BoundaryRecoveryAction) { match step { - SealEndStep::Query { + BoundaryRecoveryAction::QueryDurableEnds { segment_key, targets, - } => self.emit_seal_boundary_queries(segment_key, targets), - SealEndStep::Seal { + } => self.emit_durable_end_queries(segment_key, targets), + BoundaryRecoveryAction::ProposeRoll { shard_group_id, segment_key, end, @@ -655,6 +661,7 @@ impl MultiRaft { sealed_at: now_ms(), new_replica_set, end_entry_id: end, + intent: SegmentRollIntent::Recovery, }; match raft.propose(cmd.into()) { Ok(_) => { @@ -664,7 +671,7 @@ impl MultiRaft { } } - fn emit_seal_boundary_queries(&mut self, segment_key: SegmentKey, targets: Vec) { + fn emit_durable_end_queries(&mut self, segment_key: SegmentKey, targets: Vec) { if targets.is_empty() { return; } @@ -676,7 +683,7 @@ impl MultiRaft { }, ); self.pending_events - .push(RaftEvent::SealBoundaryQueries(vec![cmd])); + .push(RaftEvent::DurableEndQueries(vec![cmd])); } fn find_group_for_topic(&self, topic_id: &TopicId) -> Option { @@ -793,13 +800,13 @@ impl MultiRaft { if let LogMutation::TruncateFrom(from_index) = &log { // Rare and destructive: a new leader is overwriting this // replica's conflicting log suffix. Worth a trace — both - // the data loss and the dropped seal contexts. + // the data loss and the dropped roll contexts. tracing::debug!( group = id.0, from = *from_index, "log truncation: dropping conflicting suffix and its pending seals", ); - self.pending_seals.drop_from(*id, *from_index); + self.pending_rolls.drop_from(*id, *from_index); } mutations.push((*id, log)); } @@ -808,10 +815,10 @@ impl MultiRaft { // that would otherwise prune is leader-gated and stops firing). A new // leader re-derives: re-proposes the seal / re-polls the survivors. if !raft.is_leader() { - // pending_seals: its committed roll notification won't come from here. - self.pending_seals.drop_group(*id); - // seal-end gathers: we can't propose their recovery roll. - self.seal_recovery.drop_group(*id); + // pending_rolls: its committed roll notification won't come from here. + self.pending_rolls.drop_group(*id); + // boundary-recovery rounds: we can't propose their recovery roll. + self.boundary_recovery.drop_group(*id); } self.route_group_events(*id); } @@ -820,7 +827,7 @@ impl MultiRaft { /// Drain a group's buffered events into `pending_events`. `MetadataCommitted` /// is dropped on non-leaders (only the current leader dispatches to the data - /// plane — see invariant 3); seal context is attached for the leader. Called + /// plane — see invariant 3); roll context is attached for the leader. Called /// both after step-driven apply (`collect_mutations`) and after the durability /// gate advances apply (`persist_and_advance`). fn route_group_events(&mut self, id: ShardGroupId) { @@ -836,8 +843,8 @@ impl MultiRaft { if let RaftEvent::MetadataCommitted(committed) = &mut event { // For the leader to send the committed roll back to the requester. - committed.seal_context = - self.pending_seals.pop_seal_context(id, committed.log_index); + committed.roll_context = + self.pending_rolls.pop_roll_context(id, committed.log_index); } self.pending_events.push(event); } @@ -934,8 +941,8 @@ mod tests { use super::*; use crate::control_plane::Replicas; + use crate::control_plane::consensus::boundary_recovery; use crate::control_plane::consensus::raft::storage::RaftPersistentState; - use crate::control_plane::consensus::seal_recovery; use crate::impls::metadata_storage::MetadataStorage; use crate::schedulers::ticker_message::TimerCommand; use std::collections::BTreeSet; @@ -1836,7 +1843,7 @@ mod tests { ); } - // --- Leader-crash seal-end recovery ----------------------------------- + // --- Leader-crash boundary recovery ----------------------------------- fn create_topic_via_store(store: &mut MultiRaft, name: &str, replica_set: Vec) { use crate::control_plane::metadata::command::{CreateTopic, MetadataCommand}; @@ -1861,11 +1868,11 @@ mod tests { } /// Targets a `RequestDurableSegmentEnd` for `seg` was fanned out to (sorted). - fn seal_boundary_query_targets(events: &[RaftEvent], seg: SegmentKey) -> Vec { + fn durable_end_query_targets(events: &[RaftEvent], seg: SegmentKey) -> Vec { use crate::data_plane::messages::command::DataPlanePeerMessage; let mut out = Vec::new(); for e in events { - let RaftEvent::SealBoundaryQueries(cmds) = e else { + let RaftEvent::DurableEndQueries(cmds) = e else { continue; }; for cmd in cmds { @@ -1894,7 +1901,7 @@ mod tests { } #[test] - fn seal_end_gather_seals_at_min_with_recency_leader() { + fn boundary_recovery_proposes_roll_at_min_with_recency_leader() { use crate::control_plane::metadata::{RangeId, SegmentId, TopicId}; // Coordinator n1 leads the shard group; the segment's data replicas are @@ -1911,13 +1918,13 @@ mod tests { let seg0 = SegmentKey::new(TopicId((TEST_GROUP_ID.0) << 32), RangeId(0), SegmentId(0)); // x (the leader) crashed: the death drives reconcile, which finds the - // leaderless segment and starts a seal-end gather querying both + // leaderless segment and starts a boundary-recovery round querying both // survivors. store.handle_node_death(node("x")); - assert!(store.seal_recovery.contains(&seg0)); + assert!(store.boundary_recovery.contains(&seg0)); let events = store.flush(); assert_eq!( - seal_boundary_query_targets(&events, seg0), + durable_end_query_targets(&events, seg0), vec![node("y"), node("z")], "query fans out to both survivors" ); @@ -1925,12 +1932,12 @@ mod tests { let before = proposals_after_become_leader(&store).len(); // Survivors report durable extents; the second completes the gather. - store.handle_seal_boundary_report(DurableSegmentEndReported { + store.handle_durable_end_report(DurableSegmentEndReported { segment_key: seg0, from: node("y"), durable_end: Some(EntryId(50)), }); - store.handle_seal_boundary_report(DurableSegmentEndReported { + store.handle_durable_end_report(DurableSegmentEndReported { segment_key: seg0, from: node("z"), durable_end: Some(EntryId(40)), @@ -1953,7 +1960,7 @@ mod tests { } #[test] - fn seal_end_gather_expires_to_unknown_end() { + fn boundary_recovery_expires_to_unknown_end() { use crate::control_plane::metadata::{RangeId, SegmentId, TopicId}; let (storage, _tmp) = temp_storage(); @@ -1970,7 +1977,7 @@ mod tests { // No reports ever arrive: each death-driven reconcile re-drives the // gather; after the attempt budget it falls back to an unknown-end roll. - for _ in 0..(seal_recovery::GATHER_ATTEMPTS + 2) { + for _ in 0..(boundary_recovery::GATHER_ATTEMPTS + 2) { store.handle_node_death(node("x")); } store.flush(); @@ -1985,7 +1992,7 @@ mod tests { } #[test] - fn seal_end_gather_dropped_on_step_down() { + fn boundary_recovery_dropped_on_step_down() { use crate::control_plane::consensus::messages::{AppendEntries, RaftRpc}; use crate::control_plane::metadata::{RangeId, SegmentId, TopicId}; @@ -2001,8 +2008,8 @@ mod tests { let seg0 = SegmentKey::new(TopicId((TEST_GROUP_ID.0) << 32), RangeId(0), SegmentId(0)); store.handle_node_death(node("x")); assert!( - store.seal_recovery.contains(&seg0), - "the leader started a seal-end gather" + store.boundary_recovery.contains(&seg0), + "the leader started a boundary-recovery round" ); // A higher-term AppendEntries deposes n1. The drain-time `!is_leader` @@ -2023,8 +2030,8 @@ mod tests { store.flush(); assert!( - store.seal_recovery.is_empty(), - "a deposed leader drops its seal-end gathers" + store.boundary_recovery.is_empty(), + "a deposed leader drops its boundary-recovery rounds" ); } } diff --git a/src/control_plane/consensus/raft/states/metadata_state.rs b/src/control_plane/consensus/raft/states/metadata_state.rs index 3805f3ed..27e9b460 100644 --- a/src/control_plane/consensus/raft/states/metadata_state.rs +++ b/src/control_plane/consensus/raft/states/metadata_state.rs @@ -216,17 +216,23 @@ impl MetadataState { return Ok(ApplyResult::Noop); }; - return Ok(ApplyResult::SegmentSealCorrected(SegmentSealCorrected { - segment_key: cmd.segment_key, - replica_set, - committed_entry_id: cmd.end_entry_id, - })); + return Ok(ApplyResult::SegmentBoundaryCorrected( + SegmentBoundaryCorrected { + segment_key: cmd.segment_key, + replica_set, + committed_entry_id: cmd.end_entry_id, + }, + )); + } + + if cmd.intent == SegmentRollIntent::BoundaryCorrection { + return Ok(ApplyResult::Noop); } // If Active, Roll let new_segment_id = range.roll_segment(cmd.clone())?; - let split_proposal = (range.should_split(cmd.sealed_at) && can_split) - .then(|| range.build_split_proposal(&cmd)); + let split_proposal = + (range.should_split() && can_split).then(|| range.build_split_proposal(&cmd)); // For segment roll, unless data nodes got changed, consumer // TODO For now, it loops over EVERY consumger groups and take epoch snapshot for every topic. @@ -238,6 +244,10 @@ impl MetadataState { .collect(); tracing::debug!("Consumer groups: {:?}", consumer_group_epochs); + let merge_proposal = (cmd.intent == SegmentRollIntent::IdleMaintenance) + .then(|| topic.find_mergeable_range_pair(cmd.sealed_at)) + .flatten(); + if let Some(proposal) = split_proposal { match proposal { Ok(proposal) => self @@ -250,6 +260,9 @@ impl MetadataState { ), } } + if let Some(proposal) = merge_proposal { + self.pending_proposals.push(proposal); + } Ok(SegmentRolled { new_segment_key: cmd.segment_key.with_segment_id(new_segment_id), new_replica_set: cmd.new_replica_set, @@ -482,8 +495,6 @@ mod tests { strategy::{PartitionStrategy, StoragePolicy}, }, }; - use std::collections::VecDeque; - fn default_policy() -> StoragePolicy { StoragePolicy { retention_ms: Some(3_600_000), @@ -569,12 +580,31 @@ mod tests { range_id: RangeId, segment_id: SegmentId, sealed_at: u64, + ) { + roll_segment_with_intent( + sm, + topic_id, + range_id, + segment_id, + sealed_at, + SegmentRollIntent::DataPressure, + ); + } + + fn roll_segment_with_intent( + sm: &mut MetadataState, + topic_id: TopicId, + range_id: RangeId, + segment_id: SegmentId, + sealed_at: u64, + intent: SegmentRollIntent, ) { let result = sm.apply(MetadataCommand::RollSegment(RollSegment { segment_key: SegmentKey::new(topic_id, range_id, segment_id), sealed_at, new_replica_set: replica_set(), end_entry_id: None, + intent, })); assert!(matches!(result.unwrap(), ApplyResult::SegmentRolled(_))); } @@ -596,6 +626,7 @@ mod tests { sealed_at, new_replica_set: replica_set(), end_entry_id: Some(EntryId(end_entry_id)), + intent: SegmentRollIntent::Recovery, })); assert!(matches!(result.unwrap(), ApplyResult::SegmentRolled(_))); } @@ -966,6 +997,7 @@ mod tests { sealed_at: 2000, new_replica_set: replica_set(), end_entry_id: None, + intent: SegmentRollIntent::Recovery, })); assert_eq!(result, Err(TopicNotFound(TopicId(99)))); } @@ -980,6 +1012,7 @@ mod tests { sealed_at: 2000, new_replica_set: replica_set(), end_entry_id: None, + intent: SegmentRollIntent::Recovery, })); assert_eq!(result, Err(RangeNotFound)); } @@ -994,6 +1027,7 @@ mod tests { sealed_at: 2000, new_replica_set: replica_set(), end_entry_id: None, + intent: SegmentRollIntent::Recovery, })); assert_eq!(result, Ok(ApplyResult::Noop)); @@ -1130,6 +1164,7 @@ mod tests { assert_eq!(merged.keyspace_start, KEYSPACE_MIN); assert_eq!(merged.keyspace_end, KEYSPACE_MAX); assert_eq!(merged.state, RangeState::Active); + assert_eq!(merged.load_state, RangeLoadState::Unclassified); } #[test] @@ -1281,6 +1316,7 @@ mod tests { sealed_at: 3000, new_replica_set: replica_set(), end_entry_id: None, + intent: SegmentRollIntent::DataPressure, })); assert_eq!(result, Ok(ApplyResult::Noop)); @@ -1289,90 +1325,93 @@ mod tests { assert_eq!(range.segments.len(), 2); } - // --- Hot Range Detection --- - #[test] - fn seal_history_records_timestamps() { + fn boundary_correction_cannot_roll_active_segment() { let mut sm = MetadataState::new(ShardGroupId(0)); let tid = create_topic(&mut sm, "blue"); - roll_segment(&mut sm, tid, RangeId(0), SegmentId(0), 1000); - roll_segment(&mut sm, tid, RangeId(0), SegmentId(1), 2000); - roll_segment(&mut sm, tid, RangeId(0), SegmentId(2), 3000); + let result = sm.apply(MetadataCommand::RollSegment(RollSegment { + segment_key: SegmentKey::new(tid, RangeId(0), SegmentId(0)), + sealed_at: 2000, + new_replica_set: replica_set(), + end_entry_id: Some(EntryId(10)), + intent: SegmentRollIntent::BoundaryCorrection, + })); + assert_eq!(result, Ok(ApplyResult::Noop)); let range = &sm.get_topic(&tid).unwrap().ranges[&RangeId(0)]; - assert_eq!(range.seal_history.seal_count(), 3); + assert_eq!(range.active_segment, Some(SegmentId(0))); + assert_eq!(range.load_state, RangeLoadState::Unclassified); } + // --- Hot Range Detection --- + #[test] - fn seal_history_prunes_old_entries() { + fn pressure_rolls_advance_consecutive_streak() { let mut sm = MetadataState::new(ShardGroupId(0)); let tid = create_topic(&mut sm, "blue"); roll_segment(&mut sm, tid, RangeId(0), SegmentId(0), 1000); roll_segment(&mut sm, tid, RangeId(0), SegmentId(1), 2000); - // Jump far beyond the window — both old entries pruned - let far_future = 2000 + MEASUREMENT_WINDOW_MS + 1; - roll_segment(&mut sm, tid, RangeId(0), SegmentId(2), far_future); let range = &sm.get_topic(&tid).unwrap().ranges[&RangeId(0)]; - assert_eq!(range.seal_history.seal_count(), 1); - } - - #[test] - fn seal_history_orders_delayed_proposal_timestamps() { - let mut history = RangeSealHistory::default(); - history.record_seal(2000); - history.record_seal(1000); - history.record_seal(1500); - - assert_eq!(history.seal_timestamps, VecDeque::from([1000, 1500, 2000])); - } - - #[test] - fn delayed_old_seal_is_pruned_against_newest_timestamp() { - let mut history = RangeSealHistory::default(); - let newest = MEASUREMENT_WINDOW_MS + 2000; - history.record_seal(newest); - history.record_seal(1000); - - assert_eq!(history.seal_timestamps, VecDeque::from([newest])); + assert_eq!( + range.load_state, + RangeLoadState::Pressure(RangePressure { + consecutive_rolls: 2 + }) + ); } #[test] - fn should_split_threshold_met() { - let mut history = RangeSealHistory::default(); - history.record_seal(1000); - history.record_seal(2000); - history.record_seal(3000); - - assert!(history.should_split(3000)); - } + fn idle_roll_resets_pressure() { + let mut sm = MetadataState::new(ShardGroupId(0)); + let tid = create_topic(&mut sm, "blue"); - #[test] - fn should_split_below_threshold() { - let mut history = RangeSealHistory::default(); - history.record_seal(1000); - history.record_seal(2000); + roll_segment(&mut sm, tid, RangeId(0), SegmentId(0), 1000); + roll_segment_with_intent( + &mut sm, + tid, + RangeId(0), + SegmentId(1), + 2000, + SegmentRollIntent::IdleMaintenance, + ); - assert!(!history.should_split(2000)); + let range = &sm.get_topic(&tid).unwrap().ranges[&RangeId(0)]; + assert_eq!(range.load_state, RangeLoadState::Idle); } #[test] - fn should_split_cooldown_blocks() { - let mut history = RangeSealHistory { - seal_timestamps: VecDeque::new(), - created_by_split_at: Some(1000), - }; - history.record_seal(1100); - history.record_seal(1200); - history.record_seal(1300); + fn neutral_rolls_do_not_change_load_state() { + let mut sm = MetadataState::new(ShardGroupId(0)); + let tid = create_topic(&mut sm, "blue"); - // Within cooldown — blocked - assert!(!history.should_split(1300)); + roll_segment(&mut sm, tid, RangeId(0), SegmentId(0), 2000); + roll_segment_with_intent( + &mut sm, + tid, + RangeId(0), + SegmentId(1), + 1000, + SegmentRollIntent::Recovery, + ); + roll_segment_with_intent( + &mut sm, + tid, + RangeId(0), + SegmentId(2), + 500, + SegmentRollIntent::ReplicationFailure, + ); - // After cooldown — allowed - assert!(history.should_split(1000 + SPLIT_COOLDOWN_MS)); + let range = &sm.get_topic(&tid).unwrap().ranges[&RangeId(0)]; + assert_eq!( + range.load_state, + RangeLoadState::Pressure(RangePressure { + consecutive_rolls: 1 + }) + ); } #[test] @@ -1446,12 +1485,57 @@ mod tests { fn evaluate_merges_cold_adjacent() { let mut sm = MetadataState::new(ShardGroupId(0)); let tid = create_topic(&mut sm, "blue"); - split_range(&mut sm, tid, RangeId(0), vec![0x80], 2000); + let (left, right) = split_range(&mut sm, tid, RangeId(0), vec![0x80], 2000); + roll_segment_with_intent( + &mut sm, + tid, + left, + SegmentId(0), + 3000, + SegmentRollIntent::IdleMaintenance, + ); + roll_segment_with_intent( + &mut sm, + tid, + right, + SegmentId(0), + 3000, + SegmentRollIntent::IdleMaintenance, + ); - // Both children are cold (no seals) - let proposals = sm.evaluate_merges(2000 + SPLIT_COOLDOWN_MS + 1); + let proposals = sm.evaluate_merges(3000); assert_eq!(proposals.len(), 1); assert!(matches!(proposals[0], MetadataCommand::MergeRange(_))); + assert!(matches!( + sm.take_pending_proposals().last(), + Some(MetadataCommand::MergeRange(_)) + )); + } + + #[test] + fn split_children_do_not_merge_without_idle_signals() { + let mut sm = MetadataState::new(ShardGroupId(0)); + let tid = create_topic(&mut sm, "blue"); + split_range(&mut sm, tid, RangeId(0), vec![0x80], 2000); + + assert!(sm.evaluate_merges(3000).is_empty()); + } + + #[test] + fn one_idle_child_does_not_merge() { + let mut sm = MetadataState::new(ShardGroupId(0)); + let tid = create_topic(&mut sm, "blue"); + let (left, _right) = split_range(&mut sm, tid, RangeId(0), vec![0x80], 2000); + roll_segment_with_intent( + &mut sm, + tid, + left, + SegmentId(0), + 3000, + SegmentRollIntent::IdleMaintenance, + ); + + assert!(sm.evaluate_merges(3000).is_empty()); } #[test] @@ -1469,7 +1553,7 @@ mod tests { } #[test] - fn split_clears_seal_history() { + fn split_children_start_unclassified() { let mut sm = MetadataState::new(ShardGroupId(0)); let tid = create_topic(&mut sm, "blue"); @@ -1477,25 +1561,8 @@ mod tests { let (c1, c2) = split_range(&mut sm, tid, RangeId(0), vec![0x80], 3000); let topic = sm.get_topic(&tid).unwrap(); - assert!(topic.ranges[&c1].seal_history.seal_timestamps.is_empty()); - assert!(topic.ranges[&c2].seal_history.seal_timestamps.is_empty()); - } - - #[test] - fn split_sets_cooldown() { - let mut sm = MetadataState::new(ShardGroupId(0)); - let tid = create_topic(&mut sm, "blue"); - let (c1, c2) = split_range(&mut sm, tid, RangeId(0), vec![0x80], 2000); - - let topic = sm.get_topic(&tid).unwrap(); - assert_eq!( - topic.ranges[&c1].seal_history.created_by_split_at, - Some(2000) - ); - assert_eq!( - topic.ranges[&c2].seal_history.created_by_split_at, - Some(2000) - ); + assert_eq!(topic.ranges[&c1].load_state, RangeLoadState::Unclassified); + assert_eq!(topic.ranges[&c2].load_state, RangeLoadState::Unclassified); } // --- D3: active_segments_for_node --- @@ -1546,6 +1613,7 @@ mod tests { sealed_at: 2000, new_replica_set: replica_set(), end_entry_id: Some(EntryId(42000)), + intent: SegmentRollIntent::DataPressure, })); let result = result.unwrap(); @@ -1577,6 +1645,7 @@ mod tests { sealed_at: 2000, new_replica_set: replica_set(), end_entry_id: None, + intent: SegmentRollIntent::Recovery, })); assert_eq!( @@ -1590,6 +1659,7 @@ mod tests { sealed_at: 2500, new_replica_set: replica_set(), end_entry_id: Some(EntryId(42000)), + intent: SegmentRollIntent::Recovery, })); assert!(result.is_ok()); @@ -1612,6 +1682,7 @@ mod tests { sealed_at: 2000, new_replica_set: replica_set(), end_entry_id: Some(EntryId(1000)), + intent: SegmentRollIntent::DataPressure, })); // Duplicate roll is rejected (end_offset already set) @@ -1620,6 +1691,7 @@ mod tests { sealed_at: 2500, new_replica_set: replica_set(), end_entry_id: Some(EntryId(42000)), + intent: SegmentRollIntent::DataPressure, })); assert_eq!(result, Ok(ApplyResult::Noop)); } diff --git a/src/control_plane/metadata/event.rs b/src/control_plane/metadata/event.rs index 6bc7e472..dc9cd0d8 100644 --- a/src/control_plane/metadata/event.rs +++ b/src/control_plane/metadata/event.rs @@ -1,5 +1,5 @@ use crate::control_plane::Replicas; -use crate::control_plane::consensus::multi_raft::SealContext; +use crate::control_plane::consensus::multi_raft::RollRequestContext; use crate::control_plane::membership::ShardGroupId; use crate::control_plane::metadata::consumer_group::GenerationId; use crate::control_plane::metadata::{EntryId, RangeId, SegmentId, TopicId}; @@ -42,7 +42,7 @@ pub struct SegmentRolled { impl SegmentRolled { pub fn into_command( self, - ctx: Option, + ctx: Option, shard_group_id: ShardGroupId, ) -> Vec { let start = self.end_entry_id.map_or(EntryId::MIN, |id| id + 1); @@ -251,13 +251,13 @@ impl SegmentsDeleted { } #[derive(Debug, Clone, PartialEq, Eq)] -pub struct SegmentSealCorrected { +pub struct SegmentBoundaryCorrected { pub segment_key: SegmentKey, pub replica_set: Replicas, pub committed_entry_id: Option, } -impl SegmentSealCorrected { +impl SegmentBoundaryCorrected { pub fn into_commands(self) -> Vec { if self.replica_set.is_empty() { vec![] @@ -281,7 +281,7 @@ pub enum ApplyResult { RangeMerged(RangeMerged), SegmentReassigned(SegmentReassigned), SegmentsDeleted(SegmentsDeleted), - SegmentSealCorrected(SegmentSealCorrected), + SegmentBoundaryCorrected(SegmentBoundaryCorrected), ConsumerGroupChanged(ConsumerGroupEpochSnapshot), TopicDeleted(TopicDeleted), Noop, @@ -295,7 +295,7 @@ impl_from_variant!( RangeMerged, SegmentReassigned, SegmentsDeleted, - SegmentSealCorrected, + SegmentBoundaryCorrected, ConsumerGroupChanged(ConsumerGroupEpochSnapshot), TopicDeleted ); diff --git a/src/control_plane/metadata/topic.rs b/src/control_plane/metadata/topic.rs index e1a594ad..2ddd28e9 100644 --- a/src/control_plane/metadata/topic.rs +++ b/src/control_plane/metadata/topic.rs @@ -194,7 +194,7 @@ impl TopicMeta { out.into_boxed_slice() } - /// Boundary-unknown sealed segments. These remain seal-end recovery + /// Boundary-unknown sealed segments. These remain boundary recovery /// candidates until a recovered boundary is committed to metadata, even if /// the crashed replica has rejoined by the next reconciliation pass. pub(crate) fn boundary_unknown_segments(&self) -> Box<[(SegmentKey, Vec)]> { @@ -349,7 +349,7 @@ impl TopicMeta { if r1.state != RangeState::Active || r2.state != RangeState::Active { return None; } - if !r1.mergeable_with(r2, now) { + if !r1.mergeable_with(r2) { return None; } let replica_set = r1 @@ -505,8 +505,6 @@ pub mod props { self.assert_delete_cascade(); for range in self.ranges.values() { range.assert_invariants(); - - self.assert_split_children_cooldown(range); } for group in self.consumer_groups.values() { group.assert_assignments(&self.active_ranges); @@ -576,24 +574,6 @@ pub mod props { } } - fn assert_split_children_cooldown(&self, range: &RangeMeta) { - let Some([left, right]) = range.split_into else { - return; - }; - if let Some(left_range) = self.ranges.get(&left) { - assert!( - left_range.seal_history.created_by_split_at.is_some(), - "split child missing created_by_split_at" - ); - } - if let Some(right_range) = self.ranges.get(&right) { - assert!( - right_range.seal_history.created_by_split_at.is_some(), - "split child missing created_by_split_at" - ); - } - } - fn assert_keyspace_coverage(&self) { if self.active_ranges.is_empty() { return; diff --git a/src/data_plane/messages/command.rs b/src/data_plane/messages/command.rs index d02f96a6..fbc7c120 100644 --- a/src/data_plane/messages/command.rs +++ b/src/data_plane/messages/command.rs @@ -1,4 +1,5 @@ use crate::control_plane::Replicas; +use crate::control_plane::metadata::SegmentRollIntent; use crate::data_plane::consumer_offset_management::ledger::ConsumerOffsetEntry; use crate::data_plane::consumer_offset_management::ledger::ConsumerOffsetUpdate; use crate::data_plane::consumer_offset_management::ledger::EpochSeal; @@ -151,6 +152,7 @@ pub struct RequestSegmentRoll { pub segment_key: SegmentKey, pub failed_nodes: Vec, pub end_entry_id: EntryId, + pub intent: SegmentRollIntent, } #[derive(Debug, Clone, BorshSerialize, BorshDeserialize)] diff --git a/src/data_plane/states/mod.rs b/src/data_plane/states/mod.rs index 13233a29..583d32f9 100644 --- a/src/data_plane/states/mod.rs +++ b/src/data_plane/states/mod.rs @@ -1,5 +1,5 @@ pub(crate) mod replication; -pub(crate) mod seal_request; +pub(crate) mod roll_request; pub(crate) mod segment; pub(crate) mod segment_store; pub(crate) mod types; diff --git a/src/data_plane/states/seal_request.rs b/src/data_plane/states/seal_request.rs deleted file mode 100644 index b3de0141..00000000 --- a/src/data_plane/states/seal_request.rs +++ /dev/null @@ -1,107 +0,0 @@ -//! Pending segment-roll requests from the write leader to the coordinator. -//! -//! A leader that can't replicate sends `RequestSegmentRoll` and tracks it until -//! `SegmentRollCommitted` clears it; the data plane re-sends timed-out requests. Owns the -//! dedup + timing bookkeeping; building/sending the request stays in the data -//! plane (it reads the live tracker's committed end). - -use std::collections::HashMap; -use std::time::Duration; -// tokio's runtime-aware Instant: real time in prod, virtualized under turmoil so -// the retry timeout fires on the sim clock (the data-plane worker runs on a tokio -// runtime in both — see CLAUDE.md "Testing"). Falls back to real time with no -// runtime, so the sync tests below still work. -use tokio::time::Instant; - -use crate::control_plane::NodeId; -use crate::data_plane::SegmentKey; - -struct PendingSegmentRollRequest { - sent_at: Instant, - failed_nodes: Vec, -} - -/// Segment-roll requests awaiting `SegmentRollCommitted`, keyed by segment. -#[derive(Default)] -pub(crate) struct PendingSegmentRollRequests { - requests: HashMap, -} - -impl PendingSegmentRollRequests { - /// Whether a request for `key` is already in flight (the dedup guard). - pub(crate) fn is_tracked(&self, key: &SegmentKey) -> bool { - self.requests.contains_key(key) - } - - /// Record a just-sent request. - pub(crate) fn track(&mut self, key: SegmentKey, failed_nodes: Vec, now: Instant) { - self.requests.insert( - key, - PendingSegmentRollRequest { - sent_at: now, - failed_nodes, - }, - ); - } - - /// Drop a request once its committed result lands. - pub(crate) fn clear(&mut self, key: &SegmentKey) { - self.requests.remove(key); - } - - /// Timed-out requests to re-send — `(key, failed_nodes)` — refreshing their - /// clock so each fires at most once per `timeout`. - pub(crate) fn take_due( - &mut self, - now: Instant, - timeout: Duration, - ) -> Vec<(SegmentKey, Vec)> { - let mut due = Vec::new(); - for (key, req) in &mut self.requests { - if now.duration_since(req.sent_at) >= timeout { - req.sent_at = now; - due.push((*key, req.failed_nodes.clone())); - } - } - due - } - - #[cfg(test)] - pub(crate) fn failed_nodes(&self, key: &SegmentKey) -> Option<&[NodeId]> { - self.requests.get(key).map(|r| r.failed_nodes.as_slice()) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::control_plane::metadata::{RangeId, SegmentId, TopicId}; - - fn seg() -> SegmentKey { - SegmentKey::new(TopicId(0), RangeId(0), SegmentId(0)) - } - - #[test] - fn track_then_clear() { - let mut p = PendingSegmentRollRequests::default(); - assert!(!p.is_tracked(&seg())); - p.track(seg(), vec![], Instant::now()); - assert!(p.is_tracked(&seg())); - p.clear(&seg()); - assert!(!p.is_tracked(&seg())); - } - - #[test] - fn take_due_returns_only_timed_out_then_refreshes() { - let mut p = PendingSegmentRollRequests::default(); - let t0 = Instant::now(); - let timeout = Duration::from_secs(5); - p.track(seg(), vec![NodeId::new("d")], t0); - - assert!(p.take_due(t0, timeout).is_empty(), "not yet due"); - let due = p.take_due(t0 + timeout, timeout); - assert_eq!(due, vec![(seg(), vec![NodeId::new("d")])]); - // Refreshed → not due again at the same instant. - assert!(p.take_due(t0 + timeout, timeout).is_empty()); - } -} diff --git a/src/data_plane/states/segment_store.rs b/src/data_plane/states/segment_store.rs index 68f9a542..778c2788 100644 --- a/src/data_plane/states/segment_store.rs +++ b/src/data_plane/states/segment_store.rs @@ -15,7 +15,7 @@ //! file are on disk; the stored location carries the `end_entry_id` so the //! resolver knows where the segment ends. //! -//! The store does **not** own the seal-handshake state (`pending_seal_requests`) +//! The store does **not** own the seal-handshake state (`pending_roll_requests`) //! or replication bookkeeping — those are about coordinator + peer protocols, //! not about where bytes live. It is intentionally a passive registry: it //! never originates a side effect, never talks to the network, never emits diff --git a/src/it/helpers.rs b/src/it/helpers.rs index e73c3f0e..f9360c5f 100644 --- a/src/it/helpers.rs +++ b/src/it/helpers.rs @@ -42,7 +42,7 @@ pub fn default_env(idx: u32, node_id: String, client_port: u16, cluster_port: u1 batch_max_bytes: 10 * 1024 * 1024, hot_cache_budget_bytes: 4 * 1024 * 1024 * 1024, hot_cache_pressure_watermark: 0.9, - seal_request_timeout_secs: 5, + segment_roll_request_timeout_secs: 5, // Short in tests so the orphan-GC sweep actually fires within a sim run; still // comfortably longer than re-fill + catch-up, so the lottery completes first. orphan_gc_interval_secs: 60, From 7e18452208f385c21c91093ff63c02a3b3f0f4df Mon Sep 17 00:00:00 2001 From: Migorithm Date: Sat, 18 Jul 2026 16:47:38 +0400 Subject: [PATCH 05/14] range load state applied to range meta --- src/control_plane/metadata/range.rs | 113 ++++++++++++---------------- 1 file changed, 50 insertions(+), 63 deletions(-) diff --git a/src/control_plane/metadata/range.rs b/src/control_plane/metadata/range.rs index 1b11bfa2..947999f8 100644 --- a/src/control_plane/metadata/range.rs +++ b/src/control_plane/metadata/range.rs @@ -1,12 +1,13 @@ use super::constants::*; use borsh::{BorshDeserialize, BorshSerialize}; -use std::collections::{HashMap, VecDeque}; +use std::collections::HashMap; use crate::control_plane::{ Replicas, metadata::{ EntryId, RangeId, SegmentId, SegmentMeta, SegmentMetaState, SplitRange, - command::RollSegment, error::MetadataError, + command::{RollSegment, SegmentRollIntent}, + error::MetadataError, }, }; @@ -30,7 +31,7 @@ pub struct RangeMeta { pub split_into: Option<[RangeId; 2]>, pub merged_into: Option, pub merged_from: Option<[RangeId; 2]>, - pub seal_history: RangeSealHistory, + pub load_state: RangeLoadState, } impl RangeMeta { @@ -56,7 +57,7 @@ impl RangeMeta { split_into: None, merged_into: None, merged_from: None, - seal_history: RangeSealHistory::default(), + load_state: RangeLoadState::Unclassified, } } @@ -79,8 +80,7 @@ impl RangeMeta { cmd.split_point.clone(), cmd.left_replica_set, cmd.created_at, - ) - .with_split_origin(cmd.created_at); + ); let right = RangeMeta::new( right_id, @@ -88,19 +88,11 @@ impl RangeMeta { self.keyspace_end.clone(), cmd.right_replica_set, cmd.created_at, - ) - .with_split_origin(cmd.created_at); + ); Ok((left, right)) } - fn with_split_origin(mut self, created_at: u64) -> Self { - self.seal_history = RangeSealHistory { - seal_timestamps: VecDeque::new(), - created_by_split_at: Some(created_at), - }; - self - } pub(crate) fn validate_active(&self) -> Result { if self.state != RangeState::Active { return Err(MetadataError::RangeNotActive); @@ -108,8 +100,8 @@ impl RangeMeta { self.active_segment.ok_or(MetadataError::RangeNotActive) } - pub(crate) fn should_split(&self, sealed_at: u64) -> bool { - self.seal_history.should_split(sealed_at) + pub(crate) fn should_split(&self) -> bool { + self.load_state.should_split() } pub(crate) fn correct_end_offset( @@ -178,7 +170,7 @@ impl RangeMeta { self.segments.insert(new_segment_id, new_segment); self.active_segment = Some(new_segment_id); - self.seal_history.record_seal(cmd.sealed_at); + self.load_state.apply(&cmd.intent); self.next_segment_id += 1; self.next_offset = start_offset; @@ -352,54 +344,47 @@ impl RangeMeta { result } - pub(crate) fn mergeable_with(&self, r2: &RangeMeta, now: u64) -> bool { - let r1_recent = self.seal_history.recent_seal_count(now); - let r2_recent = r2.seal_history.recent_seal_count(now); - - r1_recent == MERGE_SEAL_THRESHOLD && r2_recent == MERGE_SEAL_THRESHOLD + pub(crate) fn mergeable_with(&self, r2: &RangeMeta) -> bool { + self.load_state == RangeLoadState::Idle && r2.load_state == RangeLoadState::Idle } } -#[derive(Default, Debug, Clone, PartialEq, Eq, BorshSerialize, BorshDeserialize)] -pub struct RangeSealHistory { - pub seal_timestamps: VecDeque, - pub created_by_split_at: Option, +#[derive(Debug, Clone, PartialEq, Eq, BorshSerialize, BorshDeserialize)] +pub enum RangeLoadState { + Unclassified, + Idle, + Pressure { consecutive_rolls: u8 }, } -impl RangeSealHistory { - pub fn record_seal(&mut self, sealed_at: u64) { - // Proposal timestamps are captured before Raft serialization, so a - // delayed proposal can commit after one with a newer wall-clock value. - // Preserve the event times while keeping the committed history ordered. - let position = self - .seal_timestamps - .partition_point(|timestamp| *timestamp <= sealed_at); - self.seal_timestamps.insert(position, sealed_at); - - let newest = self.seal_timestamps.back().copied().unwrap_or(sealed_at); - let cutoff = newest.saturating_sub(MEASUREMENT_WINDOW_MS); - while self.seal_timestamps.front().is_some_and(|&t| t <= cutoff) { - self.seal_timestamps.pop_front(); - } - } - - pub fn seal_count(&self) -> usize { - self.seal_timestamps.len() - } - - pub fn should_split(&self, now: u64) -> bool { - if self.seal_count() < SPLIT_SEAL_THRESHOLD { - return false; - } - match self.created_by_split_at { - Some(split_at) => now.saturating_sub(split_at) >= SPLIT_COOLDOWN_MS, - None => true, +impl RangeLoadState { + pub fn apply(&mut self, intent: &SegmentRollIntent) { + match intent { + SegmentRollIntent::DataPressure => { + let consecutive_rolls = match self { + Self::Pressure { consecutive_rolls } => *consecutive_rolls + 1, + Self::Unclassified | Self::Idle => 1, + }; + *self = Self::Pressure { + consecutive_rolls: consecutive_rolls.min(SPLIT_SEAL_THRESHOLD), + }; + } + SegmentRollIntent::IdleMaintenance => *self = Self::Idle, + SegmentRollIntent::ReplicationFailure | SegmentRollIntent::Recovery => {} + SegmentRollIntent::BoundaryCorrection => { + debug_assert!( + false, + "boundary correction applied as an active segment roll" + ); + } } } - pub fn recent_seal_count(&self, now: u64) -> usize { - let cutoff = now.saturating_sub(MEASUREMENT_WINDOW_MS); - self.seal_timestamps.iter().filter(|&&t| t > cutoff).count() + pub fn should_split(&self) -> bool { + matches!( + self, + Self::Pressure{ consecutive_rolls } + if *consecutive_rolls >= SPLIT_SEAL_THRESHOLD + ) } } @@ -424,7 +409,7 @@ pub mod props { self.split_into.is_none() || self.merged_into.is_none(), "range both split and merged" ); - self.assert_seal_history(); + self.assert_load_state(); self.assert_offset_chain(); self.assert_retention_prefix(); } @@ -495,10 +480,12 @@ pub mod props { } } } - fn assert_seal_history(&self) { - let ts = &self.seal_history.seal_timestamps; - for i in 1..ts.len() { - assert!(ts[i - 1] <= ts[i], "seal_history timestamps not sorted"); + fn assert_load_state(&self) { + if let RangeLoadState::Pressure { consecutive_rolls } = &self.load_state { + assert!( + *consecutive_rolls > 0 && *consecutive_rolls <= SPLIT_SEAL_THRESHOLD, + "pressure streak outside its valid range", + ); } } From 4cfcea507c8232b063b185272e87f7e7e3ecbfc3 Mon Sep 17 00:00:00 2001 From: Migorithm Date: Sat, 18 Jul 2026 16:47:58 +0400 Subject: [PATCH 06/14] SegmentRollIntent --- src/control_plane/metadata/command.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/control_plane/metadata/command.rs b/src/control_plane/metadata/command.rs index 83ca9e03..ae4e77e8 100644 --- a/src/control_plane/metadata/command.rs +++ b/src/control_plane/metadata/command.rs @@ -21,6 +21,15 @@ pub struct CreateTopic { pub created_at: u64, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, BorshSerialize, BorshDeserialize)] +pub enum SegmentRollIntent { + DataPressure, + IdleMaintenance, + ReplicationFailure, + Recovery, + BoundaryCorrection, +} + #[derive(Debug, Clone, PartialEq, Eq, BorshSerialize, BorshDeserialize)] pub struct RollSegment { pub segment_key: SegmentKey, @@ -30,6 +39,7 @@ pub struct RollSegment { /// the actual committed offset. Corrected later via `correct_end_offset` /// or D5 sealed segment repair. pub end_entry_id: Option, + pub intent: SegmentRollIntent, } #[derive(Debug, Clone, PartialEq, Eq, BorshSerialize, BorshDeserialize)] From 356f5ba1128a5434713fe3ec33089ef3cae950e9 Mon Sep 17 00:00:00 2001 From: Migorithm Date: Sat, 18 Jul 2026 16:48:38 +0400 Subject: [PATCH 07/14] rm: MEASUREMENT_WINDOW_MS, SPLIT_COOLDOWN_MS, MERGE_SEAL_THRESHOLD MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We no longer need those constants because split/merge decisions are based on committed segment-roll history, not sampled wall-clock activity. MEASUREMENT_WINDOW_MS: Previously, load had to be measured over a time window: “How much traffic did this range receive during the last N milliseconds?” Now every successful segment roll carries its cause: - DataPressure means the segment filled because of traffic. - IdleMaintenance means it aged out without filling. - Failure, recovery, and correction rolls are neutral. The committed roll itself is the measurement. A separate time window would add timing sensitivity without contributing information SPLIT_COOLDOWN_MS: After a split, each child starts Unclassified with a fresh active segment. It must independently accumulate a complete pressure streak before it can split again. That requires real data to fill multiple segments, which acts as a workload-based cooldown. MERGE_SEAL_THRESHOLD: Merging does not need a numeric streak because there is already a strong physical idle signal: a child’s active segment reaches the age limit and rolls with IdleMaintenance --- src/control_plane/metadata/constants.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/control_plane/metadata/constants.rs b/src/control_plane/metadata/constants.rs index c9eed2a4..20e69fdf 100644 --- a/src/control_plane/metadata/constants.rs +++ b/src/control_plane/metadata/constants.rs @@ -5,7 +5,4 @@ pub const KEYSPACE_MIN: &[u8] = &[]; pub const KEYSPACE_MAX: &[u8] = &[0xFF]; // --- Hot Range Detection Constants --- -pub const SPLIT_SEAL_THRESHOLD: usize = 3; -pub const MEASUREMENT_WINDOW_MS: u64 = 300_000; // 5 min sliding window -pub const SPLIT_COOLDOWN_MS: u64 = 300_000; // 5 min cooldown after a split -pub const MERGE_SEAL_THRESHOLD: usize = 0; // both ranges must be fully idle +pub const SPLIT_SEAL_THRESHOLD: u8 = 3; From a4410657f41f7d6df4c56eb22d44f87269c74f4e Mon Sep 17 00:00:00 2001 From: Migorithm Date: Sat, 18 Jul 2026 17:10:37 +0400 Subject: [PATCH 08/14] segment meta sealed --- src/control_plane/consensus/raft/states/metadata_state.rs | 8 ++++---- src/control_plane/metadata/event.rs | 8 ++++---- src/data_plane/messages/command.rs | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/control_plane/consensus/raft/states/metadata_state.rs b/src/control_plane/consensus/raft/states/metadata_state.rs index 27e9b460..f8328461 100644 --- a/src/control_plane/consensus/raft/states/metadata_state.rs +++ b/src/control_plane/consensus/raft/states/metadata_state.rs @@ -1357,9 +1357,9 @@ mod tests { let range = &sm.get_topic(&tid).unwrap().ranges[&RangeId(0)]; assert_eq!( range.load_state, - RangeLoadState::Pressure(RangePressure { + RangeLoadState::Pressure { consecutive_rolls: 2 - }) + } ); } @@ -1408,9 +1408,9 @@ mod tests { let range = &sm.get_topic(&tid).unwrap().ranges[&RangeId(0)]; assert_eq!( range.load_state, - RangeLoadState::Pressure(RangePressure { + RangeLoadState::Pressure { consecutive_rolls: 1 - }) + } ); } diff --git a/src/control_plane/metadata/event.rs b/src/control_plane/metadata/event.rs index dc9cd0d8..0c137427 100644 --- a/src/control_plane/metadata/event.rs +++ b/src/control_plane/metadata/event.rs @@ -6,8 +6,8 @@ use crate::control_plane::metadata::{EntryId, RangeId, SegmentId, TopicId}; use crate::data_plane::SegmentKey; use crate::data_plane::consumer_offset_management::ledger::{ConsumerOffsetKey, EpochSeal}; use crate::data_plane::messages::command::{ - AssignSegmentCatchUp, DataPlanePeerMessage, DeleteSegments, PlaceSegment, SegmentRollCommitted, - SegmentSealed, + AssignSegmentCatchUp, DataPlanePeerMessage, DeleteSegments, PlaceSegment, SegmentMetaSealed, + SegmentRollCommitted, }; use crate::data_plane::transport::command::DataTransportCommand; use crate::impl_from_variant; @@ -140,7 +140,7 @@ impl RangeSplit { { cmds.push(DataTransportCommand::send_to_targets( parent_replica_set.0, - SegmentSealed { + SegmentMetaSealed { segment_key: parent_key, committed_entry_id: None, }, @@ -264,7 +264,7 @@ impl SegmentBoundaryCorrected { } else { vec![DataTransportCommand::send_to_targets( self.replica_set.0, - SegmentSealed { + SegmentMetaSealed { segment_key: self.segment_key, committed_entry_id: self.committed_entry_id, }, diff --git a/src/data_plane/messages/command.rs b/src/data_plane/messages/command.rs index fbc7c120..db5576fc 100644 --- a/src/data_plane/messages/command.rs +++ b/src/data_plane/messages/command.rs @@ -163,7 +163,7 @@ pub struct SegmentRollCommitted { } #[derive(Debug, Clone, BorshSerialize, BorshDeserialize)] -pub struct SegmentSealed { +pub struct SegmentMetaSealed { pub segment_key: SegmentKey, pub committed_entry_id: Option, } @@ -292,7 +292,7 @@ pub enum DataPlanePeerMessage { AdvanceReplicaCommit(AdvanceReplicaCommit), RequestSegmentRoll(RequestSegmentRoll), SegmentRollCommitted(SegmentRollCommitted), - SegmentSealed(SegmentSealed), + SegmentMetaSealed(SegmentMetaSealed), AssignSegmentCatchUp(AssignSegmentCatchUp), RequestCatchUpEntries(RequestCatchUpEntries), CatchUpEntries(CatchUpEntries), @@ -318,7 +318,7 @@ impl_from_variant!( AdvanceReplicaCommit, RequestSegmentRoll, SegmentRollCommitted, - SegmentSealed, + SegmentMetaSealed, AssignSegmentCatchUp, RequestCatchUpEntries, CatchUpEntries, From 342cdb748f5f6d336e52a214ceb9d12dfd73c7a6 Mon Sep 17 00:00:00 2001 From: Migorithm Date: Sat, 18 Jul 2026 17:13:58 +0400 Subject: [PATCH 09/14] state change --- src/control_plane/consensus/raft/state.rs | 18 ++- src/data_plane/state.rs | 170 +++++++++++++++------- 2 files changed, 129 insertions(+), 59 deletions(-) diff --git a/src/control_plane/consensus/raft/state.rs b/src/control_plane/consensus/raft/state.rs index 54fdef91..f4ac6ae1 100644 --- a/src/control_plane/consensus/raft/state.rs +++ b/src/control_plane/consensus/raft/state.rs @@ -13,7 +13,7 @@ use crate::control_plane::metadata::command::DeleteSegments; use crate::control_plane::metadata::event::ApplyResult; use crate::control_plane::metadata::{ ConsumerGroupAssignment, ConsumerMemberId, MetadataCommand, ReassignSegment, RollSegment, - TopicId, TopicMeta, TopicStats, + SegmentRollIntent, TopicId, TopicMeta, TopicStats, }; use crate::control_plane::{NodeId, Replicas}; use crate::data_plane::SegmentKey; @@ -373,7 +373,7 @@ impl Raft { } /// Repair this group's segments whose replica set still names a dead node: - /// - active, one replica died → stash for seal-end recovery (rolled later) + /// - active, one replica died → stash for boundary recovery (rolled later) /// - active, multiple replicas died → roll now with an unknown end (`RollSegment`) /// - sealed (known end) → swap the replica set (`ReassignSegment`); catch-up refills /// @@ -423,6 +423,7 @@ impl Raft { sealed_at: now_ms(), new_replica_set, end_entry_id: None, + intent: SegmentRollIntent::Recovery, } .into() }, @@ -607,7 +608,7 @@ impl Raft { } /// Drain the leaderless segments found by `reconcile_segments` — the actor - /// drives seal-end recovery (poll survivors, seal at the recovered end). + /// drives boundary recovery (poll survivors, seal at the recovered end). pub(crate) fn take_leaderless_segments(&mut self) -> Vec<(SegmentKey, Vec)> { self.consensus.take_leaderless_segments() } @@ -1289,7 +1290,7 @@ impl Raft { shard_group_id: self.shard_group_id, result, log_index: index, - seal_context: None, + roll_context: None, }); } Err(e) => tracing::error!( @@ -3553,6 +3554,7 @@ mod tests { sealed_at: 2000, new_replica_set: Replicas::new(vec![node("node-1")]), end_entry_id: Some(100.into()), + intent: SegmentRollIntent::DataPressure, }) .into(), ) @@ -3619,6 +3621,7 @@ mod tests { spare.clone(), ]), end_entry_id: Some(100.into()), + intent: SegmentRollIntent::DataPressure, }) .into(), ) @@ -3687,6 +3690,7 @@ mod tests { sealed_at: 2000, new_replica_set: Replicas::new(sealed_set), end_entry_id: Some(100.into()), + intent: SegmentRollIntent::DataPressure, }) .into(), ) @@ -3816,6 +3820,7 @@ mod tests { sealed_at: 2000, new_replica_set: Replicas::new(vec![node("node-1")]), end_entry_id: Some(100.into()), + intent: SegmentRollIntent::DataPressure, }) .into(), ) @@ -3914,7 +3919,7 @@ mod tests { } #[test] - fn reconcile_redrives_seal_recovery_for_unknown_end() { + fn reconcile_redrives_boundary_recovery_for_unknown_end() { use crate::control_plane::metadata::{RangeId, SegmentId, TopicId}; // Same shape, but seal with end_entry_id = None — a SWIM-death-style seal @@ -3928,6 +3933,7 @@ mod tests { sealed_at: 2000, new_replica_set: Replicas::new(vec![node("y"), node("z"), node("node-1")]), end_entry_id: None, + intent: SegmentRollIntent::Recovery, }) .into(), ) @@ -3996,7 +4002,7 @@ mod tests { let live = live_set(&["node-1", "y", "z"]); // leader x crashed raft.reconcile_segments(&live); - // No immediate roll — the segment is stashed for seal-end recovery. + // No immediate roll — the segment is stashed for boundary recovery. let after = proposals_after_become_leader(&raft); let rolls = after[before..] .iter() diff --git a/src/data_plane/state.rs b/src/data_plane/state.rs index b5e4171c..d792792b 100644 --- a/src/data_plane/state.rs +++ b/src/data_plane/state.rs @@ -15,7 +15,7 @@ use super::segment_writer::SegmentAppender; use super::states::replication::PendingReplicationBatch; use super::states::replication::ReplicationState; -use super::states::seal_request::PendingSegmentRollRequests; +use super::states::roll_request::PendingSegmentRollRequests; use super::states::segment_store::SegmentStore; use super::timer::DataPlaneTimeoutCallback; use super::transport::command::DataTransportCommand; @@ -25,6 +25,7 @@ use crate::config::DataNodeConfig; use crate::control_plane::consensus::messages::{MultiRaftActorCommand, ProposeSegmentRoll}; use crate::control_plane::membership::ShardGroupId; use crate::control_plane::metadata::EntryId; +use crate::control_plane::metadata::command::SegmentRollIntent; use crate::control_plane::{NodeId, Replicas}; use crate::data_plane::EntryPayload; use crate::data_plane::consumer_offset_management::ConsumerOffsetManager; @@ -72,7 +73,7 @@ pub struct DataPlane { replication: ReplicationState, - pending_seal_requests: PendingSegmentRollRequests, + pending_roll_requests: PendingSegmentRollRequests, /// In-progress catch-up receives (replacement side), keyed by segment. pending_catch_ups: HashMap, @@ -88,7 +89,7 @@ pub struct DataPlane { /// Consulted on a catch-up assignment to decide full-match / delta / /// full-copy and to skip transfer when we already hold the segment. recovered: LocalInventory, - pending_seals: std::collections::HashSet, + pending_boundary_corrections: std::collections::HashSet, pressure_checkpoints_in_flight: BTreeSet, consumer_offsets: ConsumerOffsetManager, } @@ -117,13 +118,13 @@ impl DataPlane { buffer_byte_count: 0, needs_flush: false, replication: ReplicationState::default(), - pending_seal_requests: PendingSegmentRollRequests::default(), pending_catch_ups: HashMap::new(), cold_read_handoff_sender, self_tx, out, recovered, - pending_seals: std::collections::HashSet::new(), + pending_roll_requests: PendingSegmentRollRequests::default(), + pending_boundary_corrections: std::collections::HashSet::new(), pressure_checkpoints_in_flight: BTreeSet::new(), consumer_offsets: ConsumerOffsetManager::new(offset_ledger), } @@ -175,7 +176,7 @@ impl DataPlane { if self.should_flush() { self.flush_batch(); } - self.enqueue_timed_out_seal_retries(); + self.enqueue_timed_out_roll_retries(); self.out.flush().await; #[cfg(any(test, debug_assertions))] @@ -487,7 +488,7 @@ impl DataPlane { } C::AdvanceReplicaCommit(cmd) => self.handle_commit_advance(cmd), C::SegmentRollCommitted(message) => self.handle_segment_roll_committed(message), - C::SegmentSealed(cmd) => self.handle_segment_sealed(cmd), + C::SegmentMetaSealed(cmd) => self.handle_segment_meta_sealed(cmd), // Pass-through to MultiRaftActor — RequestSegmentRoll is a control plane // message that shares the data transport wire format. The @@ -496,7 +497,7 @@ impl DataPlane { C::RequestSegmentRoll(cmd) => { self.out .store_coordinator_cmd(MultiRaftActorCommand::ProposeSegmentRoll( - ProposeSegmentRoll { request: cmd }, + ProposeSegmentRoll(cmd), )); } @@ -520,7 +521,7 @@ impl DataPlane { .store_coordinator_cmd(MultiRaftActorCommand::SegmentCaughtUp(cmd)); } - C::RequestDurableSegmentEnd(cmd) => self.handle_seal_boundary_query(cmd), + C::RequestDurableSegmentEnd(cmd) => self.handle_durable_end_query(cmd), // Pass-through to the local MultiRaftActor C::DurableSegmentEndReported(cmd) => { self.out @@ -572,7 +573,7 @@ impl DataPlane { } } - fn handle_seal_boundary_query(&mut self, cmd: RequestDurableSegmentEnd) { + fn handle_durable_end_query(&mut self, cmd: RequestDurableSegmentEnd) { let durable_end = self.durable_end(&cmd.segment_key); self.out .store_transport_cmd(DataTransportCommand::send_to_targets( @@ -951,7 +952,7 @@ impl DataPlane { .push((seq, ReplicationTimer::timeout(cmd.segment_key))); } - self.check_pending_seal(cmd.segment_key); + self.check_pending_boundary_correction(cmd.segment_key); } fn commit_consumer_offset(&mut self, cmd: CommitConsumerOffset) { @@ -1176,7 +1177,7 @@ impl DataPlane { // TODO refactor fn handle_segment_roll_committed(&mut self, cmd: SegmentRollCommitted) { - self.pending_seal_requests.clear(&cmd.old_segment_key); + self.pending_roll_requests.remove(&cmd.old_segment_key); let Some(old_tracker) = self.segments.get(&cmd.old_segment_key) else { return; }; @@ -1196,7 +1197,7 @@ impl DataPlane { self.out .store_transport_cmd(DataTransportCommand::send_to_targets( old_tracker.followers().to_vec(), - SegmentSealed { + SegmentMetaSealed { segment_key: cmd.old_segment_key, committed_entry_id, }, @@ -1255,7 +1256,7 @@ impl DataPlane { self.needs_flush = true; } - fn handle_segment_sealed(&mut self, cmd: SegmentSealed) { + fn handle_segment_meta_sealed(&mut self, cmd: SegmentMetaSealed) { match cmd.committed_entry_id { // Determine the end offset and notify the coordinator None => { @@ -1263,7 +1264,7 @@ impl DataPlane { && tracker.role() == SegmentRole::Leader { if !tracker.is_fully_committed() { - self.pending_seals.insert(cmd.segment_key); + self.pending_boundary_corrections.insert(cmd.segment_key); return; // Defer and exit early } @@ -1279,6 +1280,7 @@ impl DataPlane { segment_key: cmd.segment_key, failed_nodes: vec![], end_entry_id, + intent: SegmentRollIntent::BoundaryCorrection, } .into(), }); @@ -1303,8 +1305,8 @@ impl DataPlane { self.retire_old_segment(cmd.segment_key); } - fn check_pending_seal(&mut self, segment_key: SegmentKey) { - if !self.pending_seals.contains(&segment_key) { + fn check_pending_boundary_correction(&mut self, segment_key: SegmentKey) { + if !self.pending_boundary_corrections.contains(&segment_key) { return; } @@ -1328,12 +1330,13 @@ impl DataPlane { segment_key, failed_nodes: vec![], end_entry_id: actual_end_offset, + intent: SegmentRollIntent::BoundaryCorrection, } .into(), }); self.retire_old_segment(segment_key); - self.pending_seals.remove(&segment_key); + self.pending_boundary_corrections.remove(&segment_key); } // Drop the sealed segment's live tracker and checkpoint its committed @@ -1375,7 +1378,7 @@ impl DataPlane { && tracker.is_fully_committed() { self.out.store_checkpoint(tracker.checkpoint(segment_key)); - self.enqueue_seal_request(segment_key); + self.enqueue_roll_request(segment_key, SegmentRollIntent::DataPressure); } } @@ -1383,12 +1386,16 @@ impl DataPlane { let aged: Box<[SegmentKey]> = self .segments .iter() - .filter(|(_, t)| t.role() == SegmentRole::Leader && t.age_limit_reached(max_age)) + .filter(|(_, tracker)| { + tracker.role() == SegmentRole::Leader + && tracker.age_limit_reached(max_age) + && !tracker.size_limit_reached(self.config.segment_size_limit) + }) .map(|(&k, _)| k) .collect(); for key in aged { - self.enqueue_seal_request(key); + self.enqueue_roll_request(key, SegmentRollIntent::IdleMaintenance); } } @@ -1411,15 +1418,20 @@ impl DataPlane { segment_key, failed_nodes: failed_nodes.clone(), end_entry_id: tracker.committed_entry_id(), + intent: SegmentRollIntent::ReplicationFailure, } .into(), }); - self.pending_seal_requests - .track(segment_key, failed_nodes, tokio::time::Instant::now()); + self.pending_roll_requests.track( + segment_key, + failed_nodes, + SegmentRollIntent::ReplicationFailure, + tokio::time::Instant::now(), + ); } - fn enqueue_seal_request(&mut self, segment_key: SegmentKey) { - if self.pending_seal_requests.is_tracked(&segment_key) { + fn enqueue_roll_request(&mut self, segment_key: SegmentKey, intent: SegmentRollIntent) { + if self.pending_roll_requests.is_tracked(&segment_key) { return; } let Some(tracker) = self.segments.get(&segment_key) else { @@ -1436,11 +1448,12 @@ impl DataPlane { segment_key, failed_nodes: vec![], end_entry_id: tracker.committed_entry_id(), + intent, } .into(), }); - self.pending_seal_requests - .track(segment_key, vec![], tokio::time::Instant::now()); + self.pending_roll_requests + .track(segment_key, vec![], intent, tokio::time::Instant::now()); } // Fast Path should_flush check @@ -1535,10 +1548,10 @@ impl DataPlane { && tracker.is_fully_committed() { self.out.store_checkpoint(tracker.checkpoint(key)); - self.enqueue_seal_request(key); + self.enqueue_roll_request(key, SegmentRollIntent::DataPressure); } - self.check_pending_seal(key); + self.check_pending_boundary_correction(key); } for pending_repl in segment_batches { @@ -1617,12 +1630,12 @@ impl DataPlane { } } - fn enqueue_timed_out_seal_retries(&mut self) { - let due = self.pending_seal_requests.take_due( + fn enqueue_timed_out_roll_retries(&mut self) { + let due = self.pending_roll_requests.take_due( tokio::time::Instant::now(), - self.config.seal_request_timeout, + self.config.segment_roll_request_timeout, ); - for (segment_key, failed_nodes) in due { + for (segment_key, failed_nodes, intent) in due { let Some(tracker) = self.segments.get(&segment_key) else { continue; }; @@ -1637,6 +1650,7 @@ impl DataPlane { segment_key, failed_nodes, end_entry_id: tracker.committed_entry_id(), + intent, } .into(), }); @@ -1720,6 +1734,22 @@ mod tests { SegmentKey::new(TopicId(1), RangeId(0), SegmentId(0)) } + fn requested_roll_intents(dp: &DataPlane) -> Vec { + dp.out + .transport_cmds + .iter() + .filter_map(|command| { + let DataTransportCommand::SendToCoordinator(send) = command else { + return None; + }; + let DataPlanePeerMessage::RequestSegmentRoll(request) = &send.message else { + return None; + }; + Some(request.intent) + }) + .collect() + } + fn receive_peer_message(message: DataPlanePeerMessage) -> DataPlaneCommand { let from = match &message { DataPlanePeerMessage::ReplicateSegmentEntries(command) => { @@ -1746,7 +1776,7 @@ mod tests { DataPlanePeerMessage::PlaceSegment(_) | DataPlanePeerMessage::AdvanceReplicaCommit(_) | DataPlanePeerMessage::SegmentRollCommitted(_) - | DataPlanePeerMessage::SegmentSealed(_) + | DataPlanePeerMessage::SegmentMetaSealed(_) | DataPlanePeerMessage::AssignSegmentCatchUp(_) | DataPlanePeerMessage::CatchUpEntries(_) | DataPlanePeerMessage::CatchUpEntriesSent(_) @@ -1919,7 +1949,7 @@ mod tests { let dir = tempfile::tempdir().unwrap(); let mut dp = make_data_plane(&dir); dp.process(assign_segment(test_key(), vec![test_node_id()])); - dp.process_peer(DataPlanePeerMessage::SegmentSealed(SegmentSealed { + dp.process_peer(DataPlanePeerMessage::SegmentMetaSealed(SegmentMetaSealed { segment_key: test_key(), committed_entry_id: None, })); @@ -2015,7 +2045,7 @@ mod tests { | DataPlanePeerMessage::AdvanceReplicaCommit(_) | DataPlanePeerMessage::RequestSegmentRoll(_) | DataPlanePeerMessage::SegmentRollCommitted(_) - | DataPlanePeerMessage::SegmentSealed(_) + | DataPlanePeerMessage::SegmentMetaSealed(_) | DataPlanePeerMessage::AssignSegmentCatchUp(_) | DataPlanePeerMessage::RequestCatchUpEntries(_) | DataPlanePeerMessage::CatchUpEntries(_) @@ -2212,7 +2242,7 @@ mod tests { batch_max_bytes: TEST_BATCH_MAX_BYTES, hot_cache_budget_bytes: 0, hot_cache_pressure_watermark: 0.9, - seal_request_timeout: std::time::Duration::from_secs(5), + segment_roll_request_timeout: std::time::Duration::from_secs(5), orphan_gc_interval: std::time::Duration::from_secs(300), data_dir: dir, } @@ -2998,7 +3028,7 @@ mod tests { } #[test] - fn replication_timeout_generates_seal_request() { + fn replication_timeout_generates_roll_request() { let dir = tempfile::tempdir().unwrap(); let mut dp = make_data_plane(&dir); let follower = NodeId::new("follower-1"); @@ -3031,6 +3061,10 @@ mod tests { .iter() .any(|c| matches!(c, DataTransportCommand::SendToCoordinator(..))) ); + assert_eq!( + requested_roll_intents(&dp), + vec![SegmentRollIntent::ReplicationFailure] + ); } #[test] @@ -3079,7 +3113,7 @@ mod tests { let seq = dp.out.repl_schedules[0].0; dp.handle_replication_timeout(test_key(), seq); - let failed = dp.pending_seal_requests.failed_nodes(&test_key()).unwrap(); + let failed = dp.pending_roll_requests.failed_nodes(&test_key()).unwrap(); assert!(failed.contains(&follower)); } @@ -3144,12 +3178,12 @@ mod tests { } #[test] - fn enqueue_seal_request_sends_to_coordinator() { + fn enqueue_roll_request_sends_to_coordinator() { let dir = tempfile::tempdir().unwrap(); let mut dp = make_data_plane(&dir); dp.handle_command(assign_segment(test_key(), vec![test_node_id()])); - dp.enqueue_seal_request(test_key()); + dp.enqueue_roll_request(test_key(), SegmentRollIntent::DataPressure); assert!( dp.out @@ -3157,19 +3191,49 @@ mod tests { .iter() .any(|c| matches!(c, DataTransportCommand::SendToCoordinator(..))) ); - assert!(dp.pending_seal_requests.is_tracked(&test_key())); + assert!(dp.pending_roll_requests.is_tracked(&test_key())); + assert_eq!( + requested_roll_intents(&dp), + vec![SegmentRollIntent::DataPressure] + ); + } + + #[test] + fn age_limit_requests_idle_maintenance_roll() { + let dir = tempfile::tempdir().unwrap(); + let mut dp = make_data_plane(&dir); + dp.handle_command(assign_segment(test_key(), vec![test_node_id()])); + + dp.enqueue_seal_for_aged_segments(std::time::Duration::ZERO); + + assert_eq!( + requested_roll_intents(&dp), + vec![SegmentRollIntent::IdleMaintenance] + ); + } + + #[test] + fn size_limit_takes_precedence_over_age_limit() { + let dir = tempfile::tempdir().unwrap(); + let mut dp = make_data_plane(&dir); + dp.config.segment_size_limit = 0; + dp.handle_command(assign_segment(test_key(), vec![test_node_id()])); + + dp.enqueue_seal_for_aged_segments(std::time::Duration::ZERO); + + assert!(requested_roll_intents(&dp).is_empty()); } #[test] - fn enqueue_seal_request_deduplicates() { + fn enqueue_roll_request_deduplicates() { let dir = tempfile::tempdir().unwrap(); let mut dp = make_data_plane(&dir); dp.handle_command(assign_segment(test_key(), vec![test_node_id()])); - dp.enqueue_seal_request(test_key()); - dp.enqueue_seal_request(test_key()); + dp.enqueue_roll_request(test_key(), SegmentRollIntent::DataPressure); + dp.enqueue_roll_request(test_key(), SegmentRollIntent::DataPressure); - // Count only seal requests — `handle_place_segment` also emits a + // Count only roll requests — `handle_place_segment` also emits a // SegmentPlaced via SendToCoordinator, which is not a seal. assert_eq!( dp.out @@ -3186,7 +3250,7 @@ mod tests { } #[test] - fn enqueue_seal_request_skips_follower() { + fn enqueue_roll_request_skips_follower() { let dir = tempfile::tempdir().unwrap(); let mut dp = make_data_plane(&dir); let leader = NodeId::new("leader-node"); @@ -3202,18 +3266,18 @@ mod tests { .into(), )); - dp.enqueue_seal_request(test_key()); + dp.enqueue_roll_request(test_key(), SegmentRollIntent::DataPressure); assert!(dp.out.transport_cmds.is_empty()); - assert!(!dp.pending_seal_requests.is_tracked(&test_key())); + assert!(!dp.pending_roll_requests.is_tracked(&test_key())); } #[test] - fn enqueue_seal_request_skips_unknown_segment() { + fn enqueue_roll_request_skips_unknown_segment() { let dir = tempfile::tempdir().unwrap(); let mut dp = make_data_plane(&dir); - dp.enqueue_seal_request(test_key()); + dp.enqueue_roll_request(test_key(), SegmentRollIntent::DataPressure); assert!(dp.out.transport_cmds.is_empty()); } @@ -3440,7 +3504,7 @@ mod tests { sparse.put_batch(index_entries).unwrap(); dp.handle_command(receive_peer_message( - SegmentSealed { + SegmentMetaSealed { segment_key: test_key(), committed_entry_id: None, } @@ -4314,7 +4378,7 @@ mod tests { /// A `RequestDurableSegmentEnd` is answered with our durable extent, addressed back to /// the coordinator that owns this gather. #[test] - fn seal_boundary_query_replies_with_durable_end() { + fn durable_end_query_replies_with_durable_end() { let dir = tempfile::tempdir().unwrap(); let mut dp = make_data_plane_with(&dir, inventory_with(test_key(), 4)); From 7c9f69f235759e967b342388cc1c1383006f86dd Mon Sep 17 00:00:00 2001 From: Migorithm Date: Sat, 18 Jul 2026 17:22:15 +0400 Subject: [PATCH 10/14] age -> inactivity based merge trigger --- .../data-plane/d3_segment_roll_integration.md | 16 +++++----- docs/data-plane/d7_retention_gc.md | 3 +- src/config.rs | 26 ++++++++-------- src/data_plane/actor.rs | 8 ++--- src/data_plane/messages/command.rs | 4 +-- src/data_plane/state.rs | 24 +++++++-------- src/data_plane/states/segment/tracker.rs | 30 ++++++++++++++----- src/data_plane/timer.rs | 22 +++++++------- src/it/e2e/client_protocol.rs | 18 +++++------ src/it/helpers.rs | 4 +-- 10 files changed, 84 insertions(+), 71 deletions(-) diff --git a/docs/data-plane/d3_segment_roll_integration.md b/docs/data-plane/d3_segment_roll_integration.md index d0f07659..5292edad 100644 --- a/docs/data-plane/d3_segment_roll_integration.md +++ b/docs/data-plane/d3_segment_roll_integration.md @@ -1,6 +1,6 @@ # Phase D3: Segment Lifecycle Integration -**Goal:** Connect data plane storage to metadata consensus. Three classes of integration: (1) seal triggers — size, age, replication failure, node death — all converging on a single segment-roll committed through the coordinator, (2) lifecycle event propagation — topic creation, segment roll, range split, and range merge commits turn into segment assignments delivered to the data plane, (3) coordinator routing — the segment leader resolves and reaches the coordinator. +**Goal:** Connect data plane storage to metadata consensus. Three classes of integration: (1) seal triggers — size, inactivity, replication failure, node death — all converging on a single segment-roll committed through the coordinator, (2) lifecycle event propagation — topic creation, segment roll, range split, and range merge commits turn into segment assignments delivered to the data plane, (3) coordinator routing — the segment leader resolves and reaches the coordinator. **Depends on:** Phase D2 (segment replication), metadata control plane (shard leader gossip via SWIM). @@ -37,7 +37,7 @@ Four triggers, one response: seal the current segment and open a new one via a s |---|---|---|---| | Replication failure | Follower timeout | Sub-second (replication timeout) | Segment leader sends a seal request | | Segment size | ~1GB threshold | On each flush | Segment leader sends a seal request | -| Segment age | Configurable max age | Periodic ticker (~60s) | Segment leader sends a seal request | +| Segment inactivity | Configurable idle timeout | Periodic ticker (~60s) | Segment leader sends a seal request | | Node death | SWIM protocol | ~6-7s | Coordinator proposes the roll directly | ### Trigger 1: Replication Failure (D2) @@ -61,21 +61,21 @@ flush completes The reported end entry id is the segment's last committed entry, which may lag the write cursor by in-flight entries — those are replayed into the new segment when the seal response arrives. This replay relies on the coordinator keeping the seal requester as the new segment's primary: the requester is alive (it just sent the request) and holds the uncommitted tail. The size check runs after every flush, not on a timer. -### Trigger 3: Segment Age +### Trigger 3: Segment Inactivity -When a segment has been active longer than the configured max age (e.g., 24 hours), the segment leader sends a seal request. This bounds segment lifetime for low-traffic topics — it prevents stale long-lived segments and simplifies retention and WAL management. +When a segment has received no newly committed data for the configured idle timeout, the segment leader sends a seal request. This turns inactivity into a committed load signal: a quiet range can later merge with an equally quiet sibling, while occasional committed traffic keeps it active. ``` -periodic age ticker fires (~60s) +periodic idle ticker fires (~60s) │ ├── for each active segment this node leads: - │ age beyond configured max? + │ time since last committed data beyond idle timeout? │ └── yes → seal request (failed nodes = none, │ end entry id = last committed entry) └── no → continue ``` -Unlike the size and replication triggers, the age check is driven by a periodic ticker, not by writes. It is the only trigger that fires without a write — necessary for idle segments. +Unlike the size and replication triggers, the inactivity check is driven by a periodic ticker. Successful committed progress refreshes the activity timestamp; stale or duplicate commit notifications do not. The ticker can therefore roll a truly idle segment even when no new write arrives to run a check. ### Trigger 4: SWIM Node Death @@ -341,6 +341,6 @@ Broker D (segment leader) Coordinator A Raft [A,B,C] 7. **Node-death seals carry no end entry id.** The coordinator doesn't know the actual offset. It is corrected later — by a seal request from the segment leader (if alive) or by D5 sealed-segment repair. This temporarily violates the metadata state machine's offset-continuity invariant (offset continuity within a range), which holds again after correction. -8. **Size- and age-based seals reuse the failure path.** Same flow, with no failed nodes and the replica set preserved. +8. **Size- and inactivity-based seals reuse the failure path.** Same flow, with no failed nodes and the replica set preserved. 9. **Followers need no segment assignment.** They self-authorize from the leader's first replication append (D2). diff --git a/docs/data-plane/d7_retention_gc.md b/docs/data-plane/d7_retention_gc.md index 13316dee..812f32e9 100644 --- a/docs/data-plane/d7_retention_gc.md +++ b/docs/data-plane/d7_retention_gc.md @@ -219,8 +219,7 @@ The clock is the only difficulty, and it's confined to one injectable input: - **File reclamation is synchronous.** The data-plane delete (file + cache + index) is covered by the synchronous data-plane state tests. - **No flaky age-based e2e.** A real-time retention e2e would race the virtual clock; - it's deliberately avoided (the same call already made for age-based seal). Coverage - comes from the decision/apply/reclamation units above, not a wall-clock sim run. + it's deliberately avoided. Coverage comes from the decision/apply/reclamation units above, not a wall-clock sim run. --- diff --git a/src/config.rs b/src/config.rs index b2b2e046..69242b3e 100644 --- a/src/config.rs +++ b/src/config.rs @@ -93,11 +93,11 @@ pub struct Environment { #[arg(long, env = "JOIN_MAX_ATTEMPTS", default_value_t = 5)] pub join_max_attempts: u32, - #[arg(long, env = "MAX_SEGMENT_AGE_SECS", default_value_t = 3600)] - pub max_segment_age_secs: u64, + #[arg(long, env = "SEGMENT_IDLE_TIMEOUT_SECS", default_value_t = 3600)] + pub segment_idle_timeout_secs: u64, - #[arg(long, env = "SEGMENT_AGE_CHECK_INTERVAL_SECS", default_value_t = 60)] - pub segment_age_check_interval_secs: u64, + #[arg(long, env = "SEGMENT_IDLE_CHECK_INTERVAL_SECS", default_value_t = 60)] + pub segment_idle_check_interval_secs: u64, #[arg( long, @@ -371,9 +371,9 @@ impl Environment { pub(crate) fn data_node_config(&self) -> DataNodeConfig { DataNodeConfig { - max_segment_age: std::time::Duration::from_secs(self.max_segment_age_secs), - age_check_interval: std::time::Duration::from_secs( - self.segment_age_check_interval_secs, + segment_idle_timeout: std::time::Duration::from_secs(self.segment_idle_timeout_secs), + idle_check_interval: std::time::Duration::from_secs( + self.segment_idle_check_interval_secs, ), segment_size_limit: self.segment_size_limit_bytes, batch_max_bytes: self.batch_max_bytes, @@ -390,8 +390,8 @@ impl Environment { #[derive(Clone)] pub struct DataNodeConfig { - pub max_segment_age: std::time::Duration, - pub age_check_interval: std::time::Duration, + pub segment_idle_timeout: std::time::Duration, + pub idle_check_interval: std::time::Duration, pub segment_size_limit: u64, pub batch_max_bytes: usize, pub hot_cache_budget_bytes: u64, @@ -402,8 +402,8 @@ pub struct DataNodeConfig { } impl DataNodeConfig { - pub(crate) fn age_check_ticks(&self) -> u64 { - self.age_check_interval.as_millis() as u64 / TICK_PERIOD_100_MS + pub(crate) fn idle_check_ticks(&self) -> u64 { + self.idle_check_interval.as_millis() as u64 / TICK_PERIOD_100_MS } pub(crate) fn cache_pressure_threshold_bytes(&self) -> Option { @@ -440,8 +440,8 @@ mod tests { join_interval_ms: 1000, join_multiplier: 2, join_max_attempts: 5, - max_segment_age_secs: 3600, - segment_age_check_interval_secs: 60, + segment_idle_timeout_secs: 3600, + segment_idle_check_interval_secs: 60, segment_size_limit_bytes: 1024 * 1024 * 1024, batch_max_bytes: 10 * 1024 * 1024, hot_cache_budget_bytes: 4 * 1024 * 1024 * 1024, diff --git a/src/data_plane/actor.rs b/src/data_plane/actor.rs index 11f9db01..3c660ace 100644 --- a/src/data_plane/actor.rs +++ b/src/data_plane/actor.rs @@ -4,7 +4,7 @@ use super::messages::pending::DataPlaneOutputs; use super::recovery::inventory::RecoveryOutput; use super::sparse_index::SparseIndex; use super::state::DataPlane; -use super::timer::SegmentAgeTimer; +use super::timer::SegmentIdleTimer; use super::wal::WalWriter; use crate::channels::BatchSender; use crate::config::DataNodeConfig; @@ -26,7 +26,7 @@ pub struct DataPlaneActor; impl DataPlaneActor { /// Spawn the data-plane worker thread together with its three schedulers (batch-flush, - /// replication, segment-age) and the bridge that forwards scheduler callbacks (tokio + /// replication, segment-idle) and the bridge that forwards scheduler callbacks (tokio /// mpsc) into the worker's flume mailbox, plus a self-contained orphan-GC ticker. pub fn spawn( node_id: NodeId, @@ -48,11 +48,11 @@ impl DataPlaneActor { let repl_scheduler_tx = spawn_scheduling_actor(timer_tx.clone(), 64, TICK_PERIOD_10_MS, None); - let _age_scheduler_tx = spawn_scheduling_actor::( + let _idle_scheduler_tx = spawn_scheduling_actor::( timer_tx, 64, TICK_PERIOD_100_MS, - Some(config.age_check_ticks()), + Some(config.idle_check_ticks()), ); let (tx, mailbox) = flume::bounded::(4096); diff --git a/src/data_plane/messages/command.rs b/src/data_plane/messages/command.rs index db5576fc..675e61d0 100644 --- a/src/data_plane/messages/command.rs +++ b/src/data_plane/messages/command.rs @@ -374,12 +374,12 @@ impl_from_variant!( ReceivePeerMessage, ); -use crate::data_plane::timer::{BatchFlushCallback, ReplicationCallback, SegmentAgeCallback}; +use crate::data_plane::timer::{BatchFlushCallback, ReplicationCallback, SegmentIdleCallback}; impl_from_variant_via!( DataPlaneCommand, DataPlaneTimeoutCallback, BatchFlushCallback, ReplicationCallback, - SegmentAgeCallback + SegmentIdleCallback ); diff --git a/src/data_plane/state.rs b/src/data_plane/state.rs index d792792b..1ee6a6fd 100644 --- a/src/data_plane/state.rs +++ b/src/data_plane/state.rs @@ -424,8 +424,8 @@ impl DataPlane { DataPlaneTimeoutCallback::ReplicationTimeout { seq, segment_key } => { self.handle_replication_timeout(segment_key, seq); } - DataPlaneTimeoutCallback::SegmentAgeCheck => { - self.enqueue_seal_for_aged_segments(self.config.max_segment_age); + DataPlaneTimeoutCallback::SegmentIdleCheck => { + self.enqueue_roll_for_idle_segments(self.config.segment_idle_timeout); } } } @@ -1382,19 +1382,19 @@ impl DataPlane { } } - pub(crate) fn enqueue_seal_for_aged_segments(&mut self, max_age: std::time::Duration) { - let aged: Box<[SegmentKey]> = self + pub(crate) fn enqueue_roll_for_idle_segments(&mut self, idle_timeout: std::time::Duration) { + let idle: Box<[SegmentKey]> = self .segments .iter() .filter(|(_, tracker)| { tracker.role() == SegmentRole::Leader - && tracker.age_limit_reached(max_age) + && tracker.idle_timeout_reached(idle_timeout) && !tracker.size_limit_reached(self.config.segment_size_limit) }) .map(|(&k, _)| k) .collect(); - for key in aged { + for key in idle { self.enqueue_roll_request(key, SegmentRollIntent::IdleMaintenance); } } @@ -2236,8 +2236,8 @@ mod tests { fn test_config(dir: PathBuf) -> DataNodeConfig { DataNodeConfig { - max_segment_age: std::time::Duration::from_secs(3600), - age_check_interval: std::time::Duration::from_secs(60), + segment_idle_timeout: std::time::Duration::from_secs(3600), + idle_check_interval: std::time::Duration::from_secs(60), segment_size_limit: 1024 * 1024 * 1024, batch_max_bytes: TEST_BATCH_MAX_BYTES, hot_cache_budget_bytes: 0, @@ -3199,12 +3199,12 @@ mod tests { } #[test] - fn age_limit_requests_idle_maintenance_roll() { + fn idle_timeout_requests_idle_maintenance_roll() { let dir = tempfile::tempdir().unwrap(); let mut dp = make_data_plane(&dir); dp.handle_command(assign_segment(test_key(), vec![test_node_id()])); - dp.enqueue_seal_for_aged_segments(std::time::Duration::ZERO); + dp.enqueue_roll_for_idle_segments(std::time::Duration::ZERO); assert_eq!( requested_roll_intents(&dp), @@ -3213,13 +3213,13 @@ mod tests { } #[test] - fn size_limit_takes_precedence_over_age_limit() { + fn size_limit_takes_precedence_over_idle_timeout() { let dir = tempfile::tempdir().unwrap(); let mut dp = make_data_plane(&dir); dp.config.segment_size_limit = 0; dp.handle_command(assign_segment(test_key(), vec![test_node_id()])); - dp.enqueue_seal_for_aged_segments(std::time::Duration::ZERO); + dp.enqueue_roll_for_idle_segments(std::time::Duration::ZERO); assert!(requested_roll_intents(&dp).is_empty()); } diff --git a/src/data_plane/states/segment/tracker.rs b/src/data_plane/states/segment/tracker.rs index 15c82d25..6a89f497 100644 --- a/src/data_plane/states/segment/tracker.rs +++ b/src/data_plane/states/segment/tracker.rs @@ -32,7 +32,7 @@ pub(crate) struct SegmentTracker { /// pin BTreeMap keys to a per-segment identity that survives writes. start_entry_id: EntryId, staged_entries: Vec, - created_at: std::time::Instant, + last_activity_at: tokio::time::Instant, } impl SegmentTracker { @@ -54,7 +54,7 @@ impl SegmentTracker { next_entry_id: EntryId::MIN, start_entry_id: EntryId::MIN, staged_entries: Vec::new(), - created_at: std::time::Instant::now(), + last_activity_at: tokio::time::Instant::now(), } } @@ -208,6 +208,7 @@ impl SegmentTracker { } self.cache.advance_read_cursor(commit + 1); self.committed_entry_id = entry_id; + self.last_activity_at = tokio::time::Instant::now(); } pub(crate) fn advance_checkpoint( @@ -285,8 +286,8 @@ impl SegmentTracker { self.shard_group_id } - pub(crate) fn age_limit_reached(&self, max_age: Duration) -> bool { - self.created_at.elapsed() >= max_age + pub(crate) fn idle_timeout_reached(&self, idle_timeout: Duration) -> bool { + self.last_activity_at.elapsed() >= idle_timeout } pub(crate) fn size_limit_reached(&self, limit: u64) -> bool { @@ -509,14 +510,27 @@ pub mod tests { } #[test] - fn age_limit_not_reached_immediately() { + fn idle_timeout_not_reached_immediately() { let t = make_tracker(SegmentRole::Leader); - assert!(!t.age_limit_reached(Duration::from_secs(60))); + assert!(!t.idle_timeout_reached(Duration::from_secs(60))); } #[test] - fn age_limit_reached_with_zero_duration() { + fn idle_timeout_reached_with_zero_duration() { let t = make_tracker(SegmentRole::Leader); - assert!(t.age_limit_reached(Duration::ZERO)); + assert!(t.idle_timeout_reached(Duration::ZERO)); + } + + #[test] + fn committed_progress_refreshes_last_activity() { + let mut t = make_tracker(SegmentRole::Leader); + t.last_activity_at = tokio::time::Instant::now() - Duration::from_secs(60); + assert!(t.idle_timeout_reached(Duration::from_secs(30))); + + t.stage_entry(test_key(), Bytes::from("data").into(), 1, EntryId(0)); + t.publish_staged(1); + t.commit_entry(EntryId(0)); + + assert!(!t.idle_timeout_reached(Duration::from_secs(30))); } } diff --git a/src/data_plane/timer.rs b/src/data_plane/timer.rs index e1d82725..9b99eff6 100644 --- a/src/data_plane/timer.rs +++ b/src/data_plane/timer.rs @@ -98,26 +98,26 @@ impl TTimer for ReplicationTimer { // --- Segment age timer (periodic age check via protocol period) --- #[derive(Debug)] -pub struct SegmentAgeTimer { +pub struct SegmentIdleTimer { ticks_remaining: u32, } #[derive(Debug, Default)] -pub enum SegmentAgeCallback { +pub enum SegmentIdleCallback { #[default] Check, } -impl TTimer for SegmentAgeTimer { - type Callback = SegmentAgeCallback; +impl TTimer for SegmentIdleTimer { + type Callback = SegmentIdleCallback; fn tick(&mut self) -> u32 { self.ticks_remaining = self.ticks_remaining.saturating_sub(1); self.ticks_remaining } - fn to_timeout_callback(self, _seq: u64, _now: u64) -> SegmentAgeCallback { - SegmentAgeCallback::Check + fn to_timeout_callback(self, _seq: u64, _now: u64) -> SegmentIdleCallback { + SegmentIdleCallback::Check } #[cfg(test)] @@ -132,7 +132,7 @@ impl TTimer for SegmentAgeTimer { pub enum DataPlaneTimeoutCallback { BatchFlushDeadline, ReplicationTimeout { seq: u64, segment_key: SegmentKey }, - SegmentAgeCheck, + SegmentIdleCheck, } impl From for DataPlaneTimeoutCallback { @@ -152,9 +152,9 @@ impl From for DataPlaneTimeoutCallback { } } -impl From for DataPlaneTimeoutCallback { - fn from(_: SegmentAgeCallback) -> Self { - DataPlaneTimeoutCallback::SegmentAgeCheck +impl From for DataPlaneTimeoutCallback { + fn from(_: SegmentIdleCallback) -> Self { + DataPlaneTimeoutCallback::SegmentIdleCheck } } @@ -165,7 +165,7 @@ impl fmt::Display for DataPlaneTimeoutCallback { DataPlaneTimeoutCallback::ReplicationTimeout { seq, segment_key } => { write!(f, "ReplicationTimeout(seq={seq}, {segment_key:?})") } - DataPlaneTimeoutCallback::SegmentAgeCheck => write!(f, "SegmentAgeCheck"), + DataPlaneTimeoutCallback::SegmentIdleCheck => write!(f, "SegmentIdleCheck"), } } } diff --git a/src/it/e2e/client_protocol.rs b/src/it/e2e/client_protocol.rs index aea22ede..bc0c292b 100644 --- a/src/it/e2e/client_protocol.rs +++ b/src/it/e2e/client_protocol.rs @@ -511,7 +511,7 @@ fn produce_then_fetch_hot() -> turmoil::Result { /// for repair to reassign, so this gates the repair e2e. #[test] #[serial_test::serial] -fn age_seal_rolls_the_active_segment() -> turmoil::Result { +fn idle_segment_automatically_rolls() -> turmoil::Result { let mut sim = Builder::new() .tick_duration(Duration::from_millis(100)) .simulation_duration(Duration::from_secs(180)) @@ -523,14 +523,14 @@ fn age_seal_rolls_the_active_segment() -> turmoil::Result { .try_init(); host_cluster(&mut sim, &NODES_3, |env| { - // Force a fast age-based seal. - env.max_segment_age_secs = 5; - env.segment_age_check_interval_secs = 1; + // Force a fast inactivity-based roll. + env.segment_idle_timeout_secs = 5; + env.segment_idle_check_interval_secs = 1; }); sim.client("test-client", async { const NODES: [(&str, u16); 3] = [("node-1", 8081), ("node-2", 8082), ("node-3", 8083)]; - const TOPIC: &str = "age-seal"; + const TOPIC: &str = "idle-roll"; wait_for_cluster(&NODES, 3).await; create_topic_anywhere(TOPIC, &NODES, 3).await; @@ -552,7 +552,7 @@ fn age_seal_rolls_the_active_segment() -> turmoil::Result { assert!(acked, "produce {i} not acked by {leader:?}"); } - // The segment ages past `max_segment_age_secs`; the leader enqueues a + // The segment remains inactive past `segment_idle_timeout_secs`; the leader enqueues a // RequestSegmentRoll, the coordinator commits a RollSegment, and the active // segment rolls. Poll DescribeTopic until the successor's start_offset // advances past 0 (the known-end seal landed). @@ -604,7 +604,7 @@ fn pressure_rolls_automatically_split_the_range() -> turmoil::Result { env.node_id_suffix = Some("sim".to_string()); env.vnodes_per_node = 16; env.segment_size_limit_bytes = 2048; - env.max_segment_age_secs = 3600; + env.segment_idle_timeout_secs = 3600; }); sim.client("test-client", async { @@ -643,8 +643,8 @@ fn idle_split_children_automatically_merge() -> turmoil::Result { env.node_id_suffix = Some("sim".to_string()); env.vnodes_per_node = 16; env.segment_size_limit_bytes = 2048; - env.max_segment_age_secs = 0; - env.segment_age_check_interval_secs = 300; + env.segment_idle_timeout_secs = 0; + env.segment_idle_check_interval_secs = 300; }); sim.client("test-client", async { diff --git a/src/it/helpers.rs b/src/it/helpers.rs index f9360c5f..4c70b704 100644 --- a/src/it/helpers.rs +++ b/src/it/helpers.rs @@ -36,8 +36,8 @@ pub fn default_env(idx: u32, node_id: String, client_port: u16, cluster_port: u1 join_multiplier: 2, join_max_attempts: 5, data_port: 2923, - max_segment_age_secs: 3600, - segment_age_check_interval_secs: 60, + segment_idle_timeout_secs: 3600, + segment_idle_check_interval_secs: 60, segment_size_limit_bytes: 1024 * 1024 * 1024, batch_max_bytes: 10 * 1024 * 1024, hot_cache_budget_bytes: 4 * 1024 * 1024 * 1024, From 73374c1524043db182a3db940c69c0df4f5c1886 Mon Sep 17 00:00:00 2001 From: Migorithm Date: Sat, 18 Jul 2026 17:26:56 +0400 Subject: [PATCH 11/14] PendingSegmentRollRequests --- src/data_plane/states/roll_request.rs | 133 ++++++++++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 src/data_plane/states/roll_request.rs diff --git a/src/data_plane/states/roll_request.rs b/src/data_plane/states/roll_request.rs new file mode 100644 index 00000000..7c6b0fa2 --- /dev/null +++ b/src/data_plane/states/roll_request.rs @@ -0,0 +1,133 @@ +//! Pending segment-roll requests from the write leader to the coordinator. +//! +//! A leader that can't replicate sends `RequestSegmentRoll` and tracks it until +//! `SegmentRollCommitted` clears it; the data plane re-sends timed-out requests. Owns the +//! dedup + timing bookkeeping; building/sending the request stays in the data +//! plane (it reads the live tracker's committed end). + +use std::collections::HashMap; +use std::time::Duration; +// tokio's runtime-aware Instant: real time in prod, virtualized under turmoil so +// the retry timeout fires on the sim clock (the data-plane worker runs on a tokio +// runtime in both — see CLAUDE.md "Testing"). Falls back to real time with no +// runtime, so the sync tests below still work. +use tokio::time::Instant; + +use crate::control_plane::NodeId; +use crate::control_plane::metadata::command::SegmentRollIntent; +use crate::data_plane::SegmentKey; + +struct PendingSegmentRollRequest { + sent_at: Instant, + failed_nodes: Vec, + intent: SegmentRollIntent, +} + +/// Segment-roll requests awaiting `SegmentRollCommitted`, keyed by segment. +#[derive(Default)] +pub(crate) struct PendingSegmentRollRequests { + requests: HashMap, +} + +impl PendingSegmentRollRequests { + /// Whether a request for `key` is already in flight (the dedup guard). + pub(crate) fn is_tracked(&self, key: &SegmentKey) -> bool { + self.requests.contains_key(key) + } + + /// Record a just-sent request. + pub(crate) fn track( + &mut self, + key: SegmentKey, + failed_nodes: Vec, + intent: SegmentRollIntent, + sent_at: Instant, + ) { + self.requests.insert( + key, + PendingSegmentRollRequest { + sent_at, + failed_nodes, + intent, + }, + ); + } + + /// Drop a request once its committed result lands. + pub(crate) fn remove(&mut self, key: &SegmentKey) { + self.requests.remove(key); + } + + /// Timed-out requests to re-send — refreshing their + /// clock so each fires at most once per `timeout`. + pub(crate) fn take_due( + &mut self, + now: Instant, + timeout: Duration, + ) -> Vec<(SegmentKey, Vec, SegmentRollIntent)> { + let mut due = Vec::new(); + for (key, req) in &mut self.requests { + if now.duration_since(req.sent_at) >= timeout { + req.sent_at = now; + due.push((*key, req.failed_nodes.clone(), req.intent)); + } + } + due + } + + #[cfg(test)] + pub(crate) fn failed_nodes(&self, key: &SegmentKey) -> Option<&[NodeId]> { + self.requests.get(key).map(|r| r.failed_nodes.as_slice()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::control_plane::metadata::{RangeId, SegmentId, TopicId}; + + fn seg() -> SegmentKey { + SegmentKey::new(TopicId(0), RangeId(0), SegmentId(0)) + } + + #[test] + fn track_then_clear() { + let mut p = PendingSegmentRollRequests::default(); + assert!(!p.is_tracked(&seg())); + p.track( + seg(), + vec![], + SegmentRollIntent::DataPressure, + Instant::now(), + ); + assert!(p.is_tracked(&seg())); + p.remove(&seg()); + assert!(!p.is_tracked(&seg())); + } + + #[test] + fn take_due_returns_only_timed_out_then_refreshes() { + let mut p = PendingSegmentRollRequests::default(); + let t0 = Instant::now(); + let timeout = Duration::from_secs(5); + p.track( + seg(), + vec![NodeId::new("d")], + SegmentRollIntent::ReplicationFailure, + t0, + ); + + assert!(p.take_due(t0, timeout).is_empty(), "not yet due"); + let due = p.take_due(t0 + timeout, timeout); + assert_eq!( + due, + vec![( + seg(), + vec![NodeId::new("d")], + SegmentRollIntent::ReplicationFailure, + )] + ); + // Refreshed → not due again at the same instant. + assert!(p.take_due(t0 + timeout, timeout).is_empty()); + } +} From 719ecb4645a15b838576108ef5ae27f0f3b30a0e Mon Sep 17 00:00:00 2001 From: Migorithm Date: Sat, 18 Jul 2026 18:10:35 +0400 Subject: [PATCH 12/14] metadata state owns event generation, thereby simplifying into_commands --- .claude/rules/metadata-state-machine.md | 4 +- .../data-plane/d3_segment_roll_integration.md | 4 +- src/control_plane/consensus/messages/event.rs | 25 +++--- src/control_plane/consensus/multi_raft.rs | 10 ++- src/control_plane/consensus/raft/state.rs | 42 +++++----- src/control_plane/metadata/event.rs | 84 +++++++------------ 6 files changed, 74 insertions(+), 95 deletions(-) diff --git a/.claude/rules/metadata-state-machine.md b/.claude/rules/metadata-state-machine.md index c34e528e..d9636b03 100644 --- a/.claude/rules/metadata-state-machine.md +++ b/.claude/rules/metadata-state-machine.md @@ -73,10 +73,10 @@ MetadataStateMachine (one per shard group) 18. **`correct_end_offset` is write-once.** Updates the sealed segment's `end_offset` only when the current value is `None` AND the incoming `end_entry_id` is `Some`. Simultaneously sets the active segment's `start_offset` to `end_entry_id + 1`. Re-application is a no-op. Restores invariant 8 after a death-triggered roll. This is now the **fallback** correction — for the follower-death race (the live leader's write-path seal arrives after a reconcile `None`-roll) and the unknown-end fallback. The leader-crash path no longer relies on it: it recovers the committed end *before* rolling (`raft-actor.md` #6), so the successor opens at `min+1` directly rather than at 0-then-corrected. -19. **`RollSegment` is idempotent.** Re-applying a `RollSegment` for a segment that is no longer active returns `ApplyResult::Noop` rather than an error. Tolerates the race where both the write-path timeout and SWIM node death fire `RollSegment` for the same failure. +19. **`RollSegment` is idempotent.** Re-applying a `RollSegment` for a segment that is no longer active succeeds without raising a metadata event rather than returning an error. Tolerates the race where both the write-path timeout and SWIM node death fire `RollSegment` for the same failure. 20. **An entity's state never reverts.** Topic: Active → Sealed → Deleted. Range: Active → Sealed → Deleting. Segment: Active → Sealed → Deleting. No backward transitions. Once a segment is Sealed it never becomes Active again — `ReassignSegment` swaps its `replica_set` but keeps it `Sealed`. -21. **`ReassignSegment` only re-points a sealed segment.** `apply_reassign_segment()` accepts only a `Sealed` segment, swaps `replica_set`, and changes nothing else — state stays `Sealed`; data, offsets, lineage, and timestamps stay frozen (invariant 3). An active, deleting, or unknown segment is rejected (`SegmentNotSealed` / `SegmentNotFound`), logged but not fatal (invariant 11). Re-applying with the same `replica_set` returns `ApplyResult::Noop`, tolerating duplicate death detection and no-leader re-proposals (cf. invariant 19). The swap runs through `apply`, so the umbrella `assert_invariants` re-checks every other invariant afterward — a reassignment cannot leave the machine inconsistent. +21. **`ReassignSegment` only re-points a sealed segment.** `apply_reassign_segment()` accepts only a `Sealed` segment, swaps `replica_set`, and changes nothing else — state stays `Sealed`; data, offsets, lineage, and timestamps stay frozen (invariant 3). An active, deleting, or unknown segment is rejected (`SegmentNotSealed` / `SegmentNotFound`), logged but not fatal (invariant 11). Re-applying with the same `replica_set` succeeds without raising a metadata event, tolerating duplicate death detection and no-leader re-proposals (cf. invariant 19). The swap runs through `apply`, so the umbrella `assert_invariants` re-checks every other invariant afterward — a reassignment cannot leave the machine inconsistent. 22. **A committed consumer-group generation assigns each active range exactly once.** When a group has members, its assignment keys exactly equal the topic's active ranges and every assignment names a current member. When it has no members, it has no assignments. Membership or range-topology changes advance the generation and recompute the full desired assignment through the Raft log; heartbeat refreshes that do not change membership leave the generation unchanged. diff --git a/docs/data-plane/d3_segment_roll_integration.md b/docs/data-plane/d3_segment_roll_integration.md index 5292edad..eae11b0b 100644 --- a/docs/data-plane/d3_segment_roll_integration.md +++ b/docs/data-plane/d3_segment_roll_integration.md @@ -221,7 +221,7 @@ A segment assignment carries: the segment identity, the shard group id (routing ### Commit Event -Applying a committed metadata entry emits a "metadata committed" event carrying the shard group, the apply result, and the log position. **All replicas emit it** (apply is deterministic), but **only the leader dispatches** the resulting notifications — followers apply the entry and drop the event. +Applying a committed metadata entry can raise zero or more metadata events, each carrying the shard group and log position when it leaves the Raft state machine. **All replicas produce the same events** (apply is deterministic), but **only the leader dispatches** the resulting notifications — followers apply the entry and drop them. ### Dispatch (leader only) @@ -239,7 +239,7 @@ range split / range merged: segment assignment → each new segment's primary ``` -The apply result is produced on all replicas; only the leader dispatches. For a rolled segment the assignment is always sent (the result carries full context); the seal response is sent only when pending context still exists on this leader. +The event stream is produced on all replicas; only the leader dispatches it. For a rolled segment the assignment is always sent (the event carries full context); the roll response is sent only when pending context still exists on this leader. A single entry may also raise independent segment-seal or consumer-group events, so those effects are not hidden inside the roll, split, or merge event. ### Active-Segment Lookup diff --git a/src/control_plane/consensus/messages/event.rs b/src/control_plane/consensus/messages/event.rs index 63835475..afa9419c 100644 --- a/src/control_plane/consensus/messages/event.rs +++ b/src/control_plane/consensus/messages/event.rs @@ -4,7 +4,7 @@ use crate::control_plane::consensus::messages::timer::RaftTimer; use crate::control_plane::consensus::multi_raft::RollRequestContext; use crate::control_plane::consensus::raft::log::LogEntry; use crate::control_plane::membership::ShardGroupId; -use crate::control_plane::metadata::event::ApplyResult; +use crate::control_plane::metadata::event::MetadataEvent; use crate::data_plane::transport::command::DataTransportCommand; use crate::impl_from_variant; use crate::schedulers::ticker_message::TimerCommand; @@ -19,7 +19,7 @@ pub struct LeaderChange { #[derive(Debug)] pub struct MetadataCommitted { pub shard_group_id: ShardGroupId, - pub result: ApplyResult, + pub event: MetadataEvent, pub log_index: u64, pub roll_context: Option, } @@ -61,19 +61,18 @@ pub enum RaftEvent { impl MetadataCommitted { pub fn into_data_transport_cmds(self) -> Vec { let sgid = self.shard_group_id; - match self.result { - ApplyResult::TopicCreated(tc) => { + match self.event { + MetadataEvent::TopicCreated(tc) => { vec![tc.into_command(sgid)] } - ApplyResult::SegmentRolled(sr) => sr.into_command(self.roll_context, sgid), - ApplyResult::RangeSplit(rs) => rs.into_command(sgid), - ApplyResult::RangeMerged(rm) => rm.into_commands(sgid), - ApplyResult::TopicDeleted(deleted) => deleted.into_commands(), - ApplyResult::Noop => vec![], - ApplyResult::SegmentReassigned(r) => r.into_catch_up_commands(sgid), - ApplyResult::SegmentsDeleted(d) => d.into_commands(), - ApplyResult::SegmentBoundaryCorrected(ssc) => ssc.into_commands(), - ApplyResult::ConsumerGroupChanged(epoch) => epoch.into_commands(), + MetadataEvent::SegmentRolled(sr) => sr.into_command(self.roll_context, sgid), + MetadataEvent::RangeSplit(rs) => rs.into_command(sgid), + MetadataEvent::RangeMerged(rm) => rm.into_commands(sgid), + MetadataEvent::SegmentSealCommitted(sealed) => sealed.into_commands(), + MetadataEvent::SegmentReassigned(r) => r.into_catch_up_commands(sgid), + MetadataEvent::SegmentsDeleted(d) => d.into_commands(), + MetadataEvent::SegmentBoundaryCorrected(ssc) => ssc.into_commands(), + MetadataEvent::ConsumerGroupChanged(epoch) => epoch.into_commands(), } } } diff --git a/src/control_plane/consensus/multi_raft.rs b/src/control_plane/consensus/multi_raft.rs index 683fa67a..936ef61a 100644 --- a/src/control_plane/consensus/multi_raft.rs +++ b/src/control_plane/consensus/multi_raft.rs @@ -13,6 +13,7 @@ use crate::control_plane::consensus::raft::storage::RaftStorage; use crate::control_plane::consensus::raft::{compute_replacement_replica_set, now_ms}; use crate::control_plane::membership::{ShardGroup, ShardGroupId, TopologyReader}; use crate::control_plane::metadata::command::RollSegment; +use crate::control_plane::metadata::event::MetadataEvent; use crate::control_plane::metadata::{ ConsumerGroupAssignment, EntryId, SegmentRollIntent, TopicId, TopicMeta, TopicStats, }; @@ -842,9 +843,12 @@ impl MultiRaft { } if let RaftEvent::MetadataCommitted(committed) = &mut event { - // For the leader to send the committed roll back to the requester. - committed.roll_context = - self.pending_rolls.pop_roll_context(id, committed.log_index); + // Only the roll event answers the requester. Other metadata events + // from the same log entry must not consume its pending context. + if matches!(committed.event, MetadataEvent::SegmentRolled(_)) { + committed.roll_context = + self.pending_rolls.pop_roll_context(id, committed.log_index); + } } self.pending_events.push(event); } diff --git a/src/control_plane/consensus/raft/state.rs b/src/control_plane/consensus/raft/state.rs index f4ac6ae1..8c76d825 100644 --- a/src/control_plane/consensus/raft/state.rs +++ b/src/control_plane/consensus/raft/state.rs @@ -10,7 +10,7 @@ use crate::control_plane::consensus::raft::storage::RaftPersistentState; use crate::control_plane::consensus::raft::{compute_replacement_replica_set, now_ms}; use crate::control_plane::membership::{ShardGroupId, TopologyReader}; use crate::control_plane::metadata::command::DeleteSegments; -use crate::control_plane::metadata::event::ApplyResult; +use crate::control_plane::metadata::event::MetadataEvent; use crate::control_plane::metadata::{ ConsumerGroupAssignment, ConsumerMemberId, MetadataCommand, ReassignSegment, RollSegment, SegmentRollIntent, TopicId, TopicMeta, TopicStats, @@ -1273,26 +1273,7 @@ impl Raft { fn apply_metadata_entry(&mut self, cmd: MetadataCommand, index: u64) { match self.metadata.apply(cmd) { - Ok(result) => { - tracing::debug!( - "[{}] Applied metadata at index {}: {:?}", - self.node_id, - index, - result - ); - - if self.is_leader() - && let ApplyResult::SegmentReassigned(r) = &result - { - self.consensus.track_catch_up(r); - } - self.raise(MetadataCommitted { - shard_group_id: self.shard_group_id, - result, - log_index: index, - roll_context: None, - }); - } + Ok(()) => {} Err(e) => tracing::error!( "[{}] Metadata apply error at index {}: {:?}", self.node_id, @@ -1300,6 +1281,25 @@ impl Raft { e ), } + for event in self.metadata.take_pending_events() { + tracing::debug!( + "[{}] Applied metadata event at index {}: {:?}", + self.node_id, + index, + event + ); + if self.is_leader() + && let MetadataEvent::SegmentReassigned(reassigned) = &event + { + self.consensus.track_catch_up(reassigned); + } + self.raise(MetadataCommitted { + shard_group_id: self.shard_group_id, + event, + log_index: index, + roll_context: None, + }); + } if self.consensus.is_leader() { self.consensus .extend_pending_proposals(self.metadata.take_pending_proposals()); diff --git a/src/control_plane/metadata/event.rs b/src/control_plane/metadata/event.rs index 0c137427..3bf36d0e 100644 --- a/src/control_plane/metadata/event.rs +++ b/src/control_plane/metadata/event.rs @@ -36,7 +36,6 @@ pub struct SegmentRolled { pub new_segment_key: SegmentKey, pub new_replica_set: Replicas, pub end_entry_id: Option, - pub consumer_group_epochs: Box<[ConsumerGroupEpochSnapshot]>, } impl SegmentRolled { @@ -66,9 +65,6 @@ impl SegmentRolled { }, )); } - for epoch in self.consumer_group_epochs { - v.extend(epoch.into_commands()); - } v } } @@ -111,14 +107,11 @@ impl SegmentReassigned { pub struct RangeSplit { pub topic_id: TopicId, pub children: [(RangeId, SegmentId, Replicas); 2], - pub parent_active_segment: Option<(SegmentKey, Replicas)>, - pub consumer_group_epochs: Box<[ConsumerGroupEpochSnapshot]>, } impl RangeSplit { pub fn into_command(self, shard_group_id: ShardGroupId) -> Vec { - let mut cmds: Vec = self - .children + self.children .into_iter() .map(|(range_id, segment_id, replica_set)| { let target = replica_set[0].clone(); @@ -132,26 +125,7 @@ impl RangeSplit { }, ) }) - .collect(); - - // sealing parent segment - if let Some((parent_key, parent_replica_set)) = self.parent_active_segment - && !parent_replica_set.is_empty() - { - cmds.push(DataTransportCommand::send_to_targets( - parent_replica_set.0, - SegmentMetaSealed { - segment_key: parent_key, - committed_entry_id: None, - }, - )); - } - - for epoch in self.consumer_group_epochs { - cmds.extend(epoch.into_commands()); - } - - cmds + .collect() } } @@ -159,12 +133,11 @@ impl RangeSplit { pub struct RangeMerged { pub segment_key: SegmentKey, pub replica_set: Replicas, - pub consumer_group_epochs: Box<[ConsumerGroupEpochSnapshot]>, } impl RangeMerged { pub fn into_commands(self, shard_group_id: ShardGroupId) -> Vec { let target = self.replica_set[0].clone(); - let mut commands = vec![DataTransportCommand::send_to_targets( + let commands = vec![DataTransportCommand::send_to_targets( vec![target], PlaceSegment { segment_key: self.segment_key, @@ -173,13 +146,32 @@ impl RangeMerged { start_entry_id: EntryId::MIN, }, )]; - for epoch in self.consumer_group_epochs { - commands.extend(epoch.into_commands()); - } commands } } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SegmentSealCommitted { + pub segment_key: SegmentKey, + pub replica_set: Replicas, +} + +impl SegmentSealCommitted { + pub fn into_commands(self) -> Vec { + if self.replica_set.is_empty() { + vec![] + } else { + vec![DataTransportCommand::send_to_targets( + self.replica_set.0, + SegmentMetaSealed { + segment_key: self.segment_key, + committed_entry_id: None, + }, + )] + } + } +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct ConsumerGroupEpochSnapshot { pub topic_id: TopicId, @@ -188,21 +180,6 @@ pub struct ConsumerGroupEpochSnapshot { pub ranges: Box<[(RangeId, Replicas)]>, } -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct TopicDeleted { - pub consumer_group_epochs: Box<[ConsumerGroupEpochSnapshot]>, -} - -impl TopicDeleted { - pub fn into_commands(self) -> Vec { - self.consumer_group_epochs - .into_vec() - .into_iter() - .flat_map(ConsumerGroupEpochSnapshot::into_commands) - .collect() - } -} - impl ConsumerGroupEpochSnapshot { pub fn into_commands(self) -> Vec { self.ranges @@ -274,28 +251,27 @@ impl SegmentBoundaryCorrected { } #[derive(Debug, Clone, PartialEq, Eq)] -pub enum ApplyResult { +pub enum MetadataEvent { TopicCreated(TopicCreated), SegmentRolled(SegmentRolled), RangeSplit(RangeSplit), RangeMerged(RangeMerged), + SegmentSealCommitted(SegmentSealCommitted), SegmentReassigned(SegmentReassigned), SegmentsDeleted(SegmentsDeleted), SegmentBoundaryCorrected(SegmentBoundaryCorrected), ConsumerGroupChanged(ConsumerGroupEpochSnapshot), - TopicDeleted(TopicDeleted), - Noop, } impl_from_variant!( - ApplyResult, + MetadataEvent, TopicCreated, SegmentRolled, RangeSplit, RangeMerged, + SegmentSealCommitted, SegmentReassigned, SegmentsDeleted, SegmentBoundaryCorrected, - ConsumerGroupChanged(ConsumerGroupEpochSnapshot), - TopicDeleted + ConsumerGroupChanged(ConsumerGroupEpochSnapshot) ); From e1762b7853580dc49e8b16d626a513786bb4317a Mon Sep 17 00:00:00 2001 From: Migorithm Date: Sat, 18 Jul 2026 18:13:18 +0400 Subject: [PATCH 13/14] revamp idle_split_children_automatically_merge --- src/it/e2e/client_protocol.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/it/e2e/client_protocol.rs b/src/it/e2e/client_protocol.rs index bc0c292b..fba3c8f8 100644 --- a/src/it/e2e/client_protocol.rs +++ b/src/it/e2e/client_protocol.rs @@ -669,16 +669,29 @@ fn idle_split_children_automatically_merge() -> turmoil::Result { .iter() .filter(|range| range.state == RangeState::Active) .count(); - let merged_range_is_active = detail + let merged_range = detail .ranges .iter() - .any(|range| range.state == RangeState::Active && range.merged_from.is_some()); - if active_count == 1 && merged_range_is_active { + .find(|range| range.state == RangeState::Active && range.merged_from.is_some()); + let source_boundaries_are_known = merged_range + .and_then(|range| range.merged_from) + .is_some_and(|(left, right)| { + [left, right].into_iter().all(|source_id| { + detail + .ranges + .iter() + .find(|range| range.range_id == source_id) + .and_then(|range| range.sealed_segments.last()) + // having end_entry_id means previous segments got sealed and boundary recovery was made successfully. + .is_some_and(|segment| segment.end_entry_id.is_some()) + }) + }); + if active_count == 1 && source_boundaries_are_known { return Ok(()); } } - panic!("idle split children never merged through Raft"); + panic!("idle children did not merge with known source-segment boundaries"); }); sim.run() From d41ef12ac54739914903122b18a74daa2013512c Mon Sep 17 00:00:00 2001 From: Migorithm Date: Sat, 18 Jul 2026 18:31:58 +0400 Subject: [PATCH 14/14] multi event raises, add missing SegmentSealCommitted generation in merge path --- .../consensus/raft/states/metadata_state.rs | 747 +++++++++++------- src/control_plane/metadata/topic.rs | 20 + 2 files changed, 473 insertions(+), 294 deletions(-) diff --git a/src/control_plane/consensus/raft/states/metadata_state.rs b/src/control_plane/consensus/raft/states/metadata_state.rs index f8328461..99b29adf 100644 --- a/src/control_plane/consensus/raft/states/metadata_state.rs +++ b/src/control_plane/consensus/raft/states/metadata_state.rs @@ -21,6 +21,7 @@ pub struct MetadataState { topic_name_index: HashMap, next_topic_id: u64, pending_proposals: Vec, + pending_events: Vec, } impl MetadataState { @@ -31,6 +32,7 @@ impl MetadataState { topic_name_index: HashMap::new(), next_topic_id: shard_group_id.0 << 32, pending_proposals: Vec::new(), + pending_events: Vec::new(), } } @@ -76,6 +78,14 @@ impl MetadataState { std::mem::take(&mut self.pending_proposals).into_boxed_slice() } + pub(crate) fn take_pending_events(&mut self) -> Vec { + std::mem::take(&mut self.pending_events) + } + + fn raise_event(&mut self, event: impl Into) { + self.pending_events.push(event.into()); + } + pub(crate) fn active_segments_for_node( &self, node_id: &NodeId, @@ -156,27 +166,24 @@ impl MetadataState { .collect() } - pub(crate) fn apply(&mut self, command: MetadataCommand) -> Result { + pub(crate) fn apply(&mut self, command: MetadataCommand) -> Result<(), MetadataError> { use MetadataCommand::*; - let result = match command { - CreateTopic(cmd) => self.create_topic(cmd)?.into(), + match command { + CreateTopic(cmd) => self.create_topic(cmd)?, RollSegment(cmd) => self.roll_segment(cmd)?, - SplitRange(cmd) => self.split_range(cmd)?.into(), - MergeRange(cmd) => self.merge_range(cmd)?.into(), - DeleteTopic(cmd) => self.delete_topic(cmd)?.into(), + SplitRange(cmd) => self.split_range(cmd)?, + MergeRange(cmd) => self.merge_range(cmd)?, + DeleteTopic(cmd) => self.delete_topic(cmd)?, ReassignSegment(cmd) => self.reassign_segment(cmd)?, DeleteSegments(cmd) => self.delete_segments(cmd)?, - SyncConsumerGroup(cmd) => self - .sync_consumer_group(cmd)? - .map(ApplyResult::ConsumerGroupChanged) - .unwrap_or(ApplyResult::Noop), - }; + SyncConsumerGroup(cmd) => self.sync_consumer_group(cmd)?, + } #[cfg(any(test, debug_assertions))] self.assert_invariants(); - Ok(result) + Ok(()) } - fn create_topic(&mut self, cmd: CreateTopic) -> Result { + fn create_topic(&mut self, cmd: CreateTopic) -> Result<(), MetadataError> { if self.topic_name_index.contains_key(&cmd.name) { return Err(TopicNameAlreadyExists(cmd.name)); } @@ -192,13 +199,14 @@ impl MetadataState { self.topic_name_index.insert(topic.name.clone(), topic_id); self.topics.insert(topic.id, topic); self.next_topic_id += 1; - Ok(TopicCreated { + self.raise_event(TopicCreated { segment_key: SegmentKey::new(topic_id, RangeId(0), SegmentId(0)), replica_set, - }) + }); + Ok(()) } - fn roll_segment(&mut self, cmd: RollSegment) -> Result { + fn roll_segment(&mut self, cmd: RollSegment) -> Result<(), MetadataError> { let topic = self.get_active_topic_mut(cmd.segment_key.topic_id)?; let can_split = topic.can_split(); let range = topic.get_range_mut(&cmd.segment_key.range_id)?; @@ -208,25 +216,24 @@ impl MetadataState { // If Inactive, correction path if !is_active { let Some(end_entry_id) = cmd.end_entry_id else { - return Ok(ApplyResult::Noop); + return Ok(()); }; let Some(replica_set) = range.correct_end_offset(cmd.segment_key.segment_id, end_entry_id) else { - return Ok(ApplyResult::Noop); + return Ok(()); }; - return Ok(ApplyResult::SegmentBoundaryCorrected( - SegmentBoundaryCorrected { - segment_key: cmd.segment_key, - replica_set, - committed_entry_id: cmd.end_entry_id, - }, - )); + self.raise_event(SegmentBoundaryCorrected { + segment_key: cmd.segment_key, + replica_set, + committed_entry_id: cmd.end_entry_id, + }); + return Ok(()); } if cmd.intent == SegmentRollIntent::BoundaryCorrection { - return Ok(ApplyResult::Noop); + return Ok(()); } // If Active, Roll @@ -237,7 +244,7 @@ impl MetadataState { // For segment roll, unless data nodes got changed, consumer // TODO For now, it loops over EVERY consumger groups and take epoch snapshot for every topic. // TODO To reduce the load, it should specifically target groups that are affected by the possible range split - let consumer_group_epochs = topic + let consumer_group_epochs: Box<[ConsumerGroupEpochSnapshot]> = topic .consumer_groups .keys() .filter_map(|group_id| topic.consumer_group_epoch(group_id)) @@ -263,19 +270,22 @@ impl MetadataState { if let Some(proposal) = merge_proposal { self.pending_proposals.push(proposal); } - Ok(SegmentRolled { + self.raise_event(SegmentRolled { new_segment_key: cmd.segment_key.with_segment_id(new_segment_id), new_replica_set: cmd.new_replica_set, end_entry_id: cmd.end_entry_id, - consumer_group_epochs, + }); + + for epoch in consumer_group_epochs { + self.raise_event(epoch); } - .into()) + Ok(()) } /// Re-points a sealed segment's replica set. /// `Noop` when the set is unchanged, otherwise `SegmentReassigned`. /// The segment must be sealed; an active/deleting/unknown one is rejected. - fn reassign_segment(&mut self, cmd: ReassignSegment) -> Result { + fn reassign_segment(&mut self, cmd: ReassignSegment) -> Result<(), MetadataError> { let segment = self .topics .get_mut(&cmd.segment_key.topic_id) @@ -284,17 +294,18 @@ impl MetadataState { // The dispatch announces the desired replica set; receivers reconcile, so // we only carry the sealed bounds (the catch-up target) alongside it. - if segment.reassign(cmd.replica_set.clone())? { - Ok(SegmentReassigned { + let event = segment + .reassign(cmd.replica_set.clone())? + .then_some(SegmentReassigned { segment_key: cmd.segment_key, start_entry_id: segment.start_entry_id, sealed_end: segment.end_entry_id, new_replica_set: cmd.replica_set, - } - .into()) - } else { - Ok(ApplyResult::Noop) + }); + if let Some(event) = event { + self.raise_event(event); } + Ok(()) } /// Retention: mark an oldest-first prefix of a range's sealed segments `Deleting`. @@ -302,7 +313,7 @@ impl MetadataState { /// `SegmentsDeleted` carrying the deleted segments grouped by `replica_set` for /// batched dispatch. Uses plain `get_mut` (not `validate_active`) so a command /// applied after the topic is being deleted is a harmless no-op (already `Deleting`). - fn delete_segments(&mut self, cmd: DeleteSegments) -> Result { + fn delete_segments(&mut self, cmd: DeleteSegments) -> Result<(), MetadataError> { let range = self .topics .get_mut(&cmd.topic_id) @@ -311,7 +322,7 @@ impl MetadataState { let deleted_ids = range.delete_segments(&cmd.segment_ids); if deleted_ids.is_empty() { - return Ok(ApplyResult::Noop); + return Ok(()); } // Group the deleted segments by replica_set here. let mut groups: Vec<(Replicas, Vec)> = Vec::new(); @@ -325,7 +336,8 @@ impl MetadataState { None => groups.push((seg.replica_set.clone(), vec![key])), } } - Ok(SegmentsDeleted { groups }.into()) + self.raise_event(SegmentsDeleted { groups }); + Ok(()) } fn get_active_topic_mut(&mut self, id: TopicId) -> Result<&mut TopicMeta, MetadataError> { @@ -334,7 +346,7 @@ impl MetadataState { Ok(topic) } - fn split_range(&mut self, cmd: SplitRange) -> Result { + fn split_range(&mut self, cmd: SplitRange) -> Result<(), MetadataError> { let topic = self .topics .get_mut(&cmd.topic_id) @@ -361,18 +373,26 @@ impl MetadataState { let consumer_group_epochs = topic.rebalance_consumer_groups(); - Ok(RangeSplit { + self.raise_event(RangeSplit { topic_id: cmd.topic_id, children: [ (left_id, SegmentId(0), cmd.left_replica_set), (right_id, SegmentId(0), cmd.right_replica_set), ], - parent_active_segment, - consumer_group_epochs, - }) + }); + if let Some((segment_key, replica_set)) = parent_active_segment { + self.raise_event(SegmentSealCommitted { + segment_key, + replica_set, + }); + } + for epoch in consumer_group_epochs { + self.raise_event(epoch); + } + Ok(()) } - fn merge_range(&mut self, cmd: MergeRange) -> Result { + fn merge_range(&mut self, cmd: MergeRange) -> Result<(), MetadataError> { let topic_id = cmd.topic_id; let replica_set = cmd.merged_replica_set.clone(); let topic = self @@ -380,20 +400,32 @@ impl MetadataState { .get_mut(&topic_id) .ok_or(TopicNotFound(topic_id))?; topic.validate_active()?; + + let sealed_source_segments = [ + topic.active_segment_key_and_replicas(cmd.range_id_1)?, + topic.active_segment_key_and_replicas(cmd.range_id_2)?, + ]; + let merged_id = topic.execute_merge(cmd)?; let consumer_group_epochs = topic.rebalance_consumer_groups(); - Ok(RangeMerged { + self.raise_event(RangeMerged { segment_key: SegmentKey::new(topic_id, merged_id, SegmentId(0)), replica_set, - consumer_group_epochs, - }) + }); + for (segment_key, source_replica_set) in sealed_source_segments { + self.raise_event(SegmentSealCommitted { + segment_key, + replica_set: source_replica_set, + }); + } + for epoch in consumer_group_epochs { + self.raise_event(epoch); + } + Ok(()) } - fn sync_consumer_group( - &mut self, - cmd: SyncConsumerGroup, - ) -> Result, MetadataError> { + fn sync_consumer_group(&mut self, cmd: SyncConsumerGroup) -> Result<(), MetadataError> { let group_id = cmd.group_id.clone(); let topic = self @@ -403,9 +435,14 @@ impl MetadataState { .ok_or_else(|| TopicNameNotFound(cmd.topic_name.clone()))?; topic.validate_active()?; if !topic.sync_consumer_group(cmd) { - return Ok(None); + return Ok(()); } - Ok(topic.consumer_group_epoch(&group_id)) + + let event = topic.consumer_group_epoch(&group_id); + if let Some(event) = event { + self.raise_event(event); + } + Ok(()) } // ! SAFETY: When the loop evaluates the [1, 2] pair and decides it is mergeable, @@ -425,7 +462,7 @@ impl MetadataState { .collect() } - fn delete_topic(&mut self, cmd: DeleteTopic) -> Result { + fn delete_topic(&mut self, cmd: DeleteTopic) -> Result<(), MetadataError> { let topic_id = self .topic_name_index .get(&cmd.name) @@ -441,9 +478,10 @@ impl MetadataState { self.topic_name_index.remove(&cmd.name); - Ok(TopicDeleted { - consumer_group_epochs, - }) + for epoch in consumer_group_epochs { + self.raise_event(epoch); + } + Ok(()) } #[cfg(test)] @@ -519,15 +557,26 @@ mod tests { ]) } + fn apply_command( + sm: &mut MetadataState, + command: MetadataCommand, + ) -> Result, MetadataError> { + sm.apply(command)?; + Ok(sm.take_pending_events()) + } + fn create_topic(sm: &mut MetadataState, name: &str) -> TopicId { - let result = sm.apply(MetadataCommand::CreateTopic(CreateTopic { - name: name.to_string(), - storage_policy: default_policy(), - replica_set: replica_set(), - created_at: 1000, - })); - match result.unwrap() { - ApplyResult::TopicCreated(tc) => tc.segment_key.topic_id, + let result = apply_command( + sm, + MetadataCommand::CreateTopic(CreateTopic { + name: name.to_string(), + storage_policy: default_policy(), + replica_set: replica_set(), + created_at: 1000, + }), + ); + match result.unwrap().into_iter().next().unwrap() { + MetadataEvent::TopicCreated(tc) => tc.segment_key.topic_id, other => panic!("expected TopicCreated, got {:?}", other), } } @@ -548,8 +597,8 @@ mod tests { session_timeout_ms: 10_000, }; - let ApplyResult::ConsumerGroupChanged(epoch) = sm.apply(command.clone().into()).unwrap() - else { + let events = apply_command(&mut sm, command.clone().into()).unwrap(); + let [MetadataEvent::ConsumerGroupChanged(epoch)] = events.as_slice() else { panic!("first member must create a committed generation"); }; assert_eq!(*epoch.generation, 1); @@ -562,10 +611,7 @@ mod tests { let mut heartbeat = command; heartbeat.observed_at = 200; - assert!(matches!( - sm.apply(heartbeat.into()).unwrap(), - ApplyResult::Noop - )); + assert!(apply_command(&mut sm, heartbeat.into()).unwrap().is_empty()); assert_eq!( *sm.get_consumer_group_assignment("orders", "workers", member) .unwrap() @@ -599,14 +645,20 @@ mod tests { sealed_at: u64, intent: SegmentRollIntent, ) { - let result = sm.apply(MetadataCommand::RollSegment(RollSegment { - segment_key: SegmentKey::new(topic_id, range_id, segment_id), - sealed_at, - new_replica_set: replica_set(), - end_entry_id: None, - intent, - })); - assert!(matches!(result.unwrap(), ApplyResult::SegmentRolled(_))); + let result = apply_command( + sm, + MetadataCommand::RollSegment(RollSegment { + segment_key: SegmentKey::new(topic_id, range_id, segment_id), + sealed_at, + new_replica_set: replica_set(), + end_entry_id: None, + intent, + }), + ); + assert!(matches!( + result.unwrap().into_iter().next().unwrap(), + MetadataEvent::SegmentRolled(_) + )); } // ── D7 retention ─────────────────────────────────────────────────────── @@ -621,14 +673,20 @@ mod tests { end_entry_id: u64, sealed_at: u64, ) { - let result = sm.apply(MetadataCommand::RollSegment(RollSegment { - segment_key: SegmentKey::new(topic_id, RangeId(0), segment_id), - sealed_at, - new_replica_set: replica_set(), - end_entry_id: Some(EntryId(end_entry_id)), - intent: SegmentRollIntent::Recovery, - })); - assert!(matches!(result.unwrap(), ApplyResult::SegmentRolled(_))); + let result = apply_command( + sm, + MetadataCommand::RollSegment(RollSegment { + segment_key: SegmentKey::new(topic_id, RangeId(0), segment_id), + sealed_at, + new_replica_set: replica_set(), + end_entry_id: Some(EntryId(end_entry_id)), + intent: SegmentRollIntent::Recovery, + }), + ); + assert!(matches!( + result.unwrap().into_iter().next().unwrap(), + MetadataEvent::SegmentRolled(_) + )); } fn seg_state(sm: &MetadataState, topic_id: TopicId, segment_id: SegmentId) -> SegmentMetaState { @@ -651,12 +709,15 @@ mod tests { sm: &mut MetadataState, topic_id: TopicId, ids: &[u64], - ) -> Result { - sm.apply(MetadataCommand::DeleteSegments(DeleteSegments { - topic_id, - range_id: RangeId(0), - segment_ids: ids.iter().map(|&i| SegmentId(i)).collect(), - })) + ) -> Result, MetadataError> { + apply_command( + sm, + MetadataCommand::DeleteSegments(DeleteSegments { + topic_id, + range_id: RangeId(0), + segment_ids: ids.iter().map(|&i| SegmentId(i)).collect(), + }), + ) } #[test] @@ -664,8 +725,12 @@ mod tests { let mut sm = MetadataState::new(ShardGroupId(0)); let t = topic_with_three_sealed(&mut sm); - let result = delete_segments(&mut sm, t, &[0, 1]).unwrap(); - let ApplyResult::SegmentsDeleted(d) = result else { + let result = delete_segments(&mut sm, t, &[0, 1]) + .unwrap() + .into_iter() + .next() + .unwrap(); + let MetadataEvent::SegmentsDeleted(d) = result else { panic!("expected SegmentsDeleted, got {result:?}"); }; // All three sealed segments share one replica_set → a single group of 2 keys. @@ -683,8 +748,8 @@ mod tests { let t = topic_with_three_sealed(&mut sm); // Naming the active head (seg 3) alongside the sealed prefix: only the sealed // ones transition; the write head is skipped, never deleted. - let ApplyResult::SegmentsDeleted(d) = delete_segments(&mut sm, t, &[0, 1, 2, 3]).unwrap() - else { + let events = delete_segments(&mut sm, t, &[0, 1, 2, 3]).unwrap(); + let [MetadataEvent::SegmentsDeleted(d)] = events.as_slice() else { panic!("expected SegmentsDeleted"); }; let total_keys: usize = d.groups.iter().map(|(_, keys)| keys.len()).sum(); @@ -707,14 +772,11 @@ mod tests { let mut sm = MetadataState::new(ShardGroupId(0)); let t = topic_with_three_sealed(&mut sm); assert!(matches!( - delete_segments(&mut sm, t, &[0]).unwrap(), - ApplyResult::SegmentsDeleted(_) + delete_segments(&mut sm, t, &[0]).unwrap().as_slice(), + [MetadataEvent::SegmentsDeleted(_)] )); // Re-applying for an already-Deleting segment is a no-op. - assert!(matches!( - delete_segments(&mut sm, t, &[0]).unwrap(), - ApplyResult::Noop - )); + assert!(delete_segments(&mut sm, t, &[0]).unwrap().is_empty()); } #[test] @@ -735,8 +797,9 @@ mod tests { #[test] fn expired_prefix_empty_without_retention() { let mut sm = MetadataState::new(ShardGroupId(0)); - let t = sm - .apply(MetadataCommand::CreateTopic(CreateTopic { + let t = apply_command( + &mut sm, + MetadataCommand::CreateTopic(CreateTopic { name: "no-retention".into(), storage_policy: StoragePolicy { retention_ms: None, @@ -745,12 +808,13 @@ mod tests { }, replica_set: replica_set(), created_at: 1000, - })) - .map(|r| match r { - ApplyResult::TopicCreated(tc) => tc.segment_key.topic_id, - other => panic!("{other:?}"), - }) - .unwrap(); + }), + ) + .map(|r| match r.into_iter().next().unwrap() { + MetadataEvent::TopicCreated(tc) => tc.segment_key.topic_id, + other => panic!("{other:?}"), + }) + .unwrap(); roll_with_end(&mut sm, t, SegmentId(0), 9, 100); // Far past any window, but no policy → nothing expires. assert!( @@ -768,16 +832,27 @@ mod tests { split_point: Vec, created_at: u64, ) -> (RangeId, RangeId) { - let result = sm.apply(MetadataCommand::SplitRange(SplitRange { - topic_id, - range_id, - split_point, - created_at, - left_replica_set: replica_set(), - right_replica_set: replica_set(), - })); - match result.unwrap() { - ApplyResult::RangeSplit(rs) => (rs.children[0].0, rs.children[1].0), + let result = apply_command( + sm, + MetadataCommand::SplitRange(SplitRange { + topic_id, + range_id, + split_point, + created_at, + left_replica_set: replica_set(), + right_replica_set: replica_set(), + }), + ); + let events = result.unwrap(); + assert_eq!( + events + .iter() + .filter(|event| matches!(event, MetadataEvent::SegmentSealCommitted(_))) + .count(), + 1 + ); + match events.into_iter().next().unwrap() { + MetadataEvent::RangeSplit(split) => (split.children[0].0, split.children[1].0), other => panic!("expected RangeSplit, got {:?}", other), } } @@ -789,15 +864,26 @@ mod tests { range_id_2: RangeId, created_at: u64, ) -> RangeId { - let result = sm.apply(MetadataCommand::MergeRange(MergeRange { - topic_id, - range_id_1, - range_id_2, - created_at, - merged_replica_set: replica_set(), - })); - match result.unwrap() { - ApplyResult::RangeMerged(rm) => rm.segment_key.range_id, + let result = apply_command( + sm, + MetadataCommand::MergeRange(MergeRange { + topic_id, + range_id_1, + range_id_2, + created_at, + merged_replica_set: replica_set(), + }), + ); + let events = result.unwrap(); + assert_eq!( + events + .iter() + .filter(|event| matches!(event, MetadataEvent::SegmentSealCommitted(_))) + .count(), + 2 + ); + match events.into_iter().next().unwrap() { + MetadataEvent::RangeMerged(merged) => merged.segment_key.range_id, other => panic!("expected RangeMerged, got {:?}", other), } } @@ -822,15 +908,17 @@ mod tests { roll_segment(&mut sm, topic_id, RangeId(0), SegmentId(0), 2000); let sealed = SegmentKey::new(topic_id, RangeId(0), SegmentId(0)); - let result = sm - .apply(MetadataCommand::ReassignSegment(ReassignSegment { + let result = apply_command( + &mut sm, + MetadataCommand::ReassignSegment(ReassignSegment { segment_key: sealed, replica_set: replacement_set(), - })) - .unwrap(); + }), + ) + .unwrap(); - match result { - ApplyResult::SegmentReassigned(r) => { + match result.into_iter().next().unwrap() { + MetadataEvent::SegmentReassigned(r) => { assert_eq!(r.segment_key, sealed); assert_eq!(r.new_replica_set, replacement_set()); } @@ -849,21 +937,26 @@ mod tests { roll_segment(&mut sm, topic_id, RangeId(0), SegmentId(0), 2000); let sealed = SegmentKey::new(topic_id, RangeId(0), SegmentId(0)); - sm.apply(MetadataCommand::ReassignSegment(ReassignSegment { - segment_key: sealed, - replica_set: replacement_set(), - })) + apply_command( + &mut sm, + MetadataCommand::ReassignSegment(ReassignSegment { + segment_key: sealed, + replica_set: replacement_set(), + }), + ) .unwrap(); // Re-applying the identical set (duplicate death detection / re-proposal) // changes nothing. - let again = sm - .apply(MetadataCommand::ReassignSegment(ReassignSegment { + let again = apply_command( + &mut sm, + MetadataCommand::ReassignSegment(ReassignSegment { segment_key: sealed, replica_set: replacement_set(), - })) - .unwrap(); - assert_eq!(again, ApplyResult::Noop); + }), + ) + .unwrap(); + assert!(again.is_empty()); } #[test] @@ -873,10 +966,13 @@ mod tests { // SegmentId(0) is the active write head — no roll yet. let active = SegmentKey::new(topic_id, RangeId(0), SegmentId(0)); - let result = sm.apply(MetadataCommand::ReassignSegment(ReassignSegment { - segment_key: active, - replica_set: replacement_set(), - })); + let result = apply_command( + &mut sm, + MetadataCommand::ReassignSegment(ReassignSegment { + segment_key: active, + replica_set: replacement_set(), + }), + ); assert!(matches!(result, Err(MetadataError::SegmentNotSealed))); } @@ -886,10 +982,13 @@ mod tests { let topic_id = create_topic(&mut sm, "blue"); let unknown = SegmentKey::new(topic_id, RangeId(0), SegmentId(99)); - let result = sm.apply(MetadataCommand::ReassignSegment(ReassignSegment { - segment_key: unknown, - replica_set: replacement_set(), - })); + let result = apply_command( + &mut sm, + MetadataCommand::ReassignSegment(ReassignSegment { + segment_key: unknown, + replica_set: replacement_set(), + }), + ); assert!(matches!(result, Err(MetadataError::SegmentNotFound))); } @@ -917,12 +1016,15 @@ mod tests { let mut sm = MetadataState::new(ShardGroupId(0)); create_topic(&mut sm, "blue"); - let result = sm.apply(MetadataCommand::CreateTopic(CreateTopic { - name: "blue".to_string(), - storage_policy: default_policy(), - replica_set: replica_set(), - created_at: 2000, - })); + let result = apply_command( + &mut sm, + MetadataCommand::CreateTopic(CreateTopic { + name: "blue".to_string(), + storage_policy: default_policy(), + replica_set: replica_set(), + created_at: 2000, + }), + ); assert_eq!(result, Err(TopicNameAlreadyExists("blue".to_string()))); } @@ -992,13 +1094,16 @@ mod tests { #[test] fn roll_segment_bad_topic() { let mut sm = MetadataState::new(ShardGroupId(0)); - let result = sm.apply(MetadataCommand::RollSegment(RollSegment { - segment_key: SegmentKey::new(TopicId(99), RangeId(0), SegmentId(0)), - sealed_at: 2000, - new_replica_set: replica_set(), - end_entry_id: None, - intent: SegmentRollIntent::Recovery, - })); + let result = apply_command( + &mut sm, + MetadataCommand::RollSegment(RollSegment { + segment_key: SegmentKey::new(TopicId(99), RangeId(0), SegmentId(0)), + sealed_at: 2000, + new_replica_set: replica_set(), + end_entry_id: None, + intent: SegmentRollIntent::Recovery, + }), + ); assert_eq!(result, Err(TopicNotFound(TopicId(99)))); } @@ -1007,13 +1112,16 @@ mod tests { let mut sm = MetadataState::new(ShardGroupId(0)); let tid = create_topic(&mut sm, "blue"); - let result = sm.apply(MetadataCommand::RollSegment(RollSegment { - segment_key: SegmentKey::new(tid, RangeId(99), SegmentId(0)), - sealed_at: 2000, - new_replica_set: replica_set(), - end_entry_id: None, - intent: SegmentRollIntent::Recovery, - })); + let result = apply_command( + &mut sm, + MetadataCommand::RollSegment(RollSegment { + segment_key: SegmentKey::new(tid, RangeId(99), SegmentId(0)), + sealed_at: 2000, + new_replica_set: replica_set(), + end_entry_id: None, + intent: SegmentRollIntent::Recovery, + }), + ); assert_eq!(result, Err(RangeNotFound)); } @@ -1022,14 +1130,17 @@ mod tests { let mut sm = MetadataState::new(ShardGroupId(0)); let tid = create_topic(&mut sm, "blue"); - let result = sm.apply(MetadataCommand::RollSegment(RollSegment { - segment_key: SegmentKey::new(tid, RangeId(0), SegmentId(99)), - sealed_at: 2000, - new_replica_set: replica_set(), - end_entry_id: None, - intent: SegmentRollIntent::Recovery, - })); - assert_eq!(result, Ok(ApplyResult::Noop)); + let result = apply_command( + &mut sm, + MetadataCommand::RollSegment(RollSegment { + segment_key: SegmentKey::new(tid, RangeId(0), SegmentId(99)), + sealed_at: 2000, + new_replica_set: replica_set(), + end_entry_id: None, + intent: SegmentRollIntent::Recovery, + }), + ); + assert_eq!(result, Ok(vec![])); let range = &sm.get_topic(&tid).unwrap().ranges[&RangeId(0)]; assert_eq!(range.active_segment, Some(SegmentId(0))); @@ -1085,25 +1196,31 @@ mod tests { #[test] fn split_range_fixed_rejected() { let mut sm = MetadataState::new(ShardGroupId(0)); - let result = sm.apply(MetadataCommand::CreateTopic(CreateTopic { - name: "ordered".to_string(), - storage_policy: fixed_policy(), - replica_set: replica_set(), - created_at: 1000, - })); - let tid = match result.unwrap() { - ApplyResult::TopicCreated(tc) => tc.segment_key.topic_id, + let result = apply_command( + &mut sm, + MetadataCommand::CreateTopic(CreateTopic { + name: "ordered".to_string(), + storage_policy: fixed_policy(), + replica_set: replica_set(), + created_at: 1000, + }), + ); + let tid = match result.unwrap().into_iter().next().unwrap() { + MetadataEvent::TopicCreated(tc) => tc.segment_key.topic_id, other => panic!("expected TopicCreated, got {:?}", other), }; - let split_result = sm.apply(MetadataCommand::SplitRange(SplitRange { - topic_id: tid, - range_id: RangeId(0), - split_point: vec![0x80], - created_at: 2000, - left_replica_set: replica_set(), - right_replica_set: replica_set(), - })); + let split_result = apply_command( + &mut sm, + MetadataCommand::SplitRange(SplitRange { + topic_id: tid, + range_id: RangeId(0), + split_point: vec![0x80], + created_at: 2000, + left_replica_set: replica_set(), + right_replica_set: replica_set(), + }), + ); assert_eq!(split_result, Err(SplitNotAllowed(tid))); } @@ -1112,24 +1229,30 @@ mod tests { let mut sm = MetadataState::new(ShardGroupId(0)); let tid = create_topic(&mut sm, "blue"); - let upper_bound = sm.apply(MetadataCommand::SplitRange(SplitRange { - topic_id: tid, - range_id: RangeId(0), - split_point: vec![0xFF], - created_at: 2000, - left_replica_set: replica_set(), - right_replica_set: replica_set(), - })); + let upper_bound = apply_command( + &mut sm, + MetadataCommand::SplitRange(SplitRange { + topic_id: tid, + range_id: RangeId(0), + split_point: vec![0xFF], + created_at: 2000, + left_replica_set: replica_set(), + right_replica_set: replica_set(), + }), + ); assert_eq!(upper_bound, Err(InvalidSplitPoint)); - let lower_bound = sm.apply(MetadataCommand::SplitRange(SplitRange { - topic_id: tid, - range_id: RangeId(0), - split_point: vec![], - created_at: 2000, - left_replica_set: replica_set(), - right_replica_set: replica_set(), - })); + let lower_bound = apply_command( + &mut sm, + MetadataCommand::SplitRange(SplitRange { + topic_id: tid, + range_id: RangeId(0), + split_point: vec![], + created_at: 2000, + left_replica_set: replica_set(), + right_replica_set: replica_set(), + }), + ); assert_eq!(lower_bound, Err(InvalidSplitPoint)); } @@ -1200,13 +1323,16 @@ mod tests { let (c1, c2) = split_range(&mut sm, tid, RangeId(0), vec![0x80], 2000); let (c1a, _c1b) = split_range(&mut sm, tid, c1, vec![0x40], 3000); - let result = sm.apply(MetadataCommand::MergeRange(MergeRange { - topic_id: tid, - range_id_1: c1a, - range_id_2: c2, - created_at: 4000, - merged_replica_set: replica_set(), - })); + let result = apply_command( + &mut sm, + MetadataCommand::MergeRange(MergeRange { + topic_id: tid, + range_id_1: c1a, + range_id_2: c2, + created_at: 4000, + merged_replica_set: replica_set(), + }), + ); assert_eq!(result, Err(RangesNotAdjacent)); } @@ -1217,9 +1343,12 @@ mod tests { let mut sm = MetadataState::new(ShardGroupId(0)); let tid = create_topic(&mut sm, "blue"); - sm.apply(MetadataCommand::DeleteTopic(DeleteTopic { - name: "blue".into(), - })) + apply_command( + &mut sm, + MetadataCommand::DeleteTopic(DeleteTopic { + name: "blue".into(), + }), + ) .unwrap(); let topic = sm.get_topic(&tid).unwrap(); @@ -1239,9 +1368,12 @@ mod tests { let mut sm = MetadataState::new(ShardGroupId(0)); let _tid = create_topic(&mut sm, "blue"); - sm.apply(MetadataCommand::DeleteTopic(DeleteTopic { - name: "blue".into(), - })) + apply_command( + &mut sm, + MetadataCommand::DeleteTopic(DeleteTopic { + name: "blue".into(), + }), + ) .unwrap(); assert!(sm.get_topic_by_name("blue").is_none()); @@ -1250,9 +1382,12 @@ mod tests { #[test] fn delete_topic_nonexistent() { let mut sm = MetadataState::new(ShardGroupId(0)); - let result = sm.apply(MetadataCommand::DeleteTopic(DeleteTopic { - name: "nope".into(), - })); + let result = apply_command( + &mut sm, + MetadataCommand::DeleteTopic(DeleteTopic { + name: "nope".into(), + }), + ); assert_eq!(result, Err(MetadataError::TopicNameNotFound("nope".into()))); } @@ -1311,14 +1446,17 @@ mod tests { roll_segment(&mut sm, tid, RangeId(0), SegmentId(0), 2000); - let result = sm.apply(MetadataCommand::RollSegment(RollSegment { - segment_key: SegmentKey::new(tid, RangeId(0), SegmentId(0)), - sealed_at: 3000, - new_replica_set: replica_set(), - end_entry_id: None, - intent: SegmentRollIntent::DataPressure, - })); - assert_eq!(result, Ok(ApplyResult::Noop)); + let result = apply_command( + &mut sm, + MetadataCommand::RollSegment(RollSegment { + segment_key: SegmentKey::new(tid, RangeId(0), SegmentId(0)), + sealed_at: 3000, + new_replica_set: replica_set(), + end_entry_id: None, + intent: SegmentRollIntent::DataPressure, + }), + ); + assert_eq!(result, Ok(vec![])); let range = &sm.get_topic(&tid).unwrap().ranges[&RangeId(0)]; assert_eq!(range.active_segment, Some(SegmentId(1))); @@ -1330,15 +1468,18 @@ mod tests { let mut sm = MetadataState::new(ShardGroupId(0)); let tid = create_topic(&mut sm, "blue"); - let result = sm.apply(MetadataCommand::RollSegment(RollSegment { - segment_key: SegmentKey::new(tid, RangeId(0), SegmentId(0)), - sealed_at: 2000, - new_replica_set: replica_set(), - end_entry_id: Some(EntryId(10)), - intent: SegmentRollIntent::BoundaryCorrection, - })); + let result = apply_command( + &mut sm, + MetadataCommand::RollSegment(RollSegment { + segment_key: SegmentKey::new(tid, RangeId(0), SegmentId(0)), + sealed_at: 2000, + new_replica_set: replica_set(), + end_entry_id: Some(EntryId(10)), + intent: SegmentRollIntent::BoundaryCorrection, + }), + ); - assert_eq!(result, Ok(ApplyResult::Noop)); + assert_eq!(result, Ok(vec![])); let range = &sm.get_topic(&tid).unwrap().ranges[&RangeId(0)]; assert_eq!(range.active_segment, Some(SegmentId(0))); assert_eq!(range.load_state, RangeLoadState::Unclassified); @@ -1456,14 +1597,17 @@ mod tests { #[test] fn no_auto_proposal_for_fixed_strategy() { let mut sm = MetadataState::new(ShardGroupId(0)); - let result = sm.apply(MetadataCommand::CreateTopic(CreateTopic { - name: "ordered".to_string(), - storage_policy: fixed_policy(), - replica_set: replica_set(), - created_at: 1000, - })); - let tid = match result.unwrap() { - ApplyResult::TopicCreated(tc) => tc.segment_key.topic_id, + let result = apply_command( + &mut sm, + MetadataCommand::CreateTopic(CreateTopic { + name: "ordered".to_string(), + storage_policy: fixed_policy(), + replica_set: replica_set(), + created_at: 1000, + }), + ); + let tid = match result.unwrap().into_iter().next().unwrap() { + MetadataEvent::TopicCreated(tc) => tc.segment_key.topic_id, other => panic!("expected TopicCreated, got {:?}", other), }; @@ -1608,21 +1752,24 @@ mod tests { let mut sm = MetadataState::new(ShardGroupId(0)); let tid = create_topic(&mut sm, "blue"); - let result = sm.apply(MetadataCommand::RollSegment(RollSegment { - segment_key: SegmentKey::new(tid, RangeId(0), SegmentId(0)), - sealed_at: 2000, - new_replica_set: replica_set(), - end_entry_id: Some(EntryId(42000)), - intent: SegmentRollIntent::DataPressure, - })); + let result = apply_command( + &mut sm, + MetadataCommand::RollSegment(RollSegment { + segment_key: SegmentKey::new(tid, RangeId(0), SegmentId(0)), + sealed_at: 2000, + new_replica_set: replica_set(), + end_entry_id: Some(EntryId(42000)), + intent: SegmentRollIntent::DataPressure, + }), + ); let result = result.unwrap(); assert!(matches!( - result, - ApplyResult::SegmentRolled(SegmentRolled { + result.as_slice(), + [MetadataEvent::SegmentRolled(SegmentRolled { end_entry_id: Some(EntryId(42000)), .. - }) + })] )); let range = &sm.get_topic(&tid).unwrap().ranges[&RangeId(0)]; @@ -1640,13 +1787,16 @@ mod tests { let tid = create_topic(&mut sm, "blue"); // Death-triggered roll with end_entry_id=0 (placeholder) - let _ = sm.apply(MetadataCommand::RollSegment(RollSegment { - segment_key: SegmentKey::new(tid, RangeId(0), SegmentId(0)), - sealed_at: 2000, - new_replica_set: replica_set(), - end_entry_id: None, - intent: SegmentRollIntent::Recovery, - })); + let _ = apply_command( + &mut sm, + MetadataCommand::RollSegment(RollSegment { + segment_key: SegmentKey::new(tid, RangeId(0), SegmentId(0)), + sealed_at: 2000, + new_replica_set: replica_set(), + end_entry_id: None, + intent: SegmentRollIntent::Recovery, + }), + ); assert_eq!( sm.get_topic(&tid).unwrap().ranges[&RangeId(0)].segments[&SegmentId(0)].end_entry_id, @@ -1654,13 +1804,16 @@ mod tests { ); // Segment leader's RollSegment arrives with correct end_entry_id - let result = sm.apply(MetadataCommand::RollSegment(RollSegment { - segment_key: SegmentKey::new(tid, RangeId(0), SegmentId(0)), - sealed_at: 2500, - new_replica_set: replica_set(), - end_entry_id: Some(EntryId(42000)), - intent: SegmentRollIntent::Recovery, - })); + let result = apply_command( + &mut sm, + MetadataCommand::RollSegment(RollSegment { + segment_key: SegmentKey::new(tid, RangeId(0), SegmentId(0)), + sealed_at: 2500, + new_replica_set: replica_set(), + end_entry_id: Some(EntryId(42000)), + intent: SegmentRollIntent::Recovery, + }), + ); assert!(result.is_ok()); let range = &sm.get_topic(&tid).unwrap().ranges[&RangeId(0)]; @@ -1677,22 +1830,28 @@ mod tests { let tid = create_topic(&mut sm, "blue"); // Normal roll with actual end_entry_id - let _ = sm.apply(MetadataCommand::RollSegment(RollSegment { - segment_key: SegmentKey::new(tid, RangeId(0), SegmentId(0)), - sealed_at: 2000, - new_replica_set: replica_set(), - end_entry_id: Some(EntryId(1000)), - intent: SegmentRollIntent::DataPressure, - })); + let _ = apply_command( + &mut sm, + MetadataCommand::RollSegment(RollSegment { + segment_key: SegmentKey::new(tid, RangeId(0), SegmentId(0)), + sealed_at: 2000, + new_replica_set: replica_set(), + end_entry_id: Some(EntryId(1000)), + intent: SegmentRollIntent::DataPressure, + }), + ); // Duplicate roll is rejected (end_offset already set) - let result = sm.apply(MetadataCommand::RollSegment(RollSegment { - segment_key: SegmentKey::new(tid, RangeId(0), SegmentId(0)), - sealed_at: 2500, - new_replica_set: replica_set(), - end_entry_id: Some(EntryId(42000)), - intent: SegmentRollIntent::DataPressure, - })); - assert_eq!(result, Ok(ApplyResult::Noop)); + let result = apply_command( + &mut sm, + MetadataCommand::RollSegment(RollSegment { + segment_key: SegmentKey::new(tid, RangeId(0), SegmentId(0)), + sealed_at: 2500, + new_replica_set: replica_set(), + end_entry_id: Some(EntryId(42000)), + intent: SegmentRollIntent::DataPressure, + }), + ); + assert_eq!(result, Ok(vec![])); } } diff --git a/src/control_plane/metadata/topic.rs b/src/control_plane/metadata/topic.rs index 2ddd28e9..55b9f893 100644 --- a/src/control_plane/metadata/topic.rs +++ b/src/control_plane/metadata/topic.rs @@ -289,6 +289,26 @@ impl TopicMeta { Some(seg) } + pub(crate) fn active_segment_key_and_replicas( + &self, + range_id: RangeId, + ) -> Result<(SegmentKey, Replicas), MetadataError> { + let range = self + .ranges + .get(&range_id) + .ok_or(MetadataError::RangeNotFound)?; + + let segment_id = range.validate_active()?; + let segment = range + .segments + .get(&segment_id) + .ok_or(MetadataError::SegmentNotFound)?; + Ok(( + SegmentKey::new(self.id, range_id, segment_id), + segment.replica_set.clone(), + )) + } + pub(crate) fn get_mut( &mut self, segment_key: SegmentKey,