diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/CompositePeerLauncher.java b/fleetd/src/main/java/dev/ltms/fleet/member/CompositePeerLauncher.java index 80d0237..f28f23f 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/CompositePeerLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/CompositePeerLauncher.java @@ -385,9 +385,12 @@ public final class CompositePeerLauncher implements PeerLauncher { } } + // unreachable.size() counts DISTINCT profiles, not attempts (a HashSet dedupes a profile + // added twice) — say "distinct" so the count matches the sentence and the profile list that + // follows, rather than reading as a count of attempts made (fleetd #315). throw new PeerUnreachableException( "no reachable worker profile available after trying " + unreachable.size() - + " candidate(s): " + String.join(", ", unreachable)); + + " distinct candidate(s): " + String.join(", ", unreachable)); } /** 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 b56635d..1389eef 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/placement/FixedPlacementPolicy.java +++ b/fleetd/src/main/java/dev/ltms/fleet/placement/FixedPlacementPolicy.java @@ -5,10 +5,14 @@ import java.util.List; /** * Backward-compatible placement: an unqualified spawn always resolves to the configured default - * profile, exactly as {@code CompositePeerLauncher} did before CB-518. This ignores caps and - * reachability so that a pre-existing config behaves identically after upgrade. + * profile, exactly as {@code CompositePeerLauncher} did before CB-518. This ignores caps + * ({@code maxLoad}) so that a pre-existing config behaves identically after upgrade — capacity + * gating for automatic placement is deliberately out of scope for {@code fixed}, exactly as it + * always has been. Reachability is a narrower exception (fleetd #315, below): a profile is never + * 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. * - *

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

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

- * A fleet where nothing is ever quarantined, cooling off, or weight-0 never exercises any of these - * paths, so today's behaviour is unchanged. + * 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. */ final class FixedPlacementPolicy implements PlacementPolicy { @@ -30,12 +42,12 @@ 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) - && !weightExcluded(ctx, 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()) - && !c.excluded()) { + && !ctx.unreachable().contains(c.profile()) && !c.excluded()) { return new PlacementCandidate(c.profile(), null, c.weight(), c.maxLoad()); } } @@ -44,8 +56,9 @@ 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); + boolean dUnreachable = ctx.unreachable().contains(d); boolean dWeightExcluded = weightExcluded(ctx, d); - if (dQuarantined || dCoolingOff || dWeightExcluded) { + if (dQuarantined || dCoolingOff || dUnreachable || dWeightExcluded) { List reasons = new ArrayList<>(); if (dQuarantined) { reasons.add("is quarantined (backend exhausted)"); @@ -53,6 +66,9 @@ final class FixedPlacementPolicy implements PlacementPolicy { if (dCoolingOff) { reasons.add("is cooling off after repeated backend errors"); } + if (dUnreachable) { + reasons.add("is unreachable"); + } if (dWeightExcluded) { reasons.add("has weight 0 (excluded from automatic selection)"); } @@ -62,7 +78,7 @@ final class FixedPlacementPolicy implements PlacementPolicy { } if (!ctx.candidates().isEmpty()) { throw new PlacementException("all worker profiles are excluded from automatic " - + "selection (quarantined, cooling off, or weight-0)"); + + "selection (quarantined, cooling 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 b50d0cd..7520cbc 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java @@ -615,6 +615,67 @@ class CompositePeerLauncherTest { assertEquals(1, adapter.spawnCount("b")); } + /** + * fleetd #315: {@code CompositePeerLauncher.spawn} rebuilds the {@link PlacementContext} after + * every failed attempt "so the policy excludes this profile" (see the comment at the retry call + * site) — but {@code FixedPlacementPolicy} never read {@code ctx.unreachable()}, so under the + * default {@code fixed} placement every retry re-picked the same dead default and a second, + * healthy, configured profile was never tried. This is the same scenario as + * {@link #failoverRetriesNextCandidateWhenProfileIsUnreachable}, but pinned to {@code fixed()} + * instead of {@code weighted()} — the three existing failover tests all use {@code weighted()}, + * which is exactly why nobody caught this: the retry loop's contract has no coverage under its + * own default policy. + */ + @Test + void failoverRetriesNextCandidateUnderFixedPlacementWhenProfileIsUnreachable() { + FakeHerdr herdr = new FakeHerdr(); + Map profiles = ordered( + "a", stubWorker("a"), + "b", stubWorker("b")); + StubLauncher adapter = new StubLauncher("claude", herdr, profiles, "a", Set.of("a")); + CompositePeerLauncher composite = new CompositePeerLauncher( + List.of(adapter), "a", profiles, PlacementPolicies.fixed(), _ -> 0); + + PeerHandle h = composite.spawn(new SpawnRequest(null, null, null)); + assertEquals("b", h.profile(), + "fixed placement must fail over from the unreachable default a to the healthy b"); + assertEquals(1, adapter.spawnCount("a"), "a was tried once and failed"); + assertEquals(1, adapter.spawnCount("b"), "b was tried once and succeeded"); + } + + /** + * fleetd #315: the same fix — {@code FixedPlacementPolicy} consulting {@code ctx.unreachable()} + * — also covers the wiring-bug branch in {@code CompositePeerLauncher.spawn}: a profile that + * placement is allowed to choose (it is in the configured candidate list) but that no delegate + * declares ({@code byProfile.get(chosen.profile()) == null}). That branch adds the profile to + * {@code unreachable} and {@code continue}s without ever calling a launcher, so before this fix + * {@code fixed} handed back the same adapterless profile on every remaining attempt too. + */ + @Test + void failoverSkipsAConfiguredProfileNoAdapterDeclaresUnderFixedPlacement() { + FakeHerdr herdr = new FakeHerdr(); + // Placement's candidate list has three profiles, in this order (LinkedHashMap preserves it, + // and the fixed default resolves to the first — see the `ordered` helper's own javadoc). + Map profiles = new LinkedHashMap<>(); + profiles.put("c", stubWorker("c")); + profiles.put("a", stubWorker("a")); + profiles.put("b", stubWorker("b")); + // The adapter only declares a and b — c is a configured profile with no owning adapter, + // the "wiring bug" the comment in CompositePeerLauncher.spawn calls out. + Map adapterProfiles = new LinkedHashMap<>(); + adapterProfiles.put("a", stubWorker("a")); + adapterProfiles.put("b", stubWorker("b")); + StubLauncher adapter = new StubLauncher("claude", herdr, adapterProfiles, "a", Set.of()); + CompositePeerLauncher composite = new CompositePeerLauncher( + List.of(adapter), "a", profiles, PlacementPolicies.fixed(), _ -> 0); + + PeerHandle h = composite.spawn(new SpawnRequest(null, null, null)); + assertEquals("a", h.profile(), + "c has no adapter, so fixed placement must skip it and land on the next candidate, a"); + assertEquals(0, adapter.spawnCount("c"), "c is never spawned — no adapter owns it"); + assertEquals(1, adapter.spawnCount("a")); + } + @Test void explicitSpawnAtMaxLoadThrowsPlacementExceptionNamingProfileLiveAndCap() { FakeHerdr herdr = new FakeHerdr();