diff --git a/CHANGELOG.md b/CHANGELOG.md index efb67b3..7804ee8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,15 @@ What was left of the field report from v0.10.6. account; `/reset` sets the password once and closes every session. Without mail, `llmgw users reset-link` makes the link for an operator to hand over. +### Fixed + +- **A failover no longer replaces healthy replicas**: a standby that took + over placed at once, before the agents had reconnected and said what runs, + so it started a second copy of replicas it did not know of and then drained + the healthy ones as surplus — on the reference cluster the guardrails' + judge and a model's replica, within twenty seconds of a leader restarting. + Placement now waits after a takeover as it does at start. + ### Changed - **A GPU placement llama.cpp says it computes on the CPU is refused**, from diff --git a/internal/control/reconciler/leadership_test.go b/internal/control/reconciler/leadership_test.go index 89c0f7e..753781a 100644 --- a/internal/control/reconciler/leadership_test.go +++ b/internal/control/reconciler/leadership_test.go @@ -46,3 +46,36 @@ func TestWithoutLeadershipConfiguredNothingChanges(t *testing.T) { t.Error("a reconciler with no leadership configured placed nothing") } } + +// switchLeader is leadership that can change hands. +type switchLeader struct{ on bool } + +func (s *switchLeader) IsLeader() bool { return s.on } + +func TestAStandbyThatTakesOverWaitsForTheFleetBeforePlacing(t *testing.T) { + r, inv, disp, c := settlingHarness(t, testModel(1)) + lead := &switchLeader{} + r.SetLeadership(lead) + addNode(inv, c, gpuNode("1", 8)) + + // A standby for longer than the window: its own start is long past. + r.Reconcile(t.Context()) + c.advance(2 * DefaultConfig().SettleFor) + touchAt(inv, "1", c.now()) + r.Reconcile(t.Context()) + + // It takes over. The agents have not told it what runs yet; placing now + // would start a second copy of what they are about to report. + lead.on = true + r.Reconcile(t.Context()) + if n := len(disp.startedShards()); n != 0 { + t.Fatalf("placed %d shard(s) the moment it took over", n) + } + // Once the fleet has had its window, it places what is still missing. + c.advance(DefaultConfig().SettleFor) + touchAt(inv, "1", c.now()) + r.Reconcile(t.Context()) + if len(disp.startedShards()) == 0 { + t.Fatal("nothing placed after the window") + } +} diff --git a/internal/control/reconciler/reconciler.go b/internal/control/reconciler/reconciler.go index d82d422..9e94c43 100644 --- a/internal/control/reconciler/reconciler.go +++ b/internal/control/reconciler/reconciler.go @@ -307,6 +307,9 @@ type Reconciler struct { startedAt time.Time lastNodeCount int steadySince time.Time + // stoodBy is whether the last pass found this process a standby: one + // that then takes over settles again, as at start. + stoodBy bool } // New builds a reconciler. @@ -412,8 +415,22 @@ func (r *Reconciler) Reconcile(ctx context.Context) { // Unset means leader, so a single control plane and every test behave // exactly as they did before leadership existed. if r.leader != nil && !r.leader.IsLeader() { + r.stoodBy = true return } + // Taking over from another control plane is a start as far as the fleet + // is concerned: a standby refuses agents, so it knows none of what runs, + // and placing before they have reconnected and said starts a second + // copy of every replica — then drains the healthy one it finds next. + // Measured on the cluster: a failover replaced the guardrails' judge and + // a model's replica within twenty seconds, for nothing. + // From the moment it leads, not from when the process started: a + // standby a minute old had its window run out while it was refusing the + // very agents the window waits for. + if r.stoodBy { + r.stoodBy = false + r.resettle() + } ctx, p := beginPass(ctx) defer p.end() r.expireNodes(ctx) @@ -467,6 +484,14 @@ func (r *Reconciler) stopReplicasOfRemovedModels(ctx context.Context) { } } +// resettle opens the settling window again from now, for a control plane +// that has just become the leader. +func (r *Reconciler) resettle() { + // Like the rest of the window, touched only by Reconcile's own pass. + r.startedAt, r.lastNodeCount, r.steadySince = r.now(), -1, r.now() + r.log.Info("leading now: placement waits for the fleet to report what runs", "for up to", r.cfg.SettleFor) +} + // fleetHasSettled reports whether placement may run yet. // // Two ways out, because either alone is wrong. Waiting for the window to expire