This is an automated email from the ASF dual-hosted git repository. hubcio pushed a commit to branch durable-offset-watermark in repository https://gitbox.apache.org/repos/asf/iggy.git
commit 71a5672b19cc9a498dc95b0f1d2878a51a210605 Author: Hubert Gruszecki <[email protected]> AuthorDate: Fri Sep 4 11:35:41 2026 +0200 refactor(partitions): drop the redundant first-claim gate The create-time offset claim was guarded by `mint_frontier() == 0` so it would skip a fenced rebuild. That guard never decided anything: `reserve_offsets_through(0)` returns on its own coverage check, and a record with a live offset space always carries a reservation above zero, so the skip came from the argument and not the gate. Deleting it leaves one untested condition fewer and the same behaviour. Three comments beside it described a retry that cannot happen. A create that fails leaves its directory behind, and the reconciler routes any namespace with a directory to the loader, so nothing re-enters this builder. The claim still belongs last, but for the reason those comments missed: a reservation written before a failing step outlives the create, and the loader then resumes the append point at it, holing every offset below on a partition that never took a write. The claim failure also rendered as a packed namespace integer where its neighbours spell the stream, topic and partition. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01K8yxwD21a8EzjyxFHLiCLE --- core/partitions/src/iggy_partition.rs | 4 ++-- core/server/src/partition_helpers.rs | 33 +++++++++++++++++++++------------ core/server/src/server_error.rs | 12 ++++++++++-- 3 files changed, 33 insertions(+), 16 deletions(-) diff --git a/core/partitions/src/iggy_partition.rs b/core/partitions/src/iggy_partition.rs index 69d625579..94c577ad6 100644 --- a/core/partitions/src/iggy_partition.rs +++ b/core/partitions/src/iggy_partition.rs @@ -6811,8 +6811,8 @@ mod tests { fn given_a_storeless_partition_when_ticking_should_not_extend() { let mut partition = solo_recording_partition(); partition.set_offset_reservation_lease(test_lease(16)); - // Without this the untouched offset space would satisfy the gate on its - // own and the store check would go untested. + // Without this the assert would pass on `!append_live` alone, leaving + // the store check it is named for untested. partition.note_append_live(); assert!(partition.superblock.is_none(), "the premise: no store"); assert!(!partition.needs_offset_reservation_extension()); diff --git a/core/server/src/partition_helpers.rs b/core/server/src/partition_helpers.rs index b55aacf42..0ee71dadc 100644 --- a/core/server/src/partition_helpers.rs +++ b/core/server/src/partition_helpers.rs @@ -1113,7 +1113,8 @@ fn hydrate_reopen_error( /// already on disk is routed through the loader instead, so a prior /// life's segments are hydrated rather than built over. /// -/// Steps performed (all idempotent on retry after a partial failure): +/// Steps performed. 1 to 4 are idempotent on retry after a partial failure; the +/// claim is last precisely because it is not (see its own comment): /// 1. Create directory hierarchy on disk. /// 2. Build per-partition VSR consensus group, resuming any superblock-recorded view. /// 3. Configure empty consumer-offset storage with the on-disk paths set. @@ -1311,26 +1312,34 @@ pub async fn build_partition_fresh( // the reconciler's addition loop, so it lengthens the window a produce // arriving with the create spends parked. // - // LAST of the steps, so the rest stay idempotent on retry: a failure between - // the claim and the return would burn a lease block per reconciler pass. + // LAST of the steps, because a claim written before a step that then fails + // outlives the create. The reconciler routes any namespace whose directory + // exists to `load_partition_or_fence`, and step 1 made that directory, so + // the retry comes back through the loader: `restore_offset_frontier` there + // resumes the append point at the recorded reservation and holes every + // offset below it on a partition that never took a write. // - // No-op above one replica and with no store attached, where nothing is - // reserved. Skipped once the offset space is live (`mint_frontier` reads 0 - // only while it is not): a rebuild resumes its append point exactly ON the - // reservation it recovered, never above it, so an unconditional claim would - // write and burn a block every time. It pays one inline fence on its first - // send instead, which is what a graceful stop and boot already costs. + // `0`, not `mint_frontier()`: a rebuild recovers its append point exactly ON + // the reservation it recorded, so asking to cover the frontier would fail + // the callee's strict `>` and rewrite the record on every rebuild. Asking + // only for offset 0 leaves that same check to skip every partition already + // carrying a reservation, which pays one inline fence on its first send + // instead, the cost a graceful stop and boot already carries. No-op above + // one replica and with no store attached, where nothing is reserved. // // The shard tick takes over from the first mint onward // (`needs_offset_reservation_extension`), which stays gated on a partition // that has minted so boot cannot write a superblock per idle partition. - if partition.mint_frontier() == 0 && !partition.reserve_offsets_through(0).await { + if !partition.reserve_offsets_through(0).await { // Not degraded-but-live: the failed write armed the group's superblock // retry backoff, and `reserve_offsets_through_retryable` refuses every // send arriving inside it with a transient the HTTP plane does not - // replay. The reconciler backs the namespace off and retries with a - // fresh partition, whose backoff cell starts clear. + // replay. The reconciler backs the namespace off; the retry materialises + // through the loader, whose partition carries a clear backoff cell. return Err(ServerError::PartitionOffsetReservationClaim { + stream_id, + topic_id, + partition_id, namespace_raw: namespace.inner(), }); } diff --git a/core/server/src/server_error.rs b/core/server/src/server_error.rs index dee1fa0ac..b9e744755 100644 --- a/core/server/src/server_error.rs +++ b/core/server/src/server_error.rs @@ -206,8 +206,16 @@ pub enum ServerError { /// backoff, and a send arriving inside that window is refused with a /// transient the HTTP plane does not replay. `namespace_raw` joins this to /// the write's own `iggy.partitions.diag` line, which carries the cause. - #[error("partition namespace {namespace_raw} could not claim its first offset reservation")] - PartitionOffsetReservationClaim { namespace_raw: u64 }, + #[error( + "partition {stream_id}/{topic_id}/{partition_id} (namespace {namespace_raw}) could not \ + claim its first offset reservation" + )] + PartitionOffsetReservationClaim { + stream_id: usize, + topic_id: usize, + partition_id: usize, + namespace_raw: u64, + }, #[error( "shard {shard_id} aborted while waiting for shard-0 to broadcast the metadata \ factory bundle; shard 0 dropped its sender (most likely it failed to recover)"
