fleetd #513: rework — TIMED_OUT_QUEUED has four routes, not one
CI / shell-tests (pull_request) Successful in 5s
CI / contract (pull_request) Successful in 49s
CI / build (pull_request) Successful in 2m16s

Comment 16984 on the ticket showed my first pass (ed28b51) replaced one
wrong invariant with a narrower one: it described injector.cancel()
cancelling a queued entry as THE mechanism, when three of its four
routes (Injector.java: the send-to-terminal call throwing, the
readiness grace expiring, or the target being torn down) never cancel
anything — cancel() just reports a state a different code path already
set. It also claimed the target never saw a word of the message, which
is not established when the herdr agent.prompt call throws after
already pasting.

Rewrote all four comments (the TIMED_OUT_QUEUED enum constant,
queuedDeliveries, hasQueuedDelivery, hasOrphanedDelegation) plus the
pre-existing inline comment that seeded the original bad wording, to
state only what holds on every route: the message will not arrive
later and is not sitting in a queue. The CANCELLED case is called out
as the only one where the target is known to have seen nothing; the
NOT_DELIVERED case (including the herdr-send-threw route) is flagged
as leaving that open.

Comment-only; no behavior change.
This commit is contained in:
Dai Ha
2026-09-12 16:45:35 +07:00
parent ed28b51f12
commit cb64bc8157
@@ -93,11 +93,17 @@ public final class MessageService {
/** Timed out after the message was delivered — the worker is still working. */ /** Timed out after the message was delivered — the worker is still working. */
TIMED_OUT_WORKING, TIMED_OUT_WORKING,
/** /**
* Timed out before delivery. Despite the name, the message is not left queued: {@link * Timed out with no confirmed delivery. Despite the name, this does not mean the message
* #send} cancels the pending entry ({@link Injector#cancel}) before returning this * is sitting in a queue — on every route it will not arrive later. {@link #send} reaches
* outcome, so it will not arrive later — the target never saw a word of it. The name * this outcome through {@link Injector#cancel}, whose result tells the routes apart:
* records that the message was still queued at the moment the caller gave up, not that it * {@link Injector.Cancellation#CANCELLED} means the message was still queued and this call
* stays queued afterward. * removed it, so the target saw nothing; {@link Injector.Cancellation#NOT_DELIVERED} means
* an earlier attempt already decided the message's fate — the injector's call to the
* target's terminal ({@link dev.ltms.fleet.herdr.AgentControl#send}) threw, or the queue
* was cleared because the target never became ready or was abandoned — and {@code cancel}
* is only reporting that pre-existing state. On the failed-attempt route the terminal may
* already hold a partial paste from before the call threw, so only the {@code CANCELLED}
* case establishes that the target saw nothing.
*/ */
TIMED_OUT_QUEUED, TIMED_OUT_QUEUED,
/** Another send to this session was in flight for the whole window. */ /** Another send to this session was in flight for the whole window. */
@@ -305,14 +311,18 @@ public final class MessageService {
*/ */
private final ConcurrentHashMap<String, Boolean> strandedReplies = new ConcurrentHashMap<>(); private final ConcurrentHashMap<String, Boolean> strandedReplies = new ConcurrentHashMap<>();
/** /**
* Targets whose last send timed out with {@link Outcome#TIMED_OUT_QUEUED} (CB-640) — the * Targets whose last send timed out with {@link Outcome#TIMED_OUT_QUEUED} (CB-640) — {@link
* message never reached the {@link Injector} delivery window before the caller's deadline, so * #send} called {@link Injector#cancel} and got back something other than {@code DELIVERED}.
* {@link #send} cancelled it via {@link Injector#cancel} before it could ever be delivered. The * That covers more than one history: the message may have still been queued and {@code cancel}
* message is gone, not pending: it will not arrive later, and the target never saw it. Set * removed it right there, or an earlier attempt may already have failed (the call to the
* where {@link #send} already computes {@code wasDelivered} for that outcome; no queue is kept * target's terminal threw) or been abandoned (the target never became ready, or was torn
* here, only the fact that a delivery was cancelled. Cleared the same way as * down). What holds on every route: the message will not arrive later, and it is not sitting
* {@link #strandedReplies}: the next accepted delivery for the target ({@link #send} opening a * in any queue. What does NOT hold on every route: that the target saw nothing — a failed
* fresh waiter) or a teardown ({@link #abandon}). * delivery attempt can leave a partial paste behind. Set where {@link #send} already computes
* {@code wasDelivered} for that outcome; no queue is kept here, only the fact that the send
* ended with no confirmed delivery. Cleared the same way as {@link #strandedReplies}: the next
* accepted delivery for the target ({@link #send} opening a fresh waiter) or a teardown
* ({@link #abandon}).
*/ */
private final ConcurrentHashMap<String, Boolean> queuedDeliveries = new ConcurrentHashMap<>(); private final ConcurrentHashMap<String, Boolean> queuedDeliveries = new ConcurrentHashMap<>();
private final AtomicLong ticketSeq = new AtomicLong(); private final AtomicLong ticketSeq = new AtomicLong();
@@ -399,14 +409,17 @@ public final class MessageService {
/** /**
* Read-only delegation fact for fleet views (CB-640): {@code target}'s last send timed out * Read-only delegation fact for fleet views (CB-640): {@code target}'s last send timed out
* before the {@link Injector} ever delivered it — the caller saw * with no confirmed delivery — the caller saw {@link Outcome#TIMED_OUT_QUEUED} (see the
* {@link Outcome#TIMED_OUT_QUEUED} (see the {@code TimeoutException} branch of {@link #send}), * {@code TimeoutException} branch of {@link #send}). Despite the method's name, this is not
* and {@link Injector#cancel} removed the pending entry before it could be delivered. Despite * proof that a message is sitting in a queue: {@link Injector#cancel} reports this outcome
* the method's name, the message is not queued and will not arrive later — the target never * either because the message was still queued and got removed right there, or because an
* saw a word of it; this reports a cancellation, not a pending delivery. Distinct from * earlier attempt already failed (the call to the target's terminal threw) or was abandoned
* {@link Outcome#TIMED_OUT_WORKING}, where delivery already happened and only the reply is * (the target never became ready, or was torn down). Either way the message will not arrive
* outstanding. Cleared the next time this target's delivery is accepted or the target is * later. It does NOT follow that the target saw nothing — on the failed-attempt route the
* abandoned — see {@link #queuedDeliveries}. * terminal may already hold a partial paste. Distinct from {@link Outcome#TIMED_OUT_WORKING},
* where delivery already happened and only the reply is outstanding. Cleared the next time
* this target's delivery is accepted or the target is abandoned — see
* {@link #queuedDeliveries}.
*/ */
public boolean hasQueuedDelivery(String target) { public boolean hasQueuedDelivery(String target) {
return target != null && queuedDeliveries.containsKey(target); return target != null && queuedDeliveries.containsKey(target);
@@ -426,15 +439,15 @@ public final class MessageService {
/** /**
* Read-only delegation fact for fleet views (CB-640): an async ticket is still * Read-only delegation fact for fleet views (CB-640): an async ticket is still
* {@link Phase#PENDING} against {@code target}, yet nothing is actually in flight for it — no * {@link Phase#PENDING} against {@code target}, yet nothing is actually in flight for it — no
* open rendezvous waiter ({@link #hasAcceptedDelivery}) and no record of a cancelled, * open rendezvous waiter ({@link #hasAcceptedDelivery}) and no record of a send that ended with
* undelivered send ({@link #hasQueuedDelivery}). A healthy PENDING ticket can briefly look this * no confirmed delivery ({@link #hasQueuedDelivery}). A healthy PENDING ticket can briefly look
* way while its virtual thread has not yet been scheduled or is blocked on the session lock * this way while its virtual thread has not yet been scheduled or is blocked on the session
* behind another send to the same target, so this is a snapshot fact for the health classifier * lock behind another send to the same target, so this is a snapshot fact for the health
* to weigh across ticks, not proof on its own that the ticket is stuck. It also genuinely * classifier to weigh across ticks, not proof on its own that the ticket is stuck. It also
* persists — not just as a passing race — once an async {@code fleet_ask} lapses unanswered: * genuinely persists — not just as a passing race — once an async {@code fleet_ask} lapses
* {@link #ask} clears the ticket's question and returns it to {@code PENDING}, but {@link #send} * unanswered: {@link #ask} clears the ticket's question and returns it to {@code PENDING}, but
* already closed the forward waiter the instant the question surfaced, so the target has * {@link #send} already closed the forward waiter the instant the question surfaced, so the
* neither an accepted nor a queued delivery left to show for it. * target has neither an accepted nor a queued delivery left to show for it.
* *
* <p><strong>Deliberately still {@code question == null} only (fleetd #275).</strong> This * <p><strong>Deliberately still {@code question == null} only (fleetd #275).</strong> This
* method must not also report a still-{@link Phase#ASKING} task as orphaned: the worker may * method must not also report a still-{@link Phase#ASKING} task as orphaned: the worker may
@@ -962,7 +975,10 @@ public final class MessageService {
log.debug("send to {} timed out (delivered={})", target, wasDelivered); log.debug("send to {} timed out (delivered={})", target, wasDelivered);
if (!wasDelivered) { if (!wasDelivered) {
// CB-640: record that delivery did not happen for fleet health (see // CB-640: record that delivery did not happen for fleet health (see
// queuedDeliveries). The exact Pending was cancelled, so it cannot arrive later. // queuedDeliveries). Whatever injector.cancel() reported above — this call
// removed a still-queued Pending, or an earlier attempt already failed or
// was abandoned — the send ends with no confirmed delivery and will not
// arrive later.
queuedDeliveries.put(target, Boolean.TRUE); queuedDeliveries.put(target, Boolean.TRUE);
} }
return recorded(new Reply( return recorded(new Reply(