fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

A BACKEND_ERROR member holds its seat forever: counted as live, never reaped, never reclaimable #284

New Issue
Closed
opened 2026-09-04 05:39:29 +02:00 by ltms · 1 comment
ltms commented 2026-09-04 05:39:29 +02:00
Owner
Copy Link
Copy Source

Found by an audit of the health and placement state machines. I traced every link myself; line numbers are from main at 66e5247.

The defect

When a member's turn is classified as a backend error, SessionManager.onBackendError moves it to BACKEND_ERROR (SessionManager.java:744). From that moment the session is dead — it cannot receive deliveries and it will never work again. But nothing ever removes it, and it still occupies a seat.

Three facts, each read directly:

  1. It counts as live. Fleetd.java:253-255 sets the live counter with no state filter at all:

    liveCountRef.set(profileName -> (int) sessions.roster().stream()
        .filter(s -> profileName.equals(s.profile()))
        .count());

    That same counter is what the real spawn gate reads: CompositePeerLauncher.enforceMaxLoad (:475-491) does int live = liveCount.apply(profile); if (live >= cap) throw new PlacementException(...). It is also what free is computed from in FleetMcp.capacityView (:1208).

  2. It is never reaped. SessionManager.reapIdle (:812-836) skips anything that is not READY or DONE:

    if (s.state() != MemberSession.State.READY && s.state() != MemberSession.State.DONE) {
    continue;
}

  3. It is not reclaimable. FleetMcp.capacityView (:1197-1200) counts a seat as reclaimable only for READY or DONE, so fleet_list will not even hint that the seat could be recovered.

What the operator sees

A backend error permanently consumes one seat on that profile. Nothing frees it but an explicit fleet_stop on that exact pane, or a daemon restart.

The harm runs in the direction that hurts: the fleet refuses healthy spawns. On a maxLoad: 1 profile — opus and sol on this host today — a single backend error takes the profile out of service for good, and fleet_spawn answers "at maxLoad: 1 live >= 1 cap; refusing spawn — no fallback to another profile". The lead sees a capacity refusal and has no reason to suspect a dead seat, because reclaimable: 0 says there is nothing to reclaim.

This also outlives the cooldown that was supposed to be the whole remedy. The quarantine and the cooling-off both expire on their own; the dead seat does not. So after the credential recovers, the profile is still refusing.

The fix — decide which, and say why

Two defensible options. Pick one, and put the reasoning in your report:

  1. Release it. Treat BACKEND_ERROR like any other definite teardown: release the session and abandon its outstanding work. Cleanest, and it reuses the path #275 just fixed — sessions.onRelease now sweeps an ASKING ticket on a definite teardown, which is exactly what a dead backend is.
  2. Leave it visible but stop it counting. Keep the roster entry so a lead can still see what happened, but exclude BACKEND_ERROR from liveCount, and report it as reclaimable so fleet_list tells the truth.

Option 1 is simpler and I lean toward it. Option 2 keeps diagnostic value the operator may want. If you pick 2, the change must land in Fleetd.java:253 — the counter the real gate reads — not only in capacityView. Fixing the display and leaving the gate alone would make fleet_list say a seat is free while fleet_spawn still refuses it, which is worse than today.

Rules

  • Whichever you pick, prove it with a test that drives onBackendError and then asserts a fresh spawn on that profile is granted. A test that only asserts the state transition proves nothing about the gate — that mistake is why #272 shipped.
  • Do not widen the reaper's state filter as a shortcut. reapIdle skipping non-READY/DONE states is deliberate; a BUSY session must never be reaped.
  • If you find that something already releases these sessions and the seat does come back, say so and close this. "Not reachable, here is why" is a good result — but name the code that does it, because I looked and did not find it.
