fleetd #435: FixedPlacementPolicy now honors maxLoad #436
Reference in New Issue
Block a user
Delete Branch "worker/435-fixed-policy-cap-fe11de-12"
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?
fleetd #435: FixedPlacementPolicy ignored maxLoad on the default automatic-placement path.
The defect
FleetConfig.Profile'smaxLoadjavadoc says an at-cap profile "is excluded from everyautomatic policy's candidate pool the same way a
weight <= 0profile is", citingPlacementPolicyUtil.available(). That helper has exactly two callers --weightedandround-robin-- andFixedPlacementPolicy(the default policy for an absent/blankplacement:key) is not one of them. Measured on7667727: a single dev profile atmaxLoad: 1with 1 live, underfixed(), unqualified spawn -- "SPAWNED on profile=a".The fix (direction A, as specified in the ticket)
PlacementPolicyUtil.atCap(ctx, candidate)so allthree policies share one definition, instead of
available()/emptyException()eachcomputing it inline while
fixedcomputed nothing at all.FixedPlacementPolicy.selectnow consults it at both filter sites: the default fast pathand the candidate walk (mirroring the existing
weightExcludedlookup-by-name pattern via anew small
candidateFor/capExcludedhelper pair).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)).dAtCap) sits betweendCoolingOffanddModelOff, anddModelOff's own guard now excludesdAtCap, matchingCompositePeerLauncher'sexplicit-spawn check order (quarantine, cooling off, max load, model-off).
FixedPlacementPolicy's class javadoc: the old "this ignores caps... deliberatelyout 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 theprofile); every candidate capped throws naming the cap; an uncapped default is still chosen
(mirror -- the new term cannot exclude everything);
maxLoad: 0on the default; quarantinestill wins over at-cap when both apply on the default.
CompositePeerLauncherTest: one test throughCompositePeerLauncher.spawnwith a blankprofile (
fixedPolicyGatesDefaultProfileAtMaxLoadOnUnqualifiedSpawn) -- proves the calleractually reaches the fix, not only the policy in isolation.
Build
Full
mvn clean installin 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-identicalto the fixed source via
diff.Removed
&& !PlacementPolicyUtil.atCap(ctx, c)from the candidate walk (line 69) -> 3 testsfail (
fixedFallbackWalkSkipsCappedCandidate,fixedThrowsWhenDefaultAndEveryCandidateAtCap,fixedPolicyGatesDefaultProfileAtMaxLoadOnUnqualifiedSpawn). Restored, verifiedbyte-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:
fixednowgates on
maxLoadat both filter sites, matchingweighted/round-robin; an at-cap defaultfalls through rather than refusing the spawn.
Note (scope: observed, not fixed)
Did not investigate other places where
PlacementPolicyUtilor a similar shared helper mightbe bypassed by one implementation of an interface -- out of this ticket's scope.