From 8e02c4726eb4a8cf7de9cc3d2ce0560f00b35157 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 17:09:35 +0100 Subject: [PATCH] Clear a module's fingerprint-not-composed on every successful take-in, and stop serve's goroutines before an error return undoes anything (issue 1945) A success cleared the condition only when the same follower had raised it, so one raised before a restart or by another follower stayed open after the take-in worked, and the operator read that a build needed a look. Clearing a condition that is not open costs one read. On an error return, serve now ends its goroutines before the keeper closes and before handActConn and the bus are put back (the review of fix 12). --- cmd/mesh-controller/composed_keys.go | 6 +++-- .../composed_keys_cancel_test.go | 22 +++++++++++++++++++ cmd/mesh-controller/push.go | 3 +++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/cmd/mesh-controller/composed_keys.go b/cmd/mesh-controller/composed_keys.go index 6ae5eea9..2b2ac625 100644 --- a/cmd/mesh-controller/composed_keys.go +++ b/cmd/mesh-controller/composed_keys.go @@ -166,10 +166,12 @@ func followDoneKey(ctx context.Context, open *stores, module string) { err := takeOnceInSlot(ctx, open, module) o := doneKeyObservation(module) if err == nil { - if keeper := conditionsFrom; keeper != nil && failed { + // Cleared on every success, whoever raised it: a controller before a restart or another follower (issue + // 1945). A condition that is not open costs one read. + if keeper := conditionsFrom; keeper != nil { _, _ = keeper.Clear(ctx, o.Key(), "composed and granted") - failed = false } + failed = false if _, again := doneKeysLookAgain.LoadAndDelete(module); again { continue // a wake came while this one ran: once more, for the key written meanwhile } diff --git a/cmd/mesh-controller/composed_keys_cancel_test.go b/cmd/mesh-controller/composed_keys_cancel_test.go index 34292364..0e34904c 100644 --- a/cmd/mesh-controller/composed_keys_cancel_test.go +++ b/cmd/mesh-controller/composed_keys_cancel_test.go @@ -59,3 +59,25 @@ func TestATakeInCutByACancellationRaisesNothing(t *testing.T) { }) } } + +// A take-in that succeeds clears the module's fingerprint-not-composed condition whoever raised it: a controller before a +// restart, or another follower (issue 1945: the operator read "a build needs a look" for take-ins that had since worked). +func TestASucceedingTakeInClearsAConditionAnotherRaised(t *testing.T) { + was, wasRetry, wasSay, wasKeeper := doneKeyTakeOnce, doneKeyRetry, doneKeysSay, conditionsFrom + t.Cleanup(func() { doneKeyTakeOnce, doneKeyRetry, doneKeysSay, conditionsFrom = was, wasRetry, wasSay, wasKeeper }) + store := conditions.NewInMemory() + k := conditions.NewKeeper(t.Context(), conditions.Options{Store: store, History: store, Teller: &conditions.Told{}}) + t.Cleanup(func() { k.Close(context.Background()) }) + conditionsFrom = k + doneKeysSay = func(string) {} + o := doneKeyObservation("raised-before") + o.Summary += "the store refused the connection" + if _, err := k.Observe(t.Context(), o); err != nil { + t.Fatal(err) + } + doneKeyTakeOnce = func(context.Context, *stores, string) error { return nil } + takeDoneKey(t.Context(), nil, "raised-before") + if c, found, err := conditions.ReadOne(t.Context(), store, o.Key()); err != nil || found { + t.Fatalf("a take-in that succeeded left the condition another raised: %+v (found %v, %v)", c, found, err) + } +} diff --git a/cmd/mesh-controller/push.go b/cmd/mesh-controller/push.go index 7d18c351..70c96fc7 100644 --- a/cmd/mesh-controller/push.go +++ b/cmd/mesh-controller/push.go @@ -179,6 +179,7 @@ func serve(ctx context.Context) (err error) { keeper := openServingConditions(ctx, open, bus) if keeper != nil { defer closeConditions(keeper) + defer stopRunning() // on an error return, the goroutines end before the keeper closes } // The bus's own objects, asserted on every start. **Not created once at genesis**: a stream @@ -247,10 +248,12 @@ func serve(ctx context.Context) (err error) { // next test's `conditions` with "connection closed", issue 1945). handActConn = bus.Conn defer func() { handActConn = nil }() + defer stopRunning() // on an error return, the goroutines end before handActConn is put back // And everything else this controller does on the bus for a moment (novox/hq issue 327). servingBus.Store(server.JetStream()) // No longer serving: nothing is lent, and dead-letters says it is not read here (novox/hq issue 330). defer servingBus.Store(nil) + defer stopRunning() // on an error return, the goroutines end before the bus is taken back // And says when it replaced a value given by hand (novox/hq ADR 0228). givenEvents = bus // And a pull request's merge check, asked when the forge announces its head and said when judged