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

maxLoad says it is excluded from "every automatic policy" — fixed, the default, ignores it #435

New Issue
Closed
opened 2026-09-10 08:26:46 +02:00 by ltms · 1 comment
ltms commented 2026-09-10 08:26:46 +02:00
Owner
Copy Link
Copy Source

Two javadocs disagree about maxLoad, and the operator-facing one is wrong

FleetConfig.Profile's @param maxLoad (FleetConfig.java:389) tells the operator this:

An explicit 0 (CB-585) means "cap this profile at zero live members": it is excluded from every automatic policy's candidate pool the same way a weight <= 0 profile is (see PlacementPolicyUtil.available(), which already treats "at cap" and "excluded" alike) … a cap is a capacity statement that does not stop being true just because the profile was named.

FixedPlacementPolicy's own javadoc (FixedPlacementPolicy.java:8) says the opposite:

This ignores caps (maxLoad) so that a pre-existing config behaves identically after upgrade — capacity gating for automatic placement is deliberately out of scope for fixed, exactly as it always has been.

fixed is the default. PlacementPolicies.fromName returns it for an absent or blank placement: value (PlacementPolicies.java:16-18), so a fleet with no placement: key runs the one policy that ignores caps.

Measured

The same shared-helper shape as #422 and #429. PlacementPolicyUtil.available() — the helper the maxLoad javadoc points at — has exactly two callers:

$ grep -rn 'available(' fleetd/src/main/java/dev/ltms/fleet/placement/ | grep -v 'static.*available'
RoundRobinPlacementPolicy.java:17:  List<PlacementCandidate> available = PlacementPolicyUtil.available(ctx);
WeightedRoundRobinPolicy.java:21:   List<PlacementCandidate> available = PlacementPolicyUtil.available(ctx);

FixedPlacementPolicy is not one of them, and it never mentions a cap outside that javadoc. PlacementCandidate.excluded() is weight <= 0.0f only — it does not consider maxLoad, so fixed's own weightExcluded walk cannot catch an at-cap profile either.

Behaviour, probed on 7667727 with one dev profile a at maxLoad: 1, liveCount fixed at 1 (exactly at cap), PlacementPolicies.fixed(), and an unqualified spawn:

PROBE_RESULT: SPAWNED on profile=a

So the cap is advisory for an unqualified spawn under the default policy. It is enforced for an explicit fleet_spawn{profile:"a"}, by CompositePeerLauncher.enforceMaxLoad — that half works.

Why it matters

Delegation spawns are unqualified by design: the lead names a role and lets the fleet place it. Those are exactly the spawns that walk past the cap. An operator who sets maxLoad to protect a paid credential gets that protection only for spawns that name the profile by hand, which is the case they were least worried about.

Two ways to resolve it, and the trade

