diff --git a/fleetd/src/main/java/dev/ltms/fleet/placement/BackendOutagePolicy.java b/fleetd/src/main/java/dev/ltms/fleet/placement/BackendOutagePolicy.java index 5d202ab..12cc865 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/placement/BackendOutagePolicy.java +++ b/fleetd/src/main/java/dev/ltms/fleet/placement/BackendOutagePolicy.java @@ -20,13 +20,16 @@ import java.util.function.LongSupplier; * *

Two classified errors on the same credential — never the profile name, never the error * text, see {@code FleetConfig.Profile#effectiveCredentialId()} like {@link BackendQuarantine} — - * inside a 60-second window is treated as an outage: {@link #record} then returns an {@link - * Incident} and starts a 60-second cool-off for that credential. A single error is common (the - * classifier is a heuristic and a valid member report can quote an {@code API Error:} line), so one - * false match must never remove fleet capacity; two is the smallest threshold that protects an - * honest one-turn failure. This is deliberately a different store from {@link BackendQuarantine}: - * that one holds a 1800-second exhaustion cooldown for a spent credential, and reusing it here would - * both use the wrong duration and report "backend exhausted" for what is a short transient fault. + * from two distinct targets inside a 60-second window is treated as an outage: {@link + * #record} then returns an {@link Incident} and starts a 60-second cool-off for that credential. The + * threshold counts distinct targets, not raw events, on purpose: the classifier is a heuristic and a + * valid member report can quote an {@code API Error:} line, so one target repeating that line twice + * must never remove fleet capacity by itself. Two different targets independently producing a + * classified error is far less likely to be a coincidence, and a real backend outage hits every + * target on that credential anyway, so this loses nothing against the case being protected against. + * This is deliberately a different store from {@link BackendQuarantine}: that one holds a + * 1800-second exhaustion cooldown for a spent credential, and reusing it here would both use the + * wrong duration and report "backend exhausted" for what is a short transient fault. * *

Correlation and cool-off live in one class so that crossing the threshold and setting * the cool-off deadline happen as a single atomic update: every {@link #record} call goes through @@ -40,7 +43,7 @@ import java.util.function.LongSupplier; */ public final class BackendOutagePolicy { - /** Classified errors on one credential needed to declare an outage. */ + /** Distinct targets a credential needs a classified error from to declare an outage. */ public static final int THRESHOLD = 2; /** How long a credential's evidence stays fresh, inclusive of both endpoints. */ @@ -60,14 +63,18 @@ public final class BackendOutagePolicy { /** * Records one classified backend error for {@code credentialId} against {@code target} (e.g. a * session or worker id — this class never interprets it, only collects it for the incident's - * affected-targets list) with {@code reason} (the classifier's free-text reason, kept per event - * for whoever renders the eventual notice). + * affected-targets list, and counts distinct targets toward the threshold) with {@code reason} + * (the classifier's free-text reason, kept per event for whoever renders the eventual notice — + * every event's reason is kept even when the same target repeats, so {@link Incident#reasons()} + * can be longer than {@link Incident#evidenceCount()}). * - *

Returns a populated {@link Incident} only at the exact moment the threshold is crossed — - * never before, and never again while the resulting cool-off is active. While a credential is - * cooling off, a fresh error is ignored outright: it neither extends the deadline nor produces - * another incident. Once the cool-off has elapsed, the next error clears the old evidence and - * starts a brand-new window — two fresh errors are required to rearm. + *

Returns a populated {@link Incident} only at the exact moment a second distinct target + * is seen for this credential inside the window — never before, and never again while the + * resulting cool-off is active. A repeat error from a target already counted does not advance the + * threshold. While a credential is cooling off, a fresh error is ignored outright: it neither + * extends the deadline nor produces another incident. Once the cool-off has elapsed, the next + * error clears the old evidence and starts a brand-new window — two fresh, distinct targets are + * required to rearm. */ public Optional record(String credentialId, String target, String reason) { Objects.requireNonNull(credentialId, "credentialId"); @@ -129,6 +136,12 @@ public final class BackendOutagePolicy { * One credential crossing the outage threshold. {@code remainingCoolOffSeconds} is the cool-off * length as observed at the moment of minting — this incident is only ever produced right as the * cool-off starts, so it is always the full {@link #COOLOFF_NANOS} rounded up. + * + *

{@code evidenceCount} is {@code targets.size()} — the threshold is on distinct targets, not + * raw events — while {@code reasons} keeps every event's reason, including repeats from a target + * already counted. The two are deliberately different lengths: a single target hammering the same + * classified error never grows {@code evidenceCount} past 1, but each occurrence still lands in + * {@code reasons} for whoever renders the notice. */ public record Incident( String id, @@ -175,8 +188,9 @@ public final class BackendOutagePolicy { return new CredentialState(firstEventNanos, targets, reasons, coolOffUntilNanos); } + /** Distinct targets seen so far — the threshold counts this, never {@code reasons.size()}. */ int evidenceCount() { - return reasons.size(); + return targets.size(); } boolean inCoolOff() { diff --git a/fleetd/src/test/java/dev/ltms/fleet/placement/BackendOutagePolicyTest.java b/fleetd/src/test/java/dev/ltms/fleet/placement/BackendOutagePolicyTest.java index 88b2721..1d3e39f 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/placement/BackendOutagePolicyTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/placement/BackendOutagePolicyTest.java @@ -55,6 +55,47 @@ class BackendOutagePolicyTest { assertEquals(OptionalLongOf60(), policy.remainingCoolOffSeconds("shared-openai")); } + @Test + void repeatedErrorsFromTheSameTargetNeverCreateAnIncidentHoweverManyTimesTheyRepeat() { + // The classifier is a heuristic and a valid member report can quote an "API Error:" line, so + // one target repeating that line inside the window must never, by itself, cost the credential + // its capacity — only a SECOND, DISTINCT target crossing the threshold does. + AtomicLong now = new AtomicLong(0L); + BackendOutagePolicy policy = new BackendOutagePolicy(now::get); + + for (int i = 0; i < 5; i++) { + now.set(i * 10L * SECOND); + Optional incident = + policy.record("shared-openai", "session-1", "API Error: 429"); + assertTrue(incident.isEmpty(), + "the same target repeating must never create an incident on its own (repeat #" + i + ")"); + } + assertTrue(policy.remainingCoolOffSeconds("shared-openai").isEmpty()); + } + + @Test + void evidenceCountIsDistinctTargetsWhileReasonsKeepsEveryEvent() { + AtomicLong now = new AtomicLong(0L); + BackendOutagePolicy policy = new BackendOutagePolicy(now::get); + + assertTrue(policy.record("shared-openai", "session-1", "API Error: 429").isEmpty()); + now.set(10 * SECOND); + // a repeat from the already-counted target: still no incident, still only 1 distinct target + assertTrue(policy.record("shared-openai", "session-1", "API Error: 429 (again)").isEmpty()); + now.set(20 * SECOND); + Optional incident = + policy.record("shared-openai", "session-2", "API Error: 500"); + + assertTrue(incident.isPresent()); + BackendOutagePolicy.Incident value = incident.get(); + assertEquals(Set.of("session-1", "session-2"), value.targets()); + assertEquals(2, value.evidenceCount(), "evidenceCount is the distinct-target count"); + assertEquals(3, value.reasons().size(), + "reasons keeps every event, including the same-target repeat that did not advance the count"); + assertEquals(java.util.List.of("API Error: 429", "API Error: 429 (again)", "API Error: 500"), + value.reasons()); + } + @Test void twoErrorsMoreThanAMinuteApartCreateNoIncident() { AtomicLong now = new AtomicLong(0L);