numinnex commented on code in PR #3786:
URL: https://github.com/apache/iggy/pull/3786#discussion_r3699675580
##########
core/server-ng/src/partition_reconciler.rs:
##########
@@ -512,6 +589,78 @@ async fn reconcile_additions(
}
}
+/// Answer parked frames for namespaces this shard is not going to materialise.
+///
+/// `park_if_unmaterialised` holds a frame until `ReconcileOp::InsertOwned`
lands
+/// for its namespace, and the only other things that drain the entry are
+/// `ConfirmRemove` and `RemoveRouted`. Neither can name a namespace that was
+/// never built: it is absent from `IggyPartitions` (so `reconcile_removals`
+/// sees no owned ghost) and absent from `shards_table` (the owner seeds a row
+/// only via `InsertOwned`, and emits `InsertRouted` only for namespaces it
does
+/// NOT own). So without this sweep the frames are held for the process
+/// lifetime and every waiting client burns its full response read-timeout.
+///
+/// Immediate reclaim needs positive evidence that the build will not finish.
Two
+/// signals carry it: `build_partition_fresh` failed (ENOSPC, EPERM) and is
backed
+/// off -- the backoff clamps at 60s, well past the client's 30s read timeout,
so
+/// holding the frames cannot help -- or the namespace does not hash to this
shard
+/// at all, so no `InsertOwned` for it will ever land here.
+///
+/// Absence from the target set is NOT that evidence, which is why this no
longer
+/// consults it. "Not in the target" covers a namespace that left committed
+/// metadata AND one this replica has simply not applied yet, and those are
+/// indistinguishable from local state: `snapshot_target_namespaces` reads this
+/// node's committed metadata, so a metadata-lagging backup reports a
namespace it
+/// is milliseconds from committing exactly as it reports a deleted one.
Reclaiming
+/// on that reading destroys the in-flight traffic the park buffer exists to
hold
+/// (silently, for a replicated prepare, which has no client to answer). The
stale
+/// reading was doubly wrong: `target_set` is snapshotted before
+/// `reconcile_additions` awaits `build_partition_fresh`, so a topic committing
+/// during those awaits was judged against a set that predates it.
+///
+/// Everything without that evidence -- building, still committing, or
genuinely
+/// deleted -- is aged instead.
[`shard::IggyShard::age_parked_partition_frames`]
+/// answers frames past `MAX_PARKED_PASSES`, so residency stays bounded and no
+/// client waits out its read timeout; the deleted case simply takes a few
passes
+/// rather than one. The bound is residency only -- the SDK replays the
identical
+/// request, so answering a late frame does not stop its operation from being
+/// applied late (see `ParkedFrame::passes`).
+fn reconcile_parked_frames(ctx: &ReconcilerCtx, counters: &mut PassCounters) {
+ let parked = ctx.shard.parked_namespaces();
+ if parked.is_empty() {
+ return;
+ }
+ let partitions = ctx.shard.plane.partitions();
+ let total_shards = u32::from(ctx.total_shards);
+ let now = Instant::now();
+ for ns in parked {
+ if partitions.contains(&ns) {
Review Comment:
@/tmp/claude-1000/-home-songbird-projects-iggy/f7e8c0bd-ec61-465a-a1e5-287a1beea631/scratchpad/pr3786-comment.md
--
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]
