fleetd #435: FixedPlacementPolicy now honors maxLoad #436

Merged
ltms merged 1 commits from worker/435-fixed-policy-cap-fe11de-12 into main 2026-09-10 08:54:19 +02:00
Member

fleetd #435: FixedPlacementPolicy ignored maxLoad on the default automatic-placement path.

The defect

FleetConfig.Profile's maxLoad javadoc says an at-cap profile "is excluded from every
automatic policy's candidate pool the same way a weight <= 0 profile is", citing
PlacementPolicyUtil.available(). That helper has exactly two callers -- weighted and
round-robin -- and FixedPlacementPolicy (the default policy for an absent/blank
placement: key) is not one of them. Measured on 7667727: a single dev profile at
maxLoad: 1 with 1 live, under fixed(), unqualified spawn -- "SPAWNED on profile=a".

The fix (direction A, as specified in the ticket)

  • Extracted the "at cap" predicate into PlacementPolicyUtil.atCap(ctx, candidate) so all
    three policies share one definition, instead of available()/emptyException() each
    computing it inline while fixed computed nothing at all.
  • FixedPlacementPolicy.select now consults it at both filter sites: the default fast path
    and the candidate walk (mirroring the existing weightExcluded lookup-by-name pattern via a
    new small candidateFor/capExcluded helper pair).
  • An at-cap default now falls through to the next candidate instead of refusing the spawn --
    exactly the guarantee #429 established for model-off. Only when every candidate is unusable
    does the policy still throw, and the message now names the cap
    (is at maxLoad (N live >= M cap)).
  • The reason-priority ordering (dAtCap) sits between dCoolingOff and dModelOff, and
    dModelOff's own guard now excludes dAtCap, matching CompositePeerLauncher's
    explicit-spawn check order (quarantine, cooling off, max load, model-off).
  • Updated FixedPlacementPolicy's class javadoc: the old "this ignores caps... deliberately
    out of scope" sentence is gone (it is no longer true), and the "Five exceptions" list is now
    six. FleetConfig.Profile#maxLoad's javadoc needed no change -- it is now true.

Tests

  • PlacementPolicyTest: capped default falls through to a free second candidate (asserts the
    profile); every candidate capped throws naming the cap; an uncapped default is still chosen
    (mirror -- the new term cannot exclude everything); maxLoad: 0 on the default; quarantine
    still wins over at-cap when both apply on the default.
  • CompositePeerLauncherTest: one test through CompositePeerLauncher.spawn with a blank
    profile (fixedPolicyGatesDefaultProfileAtMaxLoadOnUnqualifiedSpawn) -- proves the caller
    actually reaches the fix, not only the policy in isolation.

Build

Full mvn clean install in the worktree: Tests run: 1555, Failures: 0, Errors: 0, Skipped: 0,
BUILD SUCCESS.

Mutation testing (both filter sites, separately, with an unmutated control)

Removed && !capExcluded(ctx, d) from the default fast path (line 63) -> 4 tests fail
(fixedSkipsCappedDefault, fixedThrowsWhenDefaultAndEveryCandidateAtCap,
fixedSkipsMaxLoadZeroDefaultEvenWithZeroLiveWorkers,
fixedPolicyGatesDefaultProfileAtMaxLoadOnUnqualifiedSpawn). Restored, verified byte-identical
to the fixed source via diff.

Removed && !PlacementPolicyUtil.atCap(ctx, c) from the candidate walk (line 69) -> 3 tests
fail (fixedFallbackWalkSkipsCappedCandidate, fixedThrowsWhenDefaultAndEveryCandidateAtCap,
fixedPolicyGatesDefaultProfileAtMaxLoadOnUnqualifiedSpawn). Restored, verified
byte-identical to the fixed source via diff.

Control (unmutated, same two test classes): 113 tests, 0 failures -- confirms the mutations
above were real changes to the running code, not no-ops.

For the wiki (not committed here -- wiki/ is a submodule)

Worth a line in the placement-policy section once someone updates the wiki: fixed now
gates on maxLoad at both filter sites, matching weighted/round-robin; an at-cap default
falls through rather than refusing the spawn.

Note (scope: observed, not fixed)

Did not investigate other places where PlacementPolicyUtil or a similar shared helper might
be bypassed by one implementation of an interface -- out of this ticket's scope.

