fleetd #726 unit 3 fix: release the single-flight claim when continuationRunner rejects the hand-off
confirm() takes the per-lead-terminal claim before handing the roll to continuationRunner, and the only release path was runRollover's own finally. If continuationRunner.accept itself throws, runRollover never starts, so that finally never runs, and nothing else ever writes rollingByTerminal — the claim is held forever and the terminal can never be rolled again. This differs from fleetd #615, which covers a throw INSIDE the continuation (runRollover already catches that and still releases the claim) — this is a throw from the hand-off itself, which is not reachable with today's virtual-thread runner but would be with a bounded executor's RejectedExecutionException. confirm() now catches that throw, releases the claim, and overwrites the IN_PROGRESS outcome with a terminal FAILED one, matching how a throw inside the continuation is already surfaced.
This commit is contained in:
@@ -524,7 +524,23 @@ public final class LeadRollover {
|
||||
pending.remove(token);
|
||||
log.info("lead-rollover: confirmed token={} lead={} — roll scheduled once the calling turn ends",
|
||||
token, callerTerminal);
|
||||
continuationRunner.accept(() -> runRollover(p, cfg));
|
||||
try {
|
||||
continuationRunner.accept(() -> runRollover(p, cfg));
|
||||
} catch (RuntimeException e) {
|
||||
// continuationRunner can reject the hand-off itself (e.g. a bounded executor's
|
||||
// RejectedExecutionException) before runRollover ever starts, so runRollover's own
|
||||
// finally — the only other place that releases rollingByTerminal — never runs either.
|
||||
// Release the claim here and overwrite the IN_PROGRESS entry with a terminal outcome,
|
||||
// or this lead terminal could never be rolled again and status() would report
|
||||
// IN_PROGRESS forever for a roll that in fact never started.
|
||||
log.warn("lead-rollover: continuationRunner rejected token={} lead={}: {} — the roll "
|
||||
+ "never started; releasing its claim and reporting it as FAILED",
|
||||
token, callerTerminal, e.toString(), e);
|
||||
rollingByTerminal.remove(p.leadTerminal(), token);
|
||||
outcomes.put(token, new RollStatus(RollState.FAILED,
|
||||
"continuationRunner rejected this roll before it ever started: " + e.toString()
|
||||
+ " — the roll never ran; open() a fresh rollover request"));
|
||||
}
|
||||
return RollDecision.approved();
|
||||
}
|
||||
|
||||
|
||||
@@ -1604,4 +1604,42 @@ class LeadRolloverTest {
|
||||
+ "have released the claim held by that roll");
|
||||
assertEquals(LeadRollover.RefusalReason.ROLL_ALREADY_RUNNING, anotherDecision.reason());
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("[SINGLE-FLIGHT 6] a continuationRunner that rejects the hand-off still releases "
|
||||
+ "the claim and leaves a terminal FAILED outcome, not a stuck IN_PROGRESS")
|
||||
void continuationRunnerThatRejectsTheHandOffStillReleasesTheClaim() throws IOException {
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
Path handover = writeHandover("handover contents");
|
||||
AtomicLong clock = new AtomicLong(1_000);
|
||||
AgentControl agents = new AgentControl(herdr);
|
||||
// A continuationRunner that rejects every hand-off, standing in for e.g. a bounded
|
||||
// executor's RejectedExecutionException — the continuation never runs, so runRollover's
|
||||
// own finally never gets a chance to release the claim either.
|
||||
LeadRollover rollover = new LeadRollover(agents, () -> cfg(handover.toString()), _ -> null,
|
||||
fixedClock(clock), () -> { },
|
||||
r -> { throw new RuntimeException("simulated continuationRunner rejection"); });
|
||||
|
||||
LeadRollover.PendingRollover pending = rollover.open(LEAD, "context is full");
|
||||
LeadRollover.RollDecision decision = rollover.confirm(LEAD, pending.token(), true);
|
||||
|
||||
assertTrue(decision.accepted(), "every gate passed before the hand-off itself rejected — "
|
||||
+ "confirm() treats that rejection the same way it treats a throw INSIDE the "
|
||||
+ "continuation: logged and surfaced only through status(), never as a refusal here: "
|
||||
+ decision.reason() + " / " + decision.detail());
|
||||
|
||||
LeadRollover.RollStatus status = rollover.status(pending.token());
|
||||
assertEquals(LeadRollover.RollState.FAILED, status.state(), "a rejected hand-off must leave "
|
||||
+ "a TERMINAL outcome, never a stuck IN_PROGRESS — status() would otherwise have no "
|
||||
+ "way to tell a roll that never started from one still genuinely running: got "
|
||||
+ status.state() + " / " + status.detail());
|
||||
assertNotEquals(LeadRollover.RollState.IN_PROGRESS, status.state());
|
||||
|
||||
LeadRollover.PendingRollover retry = rollover.open(LEAD, "retry after the rejected hand-off");
|
||||
LeadRollover.RollDecision retryDecision = rollover.confirm(LEAD, retry.token(), true);
|
||||
assertTrue(retryDecision.accepted(), "the claim must have been released even though "
|
||||
+ "continuationRunner itself threw before the continuation ever ran — otherwise "
|
||||
+ "this lead terminal could never be rolled again: " + retryDecision.reason() + " / "
|
||||
+ retryDecision.detail());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user