Found by an audit of the health and placement state machines. **I traced every link myself**; line numbers are from `main` at `66e5247`. ## The defect When a member's turn is classified as a backend error, `SessionManager.onBackendError` moves it to `BACKEND_ERROR` (`SessionManager.java:744`). From that moment the session is dead — it cannot receive deliveries and it will never work again. But nothing ever removes it, and it still occupies a seat. Three facts, each read directly: 1. **It counts as live.** `Fleetd.java:253-255` sets the live counter with no state filter at all: ```java liveCountRef.set(profileName -> (int) sessions.roster().stream() .filter(s -> profileName.equals(s.profile())) .count()); ``` That same counter is what the real spawn gate reads: `CompositePeerLauncher.enforceMaxLoad` (`:475-491`) does `int live = liveCount.apply(profile); if (live >= cap) throw new PlacementException(...)`. It is also what `free` is computed from in `FleetMcp.capacityView` (`:1208`). 2. **It is never reaped.** `SessionManager.reapIdle` (`:812-836`) skips anything that is not `READY` or `DONE`: ```java if (s.state() != MemberSession.State.READY && s.state() != MemberSession.State.DONE) { continue; } ``` 3. **It is not reclaimable.** `FleetMcp.capacityView` (`:1197-1200`) counts a seat as reclaimable only for `READY` or `DONE`, so `fleet_list` will not even hint that the seat could be recovered. ## What the operator sees A backend error permanently consumes one seat on that profile. Nothing frees it but an explicit `fleet_stop` on that exact pane, or a daemon restart. The harm runs in the direction that hurts: the fleet **refuses healthy spawns**. On a `maxLoad: 1` profile — `opus` and `sol` on this host today — a single backend error takes the profile out of service for good, and `fleet_spawn` answers "at maxLoad: 1 live >= 1 cap; refusing spawn — no fallback to another profile". The lead sees a capacity refusal and has no reason to suspect a dead seat, because `reclaimable: 0` says there is nothing to reclaim. This also outlives the cooldown that was supposed to be the whole remedy. The quarantine and the cooling-off both expire on their own; the dead seat does not. So after the credential recovers, the profile is still refusing. ## The fix — decide which, and say why Two defensible options. Pick one, and put the reasoning in your report: 1. **Release it.** Treat `BACKEND_ERROR` like any other definite teardown: release the session and abandon its outstanding work. Cleanest, and it reuses the path #275 just fixed — `sessions.onRelease` now sweeps an ASKING ticket on a definite teardown, which is exactly what a dead backend is. 2. **Leave it visible but stop it counting.** Keep the roster entry so a lead can still see what happened, but exclude `BACKEND_ERROR` from `liveCount`, and report it as reclaimable so `fleet_list` tells the truth. Option 1 is simpler and I lean toward it. Option 2 keeps diagnostic value the operator may want. **If you pick 2, the change must land in `Fleetd.java:253` — the counter the real gate reads — not only in `capacityView`.** Fixing the display and leaving the gate alone would make `fleet_list` say a seat is free while `fleet_spawn` still refuses it, which is worse than today. ## Rules - Whichever you pick, prove it with a test that drives `onBackendError` and then asserts a fresh spawn on that profile is granted. A test that only asserts the state transition proves nothing about the gate — that mistake is why #272 shipped. - **Do not widen the reaper's state filter as a shortcut.** `reapIdle` skipping non-`READY`/`DONE` states is deliberate; a `BUSY` session must never be reaped. - If you find that something already releases these sessions and the seat does come back, say so and close this. "Not reachable, here is why" is a good result — but name the code that does it, because I looked and did not find it.
ltms commented 2026-09-04 06:13:34 +02:00
Author
Owner
Copy Link
Copy Source

Merged as dab9645, corrected in 94f50e5. Real merge built green at 1296 tests.

Option 2 was chosen (keep the roster entry, stop it counting), and the worker extended it from BACKEND_ERROR to FAILED after I sent round one back — onFailed sets FAILED, reapIdle skips it for the same reason, and it was equally uncounted. The first fix was itself the shape this ticket is about: a guard put on one branch and not its sibling.

My mutation

Removed the two state filters from Fleetd.liveSessionCount:

FleetdBackendErrorSinkTest.backendErrorSessionDoesNotBlockFreshSpawnAtMaxLoad:238
  » Placement worker profile 'terra' is at maxLoad: 1 live >= 1 cap; refusing spawn — no fallback to another profile
FleetdBackendErrorSinkTest.failedSessionDoesNotBlockFreshSpawnAtMaxLoad:250
  » Placement worker profile 'terra' is at maxLoad: 1 live >= 1 cap; refusing spawn — no fallback to another profile

That is the exact operator message from the ticket, and it comes out of the real gate — the test wires CompositePeerLauncher to Fleetd.liveSessionCount and calls acquire, so it fails at the place that refuses a spawn, not at a state assertion. This ticket asked for that specifically, because a test that only checks the transition is what let #272 ship.

FleetConfig.java was the extra file the worker touched: a javadoc-only change to maxLoad, correcting "any session the registry still owns, in any state" to the new meaning. Correct, and it needed doing.

My correction — half of this ticket was wrong

I wrote: "exclude BACKEND_ERROR from liveCount, and report it as reclaimable so fleet_list tells the truth." The second half does the opposite.

reclaimable means "this member holds a seat and has no open bridge work — stop it and you get the seat back". Once liveSessionCount stops counting a terminal session, its seat is already in free. Counting it in reclaimable as well reports the same seat twice, and free + reclaimable then reads as more capacity than maxLoad allows.

The worker also widened only the profile-level count in capacityView, not the per-member flag in memberCapacityView. Both travel in the same fleet_list response, so the response contradicted itself. My first attempt at a test caught it:

{"members":[{"state":"backend_error","reclaimable":false},{"state":"failed","reclaimable":false},...],
 "capacity":[{"profile":"ltms-local","maxLoad":3,"live":1,"free":2,"reclaimable":0}]}

