fleetd #651: raise turnSettleSeconds default from 20 to 300
A lead that runs the documented handover procedure writes its goodbye message in the same turn as fleet_handover confirm. The deferred roll then waits turnSettleSeconds for that same pane to reach IDLE or DONE. An ordinary goodbye turn took 20394ms, so a 20s budget was too small and the roll refused itself with TURN_NEVER_SETTLED. The refusal branch is correct; only the budget was wrong. 300 is not derived from that single 20394ms observation. It matches leadHeartbeat.idleAfterSeconds (also default 300), the only other constant in this codebase answering "how long may a lead legitimately be mid-turn", whose own javadoc reasons that 5 minutes absorbs normal pauses without stalling. The two constants answer the same question and should not disagree by a factor of fifteen. The wait stays bounded on purpose: an unbounded wait would let a lead whose turn never ends park a continuation and hold a pending token forever, which is harder to notice than a logged refusal. Also corrects FleetConfig's javadoc for turnSettleSeconds and clearSettleSeconds, which described both waits as waiting for the pane to report an "injectable" state again. The code requires IDLE or DONE specifically and excludes BLOCKED (a live turn merely paused), which is the exact state where sending /clear would destroy context. LeadRollover.java's javadoc already states this correctly; only FleetConfig.java's was wrong. Adds FleetConfigTest coverage for the default's resolution: unset, positive, and <= 0 all resolve as expected. Coverage for the two turn-boundary properties (settles-in-time vs. never-settles, and BLOCKED is not treated as settled) already existed in LeadRolloverTest and needed no change — confirmed by mutating the default back to 20 and the IDLE||DONE check to also accept BLOCKED; both mutations were caught by existing or new tests, then reverted.
This commit is contained in:
@@ -1399,11 +1399,13 @@ public record FleetConfig(
|
||||
* called FROM the calling lead's own turn, so its pane is still {@code WORKING} the instant
|
||||
* {@code confirm()} validates every gate and schedules the roll. {@code
|
||||
* dev.ltms.fleet.lead.LeadRollover}'s deferred continuation waits up to this many seconds for
|
||||
* that SAME pane to report an injectable state again — i.e. for the calling turn to actually
|
||||
* end — before it sends {@code /clear} at all. If that wait times out, no {@code /clear} is
|
||||
* that SAME pane to report {@code IDLE} or {@code DONE} — i.e. for the calling turn to actually
|
||||
* end — before it sends {@code /clear} at all. {@code BLOCKED} does not count: that is a live
|
||||
* turn merely paused, not one that has finished. If that wait times out, no {@code /clear} is
|
||||
* ever sent: a lead that never goes idle is still doing real work, and clearing it would
|
||||
* destroy live context. This is a separate wait from {@code clearSettleSeconds} below, which
|
||||
* bounds the SECOND wait, for the pane to re-settle AFTER {@code /clear} has already gone out.
|
||||
* bounds the SECOND wait, for the pane to reach {@code IDLE} or {@code DONE} again AFTER
|
||||
* {@code /clear} has already gone out.
|
||||
*
|
||||
* @param handoverPath required when this block is present — where the handover file a fresh
|
||||
* lead session reads must live. There is no sane non-null default for an
|
||||
@@ -1422,12 +1424,13 @@ public record FleetConfig(
|
||||
* @param maxDocAgeSeconds default 3600 — refuse a handover file whose modified time is older
|
||||
* than this many seconds, so a stale leftover from an earlier rollover
|
||||
* attempt can never be mistaken for a fresh one.
|
||||
* @param turnSettleSeconds default 20 — bound on how long the deferred roll waits for the
|
||||
* CALLING lead's own turn to end (its pane to report injectable again)
|
||||
* before sending {@code /clear} at all. See the paragraph above.
|
||||
* @param turnSettleSeconds default 300 — bound on how long the deferred roll waits for the
|
||||
* CALLING lead's own turn to end (its pane to report {@code IDLE} or
|
||||
* {@code DONE}) before sending {@code /clear} at all. See the paragraph
|
||||
* above.
|
||||
* @param clearSettleSeconds default 20 — bound on how long to wait for the lead's pane to
|
||||
* report an injectable state again after {@code /clear} before giving up. A
|
||||
* roll that times out here never sends {@code bootstrapText}.
|
||||
* report {@code IDLE} or {@code DONE} again after {@code /clear} before
|
||||
* giving up. A roll that times out here never sends {@code bootstrapText}.
|
||||
* @param bootstrapText default a sentence naming the RESOLVED handover path — sent to the
|
||||
* lead's pane once it settles after {@code /clear}, telling the fresh
|
||||
* session where to read the handover and carry on. Left {@code null} here
|
||||
@@ -1444,7 +1447,7 @@ public record FleetConfig(
|
||||
public LeadRollover {
|
||||
requireOperatorConfirm = requireOperatorConfirm == null || requireOperatorConfirm;
|
||||
maxDocAgeSeconds = (maxDocAgeSeconds == null || maxDocAgeSeconds <= 0) ? 3600 : maxDocAgeSeconds;
|
||||
turnSettleSeconds = (turnSettleSeconds == null || turnSettleSeconds <= 0) ? 20 : turnSettleSeconds;
|
||||
turnSettleSeconds = (turnSettleSeconds == null || turnSettleSeconds <= 0) ? 300 : turnSettleSeconds;
|
||||
clearSettleSeconds = (clearSettleSeconds == null || clearSettleSeconds <= 0) ? 20 : clearSettleSeconds;
|
||||
bootstrapText = (bootstrapText == null || bootstrapText.isBlank()) ? null : bootstrapText;
|
||||
}
|
||||
|
||||
@@ -3175,4 +3175,41 @@ class FleetConfigTest {
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
assertTrue(cfg.models().offIds().isEmpty());
|
||||
}
|
||||
|
||||
// ── fleetd #651: leadRollover.turnSettleSeconds default resolution ─────────────────────────
|
||||
|
||||
@Test
|
||||
void turnSettleSecondsDefaultsTo300WhenUnset(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("bare-rollover.yaml");
|
||||
Files.writeString(f, "bind:\n port: 8080\nleadRollover: {}\n");
|
||||
|
||||
FleetConfig.LeadRollover rollover = FleetConfig.load(f).leadRollover();
|
||||
assertNotNull(rollover);
|
||||
assertEquals(300, rollover.turnSettleSeconds());
|
||||
}
|
||||
|
||||
@Test
|
||||
void turnSettleSecondsUsesAnExplicitPositiveValue(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("rollover.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
leadRollover:
|
||||
turnSettleSeconds: 45
|
||||
""");
|
||||
|
||||
FleetConfig.LeadRollover rollover = FleetConfig.load(f).leadRollover();
|
||||
assertEquals(45, rollover.turnSettleSeconds());
|
||||
}
|
||||
|
||||
@Test
|
||||
void turnSettleSecondsFallsBackTo300WhenZeroOrNegative(@TempDir Path dir) throws Exception {
|
||||
Path zero = dir.resolve("zero.yaml");
|
||||
Files.writeString(zero, "bind:\n port: 8080\nleadRollover:\n turnSettleSeconds: 0\n");
|
||||
assertEquals(300, FleetConfig.load(zero).leadRollover().turnSettleSeconds());
|
||||
|
||||
Path negative = dir.resolve("negative.yaml");
|
||||
Files.writeString(negative, "bind:\n port: 8080\nleadRollover:\n turnSettleSeconds: -5\n");
|
||||
assertEquals(300, FleetConfig.load(negative).leadRollover().turnSettleSeconds());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user