A — make the behaviour match the doc. Have fixed skip an at-cap candidate the way it already skips a quarantined, cooling-off and model-off one (#429 added exactly those three to both of its filter sites, so the seam exists). Cost: a fleet that has been quietly over-subscribing starts refusing spawns. That is a real behaviour change for existing configs, which is the thing fixed exists to avoid.

B — make the doc match the behaviour. Correct the maxLoad javadoc to say the cap is enforced for an explicit spawn and for the weighted/round-robin policies, but not by fixed, and say so in the operator-facing docs too — including Features, whose maxLoad entry should carry this as its gotcha.

I lean to A plus a release note, because "a cap is a capacity statement that does not stop being true just because the profile was named" is the right principle and the doc already commits to it. But B is defensible and cheaper, and either way the two javadocs must stop contradicting each other. Whoever picks this up should decide and say why on the ticket before writing code.

Interaction with #425 — read this first

PR #433 (the #425 rework) resolves an unqualified worktree spawn through real placement and then spawns with that name explicitly, which routes it through enforceMaxLoad. That makes the worktree path enforce the cap while the no-worktree path still ignores it — measured in that PR's own tree, same profile at the same cap:

b066eb1  with worktree    → THREW PlacementException: … at maxLoad: 1 live >= 1 cap
b066eb1  without worktree → SPAWNED on profile=a

I asked for that to be reworked, because #425 must not change cap behaviour as a side effect and must not change it on one path only. If A is chosen here, that inconsistency disappears on its own — so it is worth settling this ticket's direction before the #425 rework lands, and the two should not both try to own the cap.

Out of scope

Whether weight <= 0 and "at cap" should stay merged in PlacementPolicyUtil.available(). They are two different operator statements ("never auto-pick this" vs "this is full right now") and the reason strings differ, but nothing measured here depends on separating them.

## Two javadocs disagree about `maxLoad`, and the operator-facing one is wrong `FleetConfig.Profile`'s `@param maxLoad` (`FleetConfig.java:389`) tells the operator this: > An explicit `0` (CB-585) means "cap this profile at zero live members": it is excluded from **every automatic policy's** candidate pool the same way a `weight <= 0` profile is (see `PlacementPolicyUtil.available()`, which already treats "at cap" and "excluded" alike) … a cap is a capacity statement that does not stop being true just because the profile was named. `FixedPlacementPolicy`'s own javadoc (`FixedPlacementPolicy.java:8`) says the opposite: > This ignores caps (`maxLoad`) so that a pre-existing config behaves identically after upgrade — capacity gating for automatic placement is deliberately out of scope for `fixed`, exactly as it always has been. `fixed` is the default. `PlacementPolicies.fromName` returns it for an absent or blank `placement:` value (`PlacementPolicies.java:16-18`), so a fleet with no `placement:` key runs the one policy that ignores caps. ## Measured The same shared-helper shape as #422 and #429. `PlacementPolicyUtil.available()` — the helper the `maxLoad` javadoc points at — has exactly two callers: ``` $ grep -rn 'available(' fleetd/src/main/java/dev/ltms/fleet/placement/ | grep -v 'static.*available' RoundRobinPlacementPolicy.java:17: List<PlacementCandidate> available = PlacementPolicyUtil.available(ctx); WeightedRoundRobinPolicy.java:21: List<PlacementCandidate> available = PlacementPolicyUtil.available(ctx); ``` `FixedPlacementPolicy` is not one of them, and it never mentions a cap outside that javadoc. `PlacementCandidate.excluded()` is `weight <= 0.0f` only — it does not consider `maxLoad`, so `fixed`'s own `weightExcluded` walk cannot catch an at-cap profile either. Behaviour, probed on `7667727` with one dev profile `a` at `maxLoad: 1`, `liveCount` fixed at 1 (exactly at cap), `PlacementPolicies.fixed()`, and an unqualified spawn: ``` PROBE_RESULT: SPAWNED on profile=a ``` So the cap is advisory for an unqualified spawn under the default policy. It is enforced for an explicit `fleet_spawn{profile:"a"}`, by `CompositePeerLauncher.enforceMaxLoad` — that half works. ## Why it matters Delegation spawns are unqualified by design: the lead names a role and lets the fleet place it. Those are exactly the spawns that walk past the cap. An operator who sets `maxLoad` to protect a paid credential gets that protection only for spawns that name the profile by hand, which is the case they were least worried about. ## Two ways to resolve it, and the trade **A — make the behaviour match the doc.** Have `fixed` skip an at-cap candidate the way it already skips a quarantined, cooling-off and model-off one (#429 added exactly those three to both of its filter sites, so the seam exists). Cost: a fleet that has been quietly over-subscribing starts refusing spawns. That is a real behaviour change for existing configs, which is the thing `fixed` exists to avoid. **B — make the doc match the behaviour.** Correct the `maxLoad` javadoc to say the cap is enforced for an explicit spawn and for the `weighted`/`round-robin` policies, but not by `fixed`, and say so in the operator-facing docs too — including [Features](wiki/11-Features.md), whose `maxLoad` entry should carry this as its gotcha. I lean to **A** plus a release note, because "a cap is a capacity statement that does not stop being true just because the profile was named" is the right principle and the doc already commits to it. But B is defensible and cheaper, and either way the two javadocs must stop contradicting each other. Whoever picks this up should decide and say why on the ticket before writing code. ## Interaction with #425 — read this first PR #433 (the #425 rework) resolves an unqualified worktree spawn through real placement and then spawns with that name **explicitly**, which routes it through `enforceMaxLoad`. That makes the worktree path enforce the cap while the no-worktree path still ignores it — measured in that PR's own tree, same profile at the same cap: ``` b066eb1 with worktree → THREW PlacementException: … at maxLoad: 1 live >= 1 cap b066eb1 without worktree → SPAWNED on profile=a ``` I asked for that to be reworked, because #425 must not change cap behaviour as a side effect and must not change it on one path only. If **A** is chosen here, that inconsistency disappears on its own — so it is worth settling this ticket's direction before the #425 rework lands, and the two should not both try to own the cap. ## Out of scope Whether `weight <= 0` and "at cap" should stay merged in `PlacementPolicyUtil.available()`. They are two different operator statements ("never auto-pick this" vs "this is full right now") and the reason strings differ, but nothing measured here depends on separating them.
ltms commented 2026-09-10 08:44:55 +02:00
Author
Owner
Copy Link
Copy Source

Correction to my own scoping: neither live host is affected. The defect is real; my urgency was not.

I wrote that the cap is advisory "on a default config … which is every delegation spawn", and I implied this bites the running fleets. It does not. Both live daemons set the policy explicitly.

$ grep -n '^placement:' fleetd/fleetd.yaml            # the Mac
210:placement: weighted

$ ssh fleet01 "sed -n '172p' /home/ltms/LTMS/fleetd/fleetd/fleetd.yaml"
placement: weighted

And weighted does honour the cap, via the helper this ticket is about (PlacementPolicyUtil.available()):

Integer cap = c.maxLoad();
if (cap != null) {
    int live = ctx.liveCount().apply(c.profile());
    if (live >= cap) {
        continue;
    }
}

So the at-cap filter works on both hosts today, and my line on PR #433 about sonnet sitting at free: 0, maxLoad: 3, live: 3 was beside the point — that daemon runs weighted, which already routes around it.

How I got it wrong. My first pass grepped ^placement:\s*$ — an anchor that requires the value on the next line. placement: weighted has the value on the same line, so the pattern matched nothing and I read "no policy configured, therefore the default, therefore fixed". A pattern that finds nothing and a config that says nothing look identical. Third time in this session I have been caught by a pattern rather than a fact; the fix is the same each time — run a control. Here the control was grep -cE '^[a-z][A-Za-z0-9_]*:' returning 14, which proves the anchor matches top-level keys and the specific miss was real.

What survives, and why this is still worth fixing

Everything about the defect itself. The probe stands, because it constructed PlacementPolicies.fixed() directly rather than reading a config:

one dev profile at maxLoad: 1, liveCount 1, PlacementPolicies.fixed(), unqualified spawn
-> PROBE_RESULT: SPAWNED on profile=a

fixed ignores the cap, and fixed is what PlacementPolicies.fromName returns for an absent or blank placement: key. So the exposure is:

  • Any fleet that omits placement: — which is every fresh deployment, and what the key's own documentation calls the backward-compatible default.
  • Any fleet that sets placement: fixed deliberately.
  • Not the two hosts we run.

That moves this from "live capacity leak" to "the default policy contradicts the documented contract of maxLoad". Still a defect, still worth the fix, and the fix is now also a consistency argument rather than an urgency one: making fixed skip an at-cap candidate makes all three policies agree with available() and with the maxLoad javadoc. That is a better reason to do it than the one I filed.

For task-17

The technical instructions in your brief are unchanged — both filter sites, at-cap must fall through to the next candidate rather than refuse, one shared predicate, fix the two contradicting javadocs. Only the framing changes.

Two things to carry into your PR body so it does not repeat my error:

  1. Do not claim this fixes a live capacity leak on either host. Both run weighted. Say it aligns fixed with the other two policies and with maxLoad's documented contract.
  2. Your "fall through to the next candidate" requirement is not an invention of mine — it is exactly what available() already does for weighted and round-robin. Point at that as the precedent; it is the strongest justification for the shape.
## Correction to my own scoping: neither live host is affected. The defect is real; my urgency was not. I wrote that the cap is advisory "on a default config … which is every delegation spawn", and I implied this bites the running fleets. It does not. Both live daemons set the policy explicitly. ``` $ grep -n '^placement:' fleetd/fleetd.yaml # the Mac 210:placement: weighted $ ssh fleet01 "sed -n '172p' /home/ltms/LTMS/fleetd/fleetd/fleetd.yaml" placement: weighted ``` And `weighted` does honour the cap, via the helper this ticket is about (`PlacementPolicyUtil.available()`): ```java Integer cap = c.maxLoad(); if (cap != null) { int live = ctx.liveCount().apply(c.profile()); if (live >= cap) { continue; } } ``` So the at-cap filter works on both hosts today, and my line on PR #433 about `sonnet` sitting at `free: 0, maxLoad: 3, live: 3` was beside the point — that daemon runs `weighted`, which already routes around it. **How I got it wrong.** My first pass grepped `^placement:\s*$` — an anchor that requires the value on the *next* line. `placement: weighted` has the value on the same line, so the pattern matched nothing and I read "no policy configured, therefore the default, therefore `fixed`". A pattern that finds nothing and a config that says nothing look identical. Third time in this session I have been caught by a pattern rather than a fact; the fix is the same each time — run a control. Here the control was `grep -cE '^[a-z][A-Za-z0-9_]*:'` returning 14, which proves the anchor matches top-level keys and the specific miss was real. ## What survives, and why this is still worth fixing Everything about the defect itself. The probe stands, because it constructed `PlacementPolicies.fixed()` directly rather than reading a config: ``` one dev profile at maxLoad: 1, liveCount 1, PlacementPolicies.fixed(), unqualified spawn -> PROBE_RESULT: SPAWNED on profile=a ``` `fixed` ignores the cap, and `fixed` is what `PlacementPolicies.fromName` returns for an absent or blank `placement:` key. So the exposure is: - **Any fleet that omits `placement:`** — which is every fresh deployment, and what the key's own documentation calls the backward-compatible default. - **Any fleet that sets `placement: fixed`** deliberately. - Not the two hosts we run. That moves this from "live capacity leak" to "the default policy contradicts the documented contract of `maxLoad`". Still a defect, still worth the fix, and the fix is now also a **consistency** argument rather than an urgency one: making `fixed` skip an at-cap candidate makes all three policies agree with `available()` and with the `maxLoad` javadoc. That is a better reason to do it than the one I filed. ## For task-17 The technical instructions in your brief are unchanged — both filter sites, at-cap must fall through to the next candidate rather than refuse, one shared predicate, fix the two contradicting javadocs. Only the framing changes. Two things to carry into your PR body so it does not repeat my error: 1. Do **not** claim this fixes a live capacity leak on either host. Both run `weighted`. Say it aligns `fixed` with the other two policies and with `maxLoad`'s documented contract. 2. Your "fall through to the next candidate" requirement is not an invention of mine — it is exactly what `available()` already does for `weighted` and `round-robin`. Point at that as the precedent; it is the strongest justification for the shape.
ltms closed this issue 2026-09-10 08:55:03 +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#435
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?