diff --git a/fleetd/src/main/java/dev/ltms/fleet/placement/FixedPlacementPolicy.java b/fleetd/src/main/java/dev/ltms/fleet/placement/FixedPlacementPolicy.java index 1389eef..4a0ca61 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/placement/FixedPlacementPolicy.java +++ b/fleetd/src/main/java/dev/ltms/fleet/placement/FixedPlacementPolicy.java @@ -12,7 +12,7 @@ import java.util.List; * checked for reachability up front, only skipped once it has already failed in this same * spawn call's retry loop — see the unreachable case below. * - *

Four exceptions walk past the default instead of returning it unconditionally: + *

Five exceptions walk past the default instead of returning it unconditionally: *

- * A fleet where nothing is ever quarantined, cooling off, unreachable, or weight-0 never exercises - * any of these paths, so today's behaviour is unchanged — in particular, the very first selection - * of a spawn call always sees an empty {@code unreachable} set, so the first choice is untouched. + * A fleet where nothing is ever quarantined, cooling off, model-off, unreachable, or weight-0 never + * exercises any of these paths, so today's behaviour is unchanged — in particular, the very first + * selection of a spawn call always sees an empty {@code unreachable} set, so the first choice is + * untouched. */ final class FixedPlacementPolicy implements PlacementPolicy { @@ -42,12 +51,13 @@ final class FixedPlacementPolicy implements PlacementPolicy { public PlacementCandidate select(PlacementContext ctx) { String d = ctx.defaultProfile(); if (d != null && !d.isBlank() && !ctx.quarantined().contains(d) && !ctx.coolingOff().contains(d) - && !ctx.unreachable().contains(d) && !weightExcluded(ctx, d)) { + && !ctx.modelOff().contains(d) && !ctx.unreachable().contains(d) && !weightExcluded(ctx, d)) { return new PlacementCandidate(d, null, 1.0f, null); } for (PlacementCandidate c : ctx.candidates()) { if (!ctx.quarantined().contains(c.profile()) && !ctx.coolingOff().contains(c.profile()) - && !ctx.unreachable().contains(c.profile()) && !c.excluded()) { + && !ctx.modelOff().contains(c.profile()) && !ctx.unreachable().contains(c.profile()) + && !c.excluded()) { return new PlacementCandidate(c.profile(), null, c.weight(), c.maxLoad()); } } @@ -56,9 +66,14 @@ final class FixedPlacementPolicy implements PlacementPolicy { // Exhaustion quarantine takes priority: reported only when quarantine is absent, so the // message never claims "cooling off" for a profile that is really backend-exhausted. boolean dCoolingOff = !dQuarantined && ctx.coolingOff().contains(d); + // fleetd #422: model-off is a fourth, independent source (an operator decision) — but + // quarantine/cooling-off still take priority when more than one applies, matching + // CompositePeerLauncher's explicit-spawn check order (quarantine, cooling off, max load, + // then model-off). + boolean dModelOff = !dQuarantined && !dCoolingOff && ctx.modelOff().contains(d); boolean dUnreachable = ctx.unreachable().contains(d); boolean dWeightExcluded = weightExcluded(ctx, d); - if (dQuarantined || dCoolingOff || dUnreachable || dWeightExcluded) { + if (dQuarantined || dCoolingOff || dModelOff || dUnreachable || dWeightExcluded) { List reasons = new ArrayList<>(); if (dQuarantined) { reasons.add("is quarantined (backend exhausted)"); @@ -66,6 +81,9 @@ final class FixedPlacementPolicy implements PlacementPolicy { if (dCoolingOff) { reasons.add("is cooling off after repeated backend errors"); } + if (dModelOff) { + reasons.add("names a model the operator has turned off in models.allow"); + } if (dUnreachable) { reasons.add("is unreachable"); } @@ -78,7 +96,7 @@ final class FixedPlacementPolicy implements PlacementPolicy { } if (!ctx.candidates().isEmpty()) { throw new PlacementException("all worker profiles are excluded from automatic " - + "selection (quarantined, cooling off, unreachable, or weight-0)"); + + "selection (quarantined, cooling off, model-off, unreachable, or weight-0)"); } throw new PlacementException("no worker profiles configured"); } diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java index 5dd9311..ad05a68 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java @@ -1353,6 +1353,31 @@ class CompositePeerLauncherTest { + e.getMessage()); } + /** + * fleetd #422 follow-up: {@code fixed} is the DEFAULT placement policy ({@code + * PlacementPolicies.fromName} returns it for an absent/blank name), and it built its own inline + * candidate filter instead of calling {@code PlacementPolicyUtil.available()} — so it never + * checked {@code modelOff()}. Mirrors {@code placementSkipsAnOffModelProfileAndRoutesToAnotherOne} + * above with only the policy swapped, to prove the gate now fires on the path most fleets use. + */ + @Test + void fixedPlacementSkipsAnOffModelProfileToo() { + FakeHerdr herdr = new FakeHerdr(); + Map profiles = ordered( + "local", stubWorkerModel("local", "deepseek-v4-flash"), + "sonnet", stubWorkerModel("sonnet", "claude-sonnet-5")); + StubLauncher adapter = new StubLauncher("claude", herdr, profiles, "local", Set.of()); + FleetConfig.Models models = new FleetConfig.Models(List.of( + new FleetConfig.Models.ModelEntry("deepseek-v4-flash", false))); + CompositePeerLauncher composite = new CompositePeerLauncher(List.of(adapter), "local", profiles, + PlacementPolicies.fixed(), _ -> 0, null, BackendQuarantine.none(), NO_OUTAGE, + () -> models); + + PeerHandle h = composite.spawn(new SpawnRequest(null, null, null)); + assertEquals("sonnet", h.profile(), "local's model is off, so an unqualified spawn must land on sonnet"); + assertEquals(0, adapter.spawnCount("local")); + } + /** * Criterion 4: turning a model off/on is HOT — no restart — proven through a REAL * {@code ConfigRef.reload()}, not a hand-rolled supplier swap. Also proves {@code models} is diff --git a/fleetd/src/test/java/dev/ltms/fleet/placement/PlacementPolicyTest.java b/fleetd/src/test/java/dev/ltms/fleet/placement/PlacementPolicyTest.java index 69e5de4..8f0f73b 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/placement/PlacementPolicyTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/placement/PlacementPolicyTest.java @@ -90,6 +90,63 @@ class PlacementPolicyTest { assertTrue(e.getMessage().contains("quarantined"), e.getMessage()); } + // --- fleetd #422: FixedPlacementPolicy must consult modelOff too, at BOTH filter sites ------ + + /** The default-profile fast path (:44) must skip a default whose model is off. */ + @Test + void fixedSkipsModelOffDefault() { + PlacementPolicy policy = PlacementPolicies.fixed(); + PlacementContext ctx = new PlacementContext("b", + List.of(PlacementCandidate.profile("a"), PlacementCandidate.profile("b")), + noSessions(), Set.of(), Set.of(), Set.of(), Set.of("b")); + assertEquals("a", policy.select(ctx).profile(), + "the default 'b' names an off model, so fixed falls through to the first available candidate"); + } + + /** + * The fallback walk (:48-53) must skip an off-model candidate too — exercised independently of + * the default-profile fast path by using no default at all, so this is the only filter that runs. + */ + @Test + void fixedFallbackWalkSkipsModelOffCandidate() { + PlacementPolicy policy = PlacementPolicies.fixed(); + PlacementContext ctx = new PlacementContext(null, + List.of(PlacementCandidate.profile("a"), PlacementCandidate.profile("b")), + noSessions(), Set.of(), Set.of(), Set.of(), Set.of("a")); + assertEquals("b", policy.select(ctx).profile(), + "candidate 'a' names an off model, so the fallback walk skips it and picks 'b'"); + } + + @Test + void fixedThrowsWhenDefaultAndEveryCandidateModelOff() { + PlacementPolicy policy = PlacementPolicies.fixed(); + PlacementContext ctx = new PlacementContext("b", + List.of(PlacementCandidate.profile("a"), PlacementCandidate.profile("b")), + noSessions(), Set.of(), Set.of(), Set.of(), Set.of("a", "b")); + PlacementException e = assertThrows(PlacementException.class, () -> policy.select(ctx)); + assertTrue(e.getMessage().contains("turned off"), + "message names model-off as the cause: " + e.getMessage()); + assertFalse(e.getMessage().contains("quarantined"), "must not read like quarantine: " + e.getMessage()); + assertFalse(e.getMessage().contains("cooling off"), "must not read like cool-off: " + e.getMessage()); + assertFalse(e.getMessage().contains("weight 0"), "must not read like weight-0: " + e.getMessage()); + } + + /** + * Quarantine still wins when a profile is both quarantined and model-off (mirrors {@code + * fixedThrowsWhenDefaultAndEveryCandidateQuarantined}'s priority over cooling off). + */ + @Test + void fixedReportsQuarantineNotModelOffWhenBothApply() { + PlacementPolicy policy = PlacementPolicies.fixed(); + PlacementContext ctx = new PlacementContext("b", + List.of(PlacementCandidate.profile("a"), PlacementCandidate.profile("b")), + noSessions(), Set.of(), Set.of("a", "b"), Set.of(), Set.of("a", "b")); + PlacementException e = assertThrows(PlacementException.class, () -> policy.select(ctx)); + assertTrue(e.getMessage().contains("quarantined"), e.getMessage()); + assertFalse(e.getMessage().contains("turned off"), + "quarantine takes priority over model-off in the message: " + e.getMessage()); + } + @Test void roundRobinCyclesThroughAvailableProfiles() { PlacementPolicy policy = PlacementPolicies.roundRobin();