fleetd #435: FixedPlacementPolicy ignored maxLoad on the default automatic-placement path. ## The defect `FleetConfig.Profile`'s `maxLoad` javadoc says an at-cap profile "is excluded from every automatic policy's candidate pool the same way a `weight <= 0` profile is", citing `PlacementPolicyUtil.available()`. That helper has exactly two callers -- `weighted` and `round-robin` -- and `FixedPlacementPolicy` (the default policy for an absent/blank `placement:` key) is not one of them. Measured on 7667727: a single dev profile at `maxLoad: 1` with 1 live, under `fixed()`, unqualified spawn -- "SPAWNED on profile=a". ## The fix (direction A, as specified in the ticket) - Extracted the "at cap" predicate into `PlacementPolicyUtil.atCap(ctx, candidate)` so all three policies share one definition, instead of `available()`/`emptyException()` each computing it inline while `fixed` computed nothing at all. - `FixedPlacementPolicy.select` now consults it at both filter sites: the default fast path and the candidate walk (mirroring the existing `weightExcluded` lookup-by-name pattern via a new small `candidateFor`/`capExcluded` helper pair). - An at-cap default now falls through to the next candidate instead of refusing the spawn -- exactly the guarantee #429 established for model-off. Only when every candidate is unusable does the policy still throw, and the message now names the cap (`is at maxLoad (N live >= M cap)`). - The reason-priority ordering (`dAtCap`) sits between `dCoolingOff` and `dModelOff`, and `dModelOff`'s own guard now excludes `dAtCap`, matching `CompositePeerLauncher`'s explicit-spawn check order (quarantine, cooling off, max load, model-off). - Updated `FixedPlacementPolicy`'s class javadoc: the old "this ignores caps... deliberately out of scope" sentence is gone (it is no longer true), and the "Five exceptions" list is now six. `FleetConfig.Profile#maxLoad`'s javadoc needed no change -- it is now true. ## Tests - `PlacementPolicyTest`: capped default falls through to a free second candidate (asserts the profile); every candidate capped throws naming the cap; an uncapped default is still chosen (mirror -- the new term cannot exclude everything); `maxLoad: 0` on the default; quarantine still wins over at-cap when both apply on the default. - `CompositePeerLauncherTest`: one test through `CompositePeerLauncher.spawn` with a blank profile (`fixedPolicyGatesDefaultProfileAtMaxLoadOnUnqualifiedSpawn`) -- proves the caller actually reaches the fix, not only the policy in isolation. ## Build Full `mvn clean install` in the worktree: `Tests run: 1555, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`. ## Mutation testing (both filter sites, separately, with an unmutated control) Removed `&& !capExcluded(ctx, d)` from the default fast path (line 63) -> 4 tests fail (`fixedSkipsCappedDefault`, `fixedThrowsWhenDefaultAndEveryCandidateAtCap`, `fixedSkipsMaxLoadZeroDefaultEvenWithZeroLiveWorkers`, `fixedPolicyGatesDefaultProfileAtMaxLoadOnUnqualifiedSpawn`). Restored, verified byte-identical to the fixed source via `diff`. Removed `&& !PlacementPolicyUtil.atCap(ctx, c)` from the candidate walk (line 69) -> 3 tests fail (`fixedFallbackWalkSkipsCappedCandidate`, `fixedThrowsWhenDefaultAndEveryCandidateAtCap`, `fixedPolicyGatesDefaultProfileAtMaxLoadOnUnqualifiedSpawn`). Restored, verified byte-identical to the fixed source via `diff`. Control (unmutated, same two test classes): 113 tests, 0 failures -- confirms the mutations above were real changes to the running code, not no-ops. ## For the wiki (not committed here -- wiki/ is a submodule) Worth a line in the placement-policy section once someone updates the wiki: `fixed` now gates on `maxLoad` at both filter sites, matching `weighted`/`round-robin`; an at-cap default falls through rather than refusing the spawn. ## Note (scope: observed, not fixed) Did not investigate other places where `PlacementPolicyUtil` or a similar shared helper might be bypassed by one implementation of an interface -- out of this ticket's scope.
agent added 1 commit 2026-09-10 08:46:41 +02:00
fleetd #435: make FixedPlacementPolicy honor maxLoad
CI / contract (pull_request) Successful in 45s
CI / build (pull_request) Successful in 1m54s
ed2027b202
FixedPlacementPolicy (the default placement policy) never consulted
maxLoad, so an at-cap default was chosen anyway on every unqualified
spawn -- the cap was advisory, not enforced, for the one policy every
config uses by default. weighted/round-robin already gated on it via
PlacementPolicyUtil.available().

Extract the "at cap" predicate into PlacementPolicyUtil.atCap(ctx, c)
so all three policies share one definition, and consult it at both of
FixedPlacementPolicy's filter sites (the default fast path and the
candidate walk), mirroring the existing weightExcluded pattern. An
at-cap default now falls through to the next candidate instead of
refusing the spawn -- only when every candidate is unusable does the
policy still throw, naming the cap in the message. Update the class
javadoc (five exceptions -> six) and the reason-priority comments to
match CompositePeerLauncher's explicit-spawn order (quarantine,
cooling off, max load, model-off).
ltms merged commit 5d422f85fa into main 2026-09-10 08:54:19 +02:00
Sign in to join this conversation.