krishvishal commented on code in PR #3975:
URL: https://github.com/apache/iggy/pull/3975#discussion_r3907460818
##########
core/shard/src/lib.rs:
##########
@@ -6554,6 +6573,26 @@ where
}
continue;
}
+ // Same bound the metadata plane fail-stops on, applied per group.
A
+ // partition whose superblock keeps refusing withholds every
+ // view-scoped send AND refuses every append, so it serves nothing
+ // while the process still reports healthy.
+ let superblock_failures = partition.superblock_write_failures();
+ if superblock_wedged(
Review Comment:
Fixed. Comment now says the bound exits the node, scopes "refuses every
append" to solo, and the timeout's operator text covers both planes.
##########
core/partitions/src/iggy_partition.rs:
##########
@@ -975,6 +1145,46 @@ where
self.consensus.clock_realtime_micros() <
self.superblock_retry_after_micros.get()
}
+ /// Drop the reservation back onto the frontier, once a graceful flush has
+ /// made the segments account for every offset this replica confirmed.
+ ///
+ /// The reservation is there for the crash case, where they do not. Left
+ /// standing it would make every ordinary restart resume a lease block
+ /// higher and hole the offset space for nothing.
+ ///
+ /// Collapses onto [`Self::mint_frontier`], not [`Self::offset_frontier`]:
the
+ /// append point is what the next boot has to resume at, and on a boot that
+ /// consumed a reservation without appending it is the reservation itself,
so
+ /// reading the committed frontier here would write a record BELOW what an
+ /// earlier life already confirmed to a client. A clean stop is the runbook
+ /// answer to an incident, which would make it the one action that undoes
the
+ /// protection.
+ ///
+ /// The frontier field still records only what is held: a graceful stop
+ /// flushes the committed prefix, but the journal can hold an uncommitted
tail
+ /// that the next view legitimately truncates.
+ ///
+ /// Callers must have flushed FIRST, and must not call this when the flush
+ /// failed: the claim it makes is precisely that the flush succeeded.
+ #[allow(clippy::future_not_send)]
+ #[must_use = "the bool is the durability verdict; a failed collapse leaves
a gap"]
+ pub async fn collapse_offset_reservation(&self) -> bool {
+ let append_point = self.mint_frontier();
+ if self.durable_offset_reserved.get() <= append_point {
+ return true;
+ }
+ let Some(superblock) = self.superblock.as_ref().map(Rc::clone) else {
+ return true;
+ };
+ if self.superblock_write_is_backed_off() {
Review Comment:
Fixed. `collapse_offset_reservation` bypasses the backoff gate, same
rationale as `record_frontier_before_quarantine`.
##########
core/partitions/src/iggy_partition.rs:
##########
@@ -4189,41 +4613,245 @@ where
Ok(())
}
- /// Record the purge's frontier reset BEFORE the purge touches anything.
+ /// Re-anchor the append point after boot re-seeded the offset counter
above
+ /// what the recovered segment chain holds.
///
- /// The unlinks are made durable by their own directory fsync, so a crash
- /// between them and a reset written afterwards boots a purged directory
- /// whose record still names the pre-purge offset space:
- /// `restore_offset_frontier` re-seeds the counter to it while every peer
- /// restarted at 0, and the first append stamps a `base_offset` and
- /// `batch_checksum` no peer shares. Writing 0 first inverts the window
into
- /// a harmless one -- the record under-claims while the segments still
- /// exist, and boot takes the max of the record and what the segments
prove.
+ /// A hole INSIDE a segment is not survivable: `recover_segment_bounds`
walks
+ /// a segment from its FILENAME with a running `expected_offset` and
REFUSES
+ /// at the first offset that does not continue it (`OffsetDiscontinuity`),
+ /// which on a solo group tombstones the partition. A surviving index does
not
+ /// help: a first entry that is not the file-name offset makes recovery
+ /// discard the index and walk from byte 0, reaching the same refusal. On a
+ /// segment BOUNDARY every reader copes -- absolute offsets in the index,
+ /// `disk_poll_start` walking on into later segments, and a chain guard
that
+ /// admits a forward gap the reservation covers.
///
- /// Spelled out rather than read off the counter, which still holds the
- /// pre-purge frontier at this point.
+ /// So an empty tail is unlinked (its name claims a range it does not
hold), a
+ /// sized tail (the only copy of its messages) is sealed with a fresh
segment
+ /// planted at the append point, and a chain the unlinks emptied is planted
+ /// directly -- `ensure_initial_segment` names its segment for the
COMMITTED
+ /// frontier and would put the first mint inside it.
///
/// # Errors
- /// [`PurgeError::FrontierNotRecorded`]. Refused rather than logged:
nothing
- /// has been mutated yet, and a purge that cannot record its reset must not
- /// be the one that erases the data proving the old frontier. The caller
- /// RETRIES; it must not fence, since the chain is still whole and the live
- /// counter still names the pre-purge space.
+ /// [`IggyError`] when the fresh segment cannot be created, leaving the
+ /// partition without a serviceable chain.
#[allow(clippy::future_not_send)]
- async fn record_purge_frontier_reset(&mut self, generation: u64) ->
Result<(), PurgeError> {
- if self.reset_offset_frontier_at(0).await {
- self.purge_deferred = false;
+ pub async fn reanchor_to_offset_frontier(
Review Comment:
Fixed. PR description gained a wipe instruction for earlier-push data dirs;
the upgrade paragraph above it was backwards and is corrected too.
##########
core/partitions/src/iggy_partition.rs:
##########
@@ -4189,41 +4613,245 @@ where
Ok(())
}
- /// Record the purge's frontier reset BEFORE the purge touches anything.
+ /// Re-anchor the append point after boot re-seeded the offset counter
above
+ /// what the recovered segment chain holds.
///
- /// The unlinks are made durable by their own directory fsync, so a crash
- /// between them and a reset written afterwards boots a purged directory
- /// whose record still names the pre-purge offset space:
- /// `restore_offset_frontier` re-seeds the counter to it while every peer
- /// restarted at 0, and the first append stamps a `base_offset` and
- /// `batch_checksum` no peer shares. Writing 0 first inverts the window
into
- /// a harmless one -- the record under-claims while the segments still
- /// exist, and boot takes the max of the record and what the segments
prove.
+ /// A hole INSIDE a segment is not survivable: `recover_segment_bounds`
walks
+ /// a segment from its FILENAME with a running `expected_offset` and
REFUSES
+ /// at the first offset that does not continue it (`OffsetDiscontinuity`),
+ /// which on a solo group tombstones the partition. A surviving index does
not
+ /// help: a first entry that is not the file-name offset makes recovery
+ /// discard the index and walk from byte 0, reaching the same refusal. On a
+ /// segment BOUNDARY every reader copes -- absolute offsets in the index,
+ /// `disk_poll_start` walking on into later segments, and a chain guard
that
+ /// admits a forward gap the reservation covers.
///
- /// Spelled out rather than read off the counter, which still holds the
- /// pre-purge frontier at this point.
+ /// So an empty tail is unlinked (its name claims a range it does not
hold), a
+ /// sized tail (the only copy of its messages) is sealed with a fresh
segment
+ /// planted at the append point, and a chain the unlinks emptied is planted
+ /// directly -- `ensure_initial_segment` names its segment for the
COMMITTED
+ /// frontier and would put the first mint inside it.
///
/// # Errors
- /// [`PurgeError::FrontierNotRecorded`]. Refused rather than logged:
nothing
- /// has been mutated yet, and a purge that cannot record its reset must not
- /// be the one that erases the data proving the old frontier. The caller
- /// RETRIES; it must not fence, since the chain is still whole and the live
- /// counter still names the pre-purge space.
+ /// [`IggyError`] when the fresh segment cannot be created, leaving the
+ /// partition without a serviceable chain.
#[allow(clippy::future_not_send)]
- async fn record_purge_frontier_reset(&mut self, generation: u64) ->
Result<(), PurgeError> {
- if self.reset_offset_frontier_at(0).await {
- self.purge_deferred = false;
+ pub async fn reanchor_to_offset_frontier(
+ &mut self,
+ config: &PartitionsConfig,
+ ) -> Result<(), IggyError> {
+ // Where the next append will land: the counter, or an armed mint floor
+ // above it. The floor is the whole reason a hole can appear, so
+ // anchoring to the counter alone would leave the chain as unprepared.
+ let frontier = self.mint_frontier();
+ if frontier == 0 {
return Ok(());
}
- self.purge_deferred = true;
- // The ONLY operator-visible signal for the withhold: `send_prepare_ok`
- // returns silently, correctly, since it runs per prepare. So this line
- // has to say that the replica is now out of quorum for this group, or
- // the symptom reads as a network fault. The consecutive count
- // correlates it with the superblock writer's own error log, which
- // carries the `ENOSPC` / `EIO` cause but is rate-limited to
- // power-of-two failures, while this deferral repeats per reconciler
- // pass.
+ let namespace = self.namespace();
+ let mut retired = 0usize;
+ while let Some(segment) = self.log.segments().last() {
+ if segment.size.as_bytes_u64() > 0 || segment.start_offset >=
frontier {
+ break;
+ }
+ let Some((segment, mut storage)) = self.log.retire_back() else {
+ break;
+ };
+ let (messages_path, index_path) =
storage.segment_and_index_paths();
+ let _ = storage.shutdown();
+ drop(storage);
+ for path in messages_path.into_iter().chain(index_path) {
+ match compio::fs::remove_file(&path).await {
+ Ok(()) => {}
+ Err(error) if error.kind() == std::io::ErrorKind::NotFound
=> {}
+ Err(error) => {
+ // Refused, not logged. The segment is already out of
the
+ // in-memory chain, so a file left behind becomes a
+ // non-tail empty segment as soon as the plant lands --
+ // `[sized][stale empty][planted]` -- which the next
boot
+ // refuses outright as `EmptyNonTailSegment`. Failing
boot
+ // here says so while the directory is still readable.
+ error!(
+ target: "iggy.partitions.diag",
+ plane = "partitions",
+ namespace_raw = namespace.inner(),
+ path = %path,
+ %error,
+ "failed to unlink a stale empty segment during the
boot \
+ re-anchor; refusing to plant beside it"
+ );
+ return Err(IggyError::CannotDeleteFile);
+ }
+ }
+ }
+ tracing::info!(
+ target: "iggy.partitions.diag",
+ plane = "partitions",
+ namespace_raw = namespace.inner(),
+ start_offset = segment.start_offset,
+ offset_frontier = frontier,
+ "unlinked an empty segment named below the restored offset
frontier"
+ );
+ // Boot DOES count the recovered chain -- `load_persisted_segments`
+ // increments per segment before it looks at the size, so empty
tails
+ // are in the total -- and retention pairs its own retire with a
+ // decrement. Without this the count stays one high on the wire for
+ // the life of the process.
+ self.stats.decrement_segments_count(1);
+ retired += 1;
+ }
+ // Durable before anything is planted beside them: a crash in between
Review Comment:
Fixed. The re-anchor's directory fsync failure returns `CannotSyncFile`
instead of warning, so the promise no longer rests on `write_anchor`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