I did not fix that by widening the second copy. Two copies of one rule in one response is how they came to disagree, so there is now one: FleetMcp.reclaimable(session, messages), called by both views. A test runs it over every MemberSession.State value, so a state added later cannot slip through unconsidered. My mutation putting BACKEND_ERROR back into it fails both:

FleetMcpTest.onlyReadyAndDoneSessionsAreReclaimable:625
  state BACKEND_ERROR must not count as reclaimable ==> expected: <false> but was: <true>
FleetMcpTest.terminalFailureSessionsFreeTheirSeatWithoutBeingCountedReclaimable:654
  the freed seats must not be counted a second time as reclaimable

Nothing is lost by not flagging them: the roster row still carries state: "backend_error" or "failed", which is what tells the lead to stop the pane.

Still true after this

Terminal sessions are never reaped — the ticket forbade widening reapIdle's filter, and that stands. They accumulate in the roster until stopped or until the daemon restarts. That is now cosmetic rather than a capacity loss, and it is the diagnostic value option 2 was chosen for.

Wiki entry added: A dead member's seat comes back.

Merged as `dab9645`, corrected in `94f50e5`. Real merge built green at **1296 tests**. Option 2 was chosen (keep the roster entry, stop it counting), and the worker extended it from `BACKEND_ERROR` to `FAILED` after I sent round one back — `onFailed` sets `FAILED`, `reapIdle` skips it for the same reason, and it was equally uncounted. The first fix was itself the shape this ticket is about: a guard put on one branch and not its sibling. ## My mutation Removed the two state filters from `Fleetd.liveSessionCount`: ``` FleetdBackendErrorSinkTest.backendErrorSessionDoesNotBlockFreshSpawnAtMaxLoad:238 » Placement worker profile 'terra' is at maxLoad: 1 live >= 1 cap; refusing spawn — no fallback to another profile FleetdBackendErrorSinkTest.failedSessionDoesNotBlockFreshSpawnAtMaxLoad:250 » Placement worker profile 'terra' is at maxLoad: 1 live >= 1 cap; refusing spawn — no fallback to another profile ``` That is the exact operator message from the ticket, and it comes out of the **real gate** — the test wires `CompositePeerLauncher` to `Fleetd.liveSessionCount` and calls `acquire`, so it fails at the place that refuses a spawn, not at a state assertion. This ticket asked for that specifically, because a test that only checks the transition is what let #272 ship. `FleetConfig.java` was the extra file the worker touched: a javadoc-only change to `maxLoad`, correcting "any session the registry still owns, in any state" to the new meaning. Correct, and it needed doing. ## My correction — half of this ticket was wrong I wrote: *"exclude `BACKEND_ERROR` from `liveCount`, and report it as reclaimable so `fleet_list` tells the truth."* The second half does the opposite. `reclaimable` means "this member holds a seat and has no open bridge work — stop it and you get the seat back". Once `liveSessionCount` stops counting a terminal session, **its seat is already in `free`**. Counting it in `reclaimable` as well reports the same seat twice, and `free + reclaimable` then reads as more capacity than `maxLoad` allows. The worker also widened only the profile-level count in `capacityView`, not the per-member flag in `memberCapacityView`. Both travel in the same `fleet_list` response, so the response contradicted itself. My first attempt at a test caught it: ``` {"members":[{"state":"backend_error","reclaimable":false},{"state":"failed","reclaimable":false},...], "capacity":[{"profile":"ltms-local","maxLoad":3,"live":1,"free":2,"reclaimable":0}]} ``` I did not fix that by widening the second copy. Two copies of one rule in one response is *how* they came to disagree, so there is now one: `FleetMcp.reclaimable(session, messages)`, called by both views. A test runs it over every `MemberSession.State` value, so a state added later cannot slip through unconsidered. My mutation putting `BACKEND_ERROR` back into it fails both: ``` FleetMcpTest.onlyReadyAndDoneSessionsAreReclaimable:625 state BACKEND_ERROR must not count as reclaimable ==> expected: <false> but was: <true> FleetMcpTest.terminalFailureSessionsFreeTheirSeatWithoutBeingCountedReclaimable:654 the freed seats must not be counted a second time as reclaimable ``` Nothing is lost by not flagging them: the roster row still carries `state: "backend_error"` or `"failed"`, which is what tells the lead to stop the pane. ## Still true after this Terminal sessions are **never reaped** — the ticket forbade widening `reapIdle`'s filter, and that stands. They accumulate in the roster until stopped or until the daemon restarts. That is now cosmetic rather than a capacity loss, and it is the diagnostic value option 2 was chosen for. Wiki entry added: *A dead member's seat comes back*.
ltms closed this issue 2026-09-04 06:14:05 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#284
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?