#197: measure the async ticket TTL from completion, not from creation #198

Merged
ltms merged 1 commits from fix/cb-197-ticket-ttl-from-completion into main 2026-08-31 05:10:46 +02:00
Owner

Fixes #197.

The bug

pruneTerminalTickets compared the cutoff against createdNanos:

boolean expired = t.future.isDone() && t.createdNanos < cutoff;

So the window to collect a reply was TTL minus however long the task ran, not TTL:

task runtime time left to collect
1 min 9 min
9 min 1 min
10 min 0 — pruned on the first sweep after it completes
20 min 0 — the reply is discarded the moment it lands

Every delegation that runs longer than the TTL loses its report unconditionally. That is the normal
case here: three workers in one session ran well past ten minutes and two of their complete reports
were destroyed. Only their pushed branches saved the work.

There is no fallback. The reply lives only in Task.future, so pruning the map discards it, and
fleet_poll{target} returns [] rather than holding it.

The fix

Task stamps completedNanos from a whenComplete hook registered in its constructor, and the
prune measures from that. Registering it in the constructor means every completion path stamps
it — a reply, the CB-106 completion fallback, a timeout, a failure, an abandon on teardown — without
each one having to remember to.

The stamp is a boxed Long rather than a long with a sentinel, on purpose: System.nanoTime may
return any value, zero and negatives included, so no numeric sentinel can mean "not stamped yet". A
task whose future is done but whose stamp has not landed is left alone; the next sweep collects it.

createdNanos had no other reader, so it is removed rather than left as a field an inspection would
flag.

What this does not change

The TTL still bounds tasks. An uncollected finished ticket is still evicted once the TTL passes
since it finished, and that is pinned by its own test — measuring from completion would be a
memory leak if a finished ticket were then kept forever.

Verification

Full mvn clean install: Tests run: 1032, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS.

Proved by revert. Restoring the old comparison (a creation stamp taken from the same injected clock,
so the sabotage is faithful rather than mixing a real clock with the test's fake one) fails the new
test:

[ERROR] MessageServiceTest.aTaskRunningLongerThanTheTtlStillKeepsItsReport:1094
a ticket that completed just now must survive the prune, however long its task ran —
the TTL is the window to COLLECT the report, not the budget for producing it
==> expected: not <null>

Restored afterwards and confirmed zero TEMP SABOTAGE markers remain.

Not verified: IDE inspections. fleetd is not currently open in IntelliJ on this machine, so
ide_diagnostics could not run. The Maven build covers compile errors but not inspections.

Left for a follow-up, deliberately not in this PR

Two further improvements from the issue, neither needed to stop the data loss:

  • On prune, push the reply into the target's reply inbox instead of discarding it, so
    fleet_poll{target} becomes the honest fallback the error message already implies.
  • Make poll distinguish "never issued" from "expired". Today both return the same string, and they
    tell an operator completely different things.
Fixes #197. ## The bug `pruneTerminalTickets` compared the cutoff against `createdNanos`: ```java boolean expired = t.future.isDone() && t.createdNanos < cutoff; ``` So the window to collect a reply was **`TTL` minus however long the task ran**, not `TTL`: | task runtime | time left to collect | |---|---| | 1 min | 9 min | | 9 min | 1 min | | 10 min | 0 — pruned on the first sweep after it completes | | 20 min | 0 — the reply is discarded the moment it lands | Every delegation that runs longer than the TTL loses its report unconditionally. That is the normal case here: three workers in one session ran well past ten minutes and two of their complete reports were destroyed. Only their pushed branches saved the work. There is no fallback. The reply lives only in `Task.future`, so pruning the map discards it, and `fleet_poll{target}` returns `[]` rather than holding it. ## The fix `Task` stamps `completedNanos` from a `whenComplete` hook registered in its constructor, and the prune measures from that. Registering it in the constructor means **every** completion path stamps it — a reply, the CB-106 completion fallback, a timeout, a failure, an abandon on teardown — without each one having to remember to. The stamp is a boxed `Long` rather than a `long` with a sentinel, on purpose: `System.nanoTime` may return any value, zero and negatives included, so no numeric sentinel can mean "not stamped yet". A task whose future is done but whose stamp has not landed is left alone; the next sweep collects it. `createdNanos` had no other reader, so it is removed rather than left as a field an inspection would flag. ## What this does not change The TTL still bounds `tasks`. An uncollected finished ticket is still evicted once the TTL passes **since it finished**, and that is pinned by its own test — measuring from completion would be a memory leak if a finished ticket were then kept forever. ## Verification Full `mvn clean install`: **Tests run: 1032, Failures: 0, Errors: 0, Skipped: 0** — BUILD SUCCESS. Proved by revert. Restoring the old comparison (a creation stamp taken from the same injected clock, so the sabotage is faithful rather than mixing a real clock with the test's fake one) fails the new test: ``` [ERROR] MessageServiceTest.aTaskRunningLongerThanTheTtlStillKeepsItsReport:1094 a ticket that completed just now must survive the prune, however long its task ran — the TTL is the window to COLLECT the report, not the budget for producing it ==> expected: not <null> ``` Restored afterwards and confirmed zero `TEMP SABOTAGE` markers remain. **Not verified:** IDE inspections. `fleetd` is not currently open in IntelliJ on this machine, so `ide_diagnostics` could not run. The Maven build covers compile errors but not inspections. ## Left for a follow-up, deliberately not in this PR Two further improvements from the issue, neither needed to stop the data loss: - On prune, push the reply into the target's reply inbox instead of discarding it, so `fleet_poll{target}` becomes the honest fallback the error message already implies. - Make `poll` distinguish "never issued" from "expired". Today both return the same string, and they tell an operator completely different things.
ltms added 1 commit 2026-08-31 05:10:40 +02:00
#197: measure the async ticket TTL from completion, not from creation
CI / contract (pull_request) Successful in 54s
CI / build (pull_request) Successful in 1m32s
ea98856130
pruneTerminalTickets compared the cutoff against createdNanos, so the real
window to collect a reply was "TTL minus however long the task ran". A
delegation that ran longer than the 10-minute TTL was already past the cutoff
the moment it finished, so the next prune destroyed its reply.

That is the normal case here, not an edge case. Real delegated work runs well
past ten minutes. Three workers in one session did, and two of their complete
reports were lost. The reply lives only in Task.future, so pruning it discards
the worker's whole report, and fleet_poll{target} returns [] rather than
holding it — there is no fallback.

Task now stamps completedNanos from a whenComplete hook registered in its
constructor, so every completion path stamps it (a reply, the completion
fallback, a timeout, a failure, an abandon on teardown) without each one having
to remember to. The stamp is a boxed Long, not a long with a sentinel:
System.nanoTime may return any value, so no number can mean "not stamped yet".
A task that is done but not yet stamped is left for the next sweep.

createdNanos had no other reader and is removed.

The TTL still bounds tasks — an uncollected finished ticket is still evicted
once the TTL passes since it finished. Both halves are pinned by a test, and
the first one was proved by restoring the old comparison and watching it fail.

Tests run: 1032, Failures: 0, Errors: 0, Skipped: 0
ltms merged commit a814d1ef00 into main 2026-08-31 05:10:46 +02:00
Sign in to join this conversation.