Merge pull request 'Assigner: take a stale ask away only once its held condition's clear was said, so a failed clear is retried (issue 1906)' (#61) from fix/1906-clear-retried into main

modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go

@@ -481,13 +481,19 @@ func (p *passer) staleAsks(ctx context.Context, trunk map[string]delivery.Module
 	}
 	for key := range p.w.read(delivery.AssignerAsksBucket) {
 		if p.staleAsk(key, trunk) {
+			// The condition is cleared first and the ask goes only once that was said: the ask is what brings this key back at the
+			// next pass, so a clear that failed once is made again, with no state of its own (issue 1906, note 29).
+			k, kerr := delivery.ParseAnyAskKey(key)
+			held := kerr == nil && (k.Kind == delivery.AskKindHold || k.Kind == delivery.AskKindRestore)
+			if held && !p.clearHeld(k.Module, k.FP) {
+				continue
+			}
 			p.logf("the ask %s is about a fingerprint or composition the trunk has left behind and is taken away", key)
 			p.remove(ctx, delivery.AssignerAsksBucket, key)
 			// The held record stays: it is the memory that this fingerprint failed (ADR 0408 §4), and a revert returns the trunk to
 			// exactly it. Only the condition, which would else ask for an answer that can no longer be given, is cleared, and it is
 			// raised again if the trunk returns to the fingerprint.
-			if k, err := delivery.ParseAnyAskKey(key); err == nil && (k.Kind == delivery.AskKindHold || k.Kind == delivery.AskKindRestore) {
-				p.clearHeld(k.Module, k.FP)
+			if held {
 				delete(p.said, assign.HeldCondition(k.Module, k.FP, "", "", false, false).Key+".raised")
 			}
 		}
@@ -566,16 +572,18 @@ func (p *passer) answeredWith(key, option string) bool {
 }
 
 // clearHeld clears, once, the condition of a hold the operator released.
-func (p *passer) clearHeld(module, fp string) {
+// It says whether nothing is left to say: the clear was said now or before, or there is no router to say it to.
+func (p *passer) clearHeld(module, fp string) bool {
 	c := assign.HeldCondition(module, fp, "", "", false, false)
 	if p.say == nil || p.said[c.Key+".cleared"] {
-		return
+		return true
 	}
 	if err := p.say(c); err != nil {
 		p.logf("the condition of the released %s could not be cleared, and is cleared at the next pass: %v", module, err)
-		return
+		return false
 	}
 	p.said[c.Key+".cleared"] = true
+	return true
 }
 
 // recordAsk writes, once, the ask about a move of a module of policy record that waits for the operator's yes.

modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go

@@ -606,6 +606,36 @@ func TestAsksOfAFingerprintTheTrunkLeftBehindAreTakenAway(t *testing.T) {
 		})
 	}
 
+	t.Run("a clear that failed to be said is said at the next pass, and the ask goes after it", func(t *testing.T) {
+		asksKV, wantsKV := &memKV{values: map[string][]byte{}}, &memKV{values: map[string][]byte{}}
+		p := newPasser(buckets{delivery.AssignerAsksBucket: asksKV, delivery.AssignerWantsBucket: wantsKV}, time.Now())
+		var said []asks.Condition
+		fail := true
+		p.say = func(c asks.Condition) error {
+			if fail {
+				fail = false
+				return errors.New("router unreachable")
+			}
+			said = append(said, c)
+			return nil
+		}
+		a := assign.HoldAsk(holdOld, "web", fpOld, "novox", "w", 1, time.Now())
+		v, err := delivery.EncodeAssignerAsk(holdOld, a)
+		if err != nil {
+			t.Fatal(err)
+		}
+		asksKV.values[holdOld] = v
+		p.w.put(delivery.AssignerAsksBucket, holdOld, v)
+		p.staleAsks(context.Background(), trunkOf(fpNew))
+		if _, there := asksKV.values[holdOld]; !there || len(said) != 0 {
+			t.Fatalf("after a failed clear: ask left = %v, conditions said = %d; want the ask kept and nothing said", there, len(said))
+		}
+		p.staleAsks(context.Background(), trunkOf(fpNew))
+		if _, there := asksKV.values[holdOld]; there || len(said) != 1 || said[0].State != asks.StateCleared {
+			t.Errorf("at the next pass: ask left = %v, conditions said = %v; want the ask gone and the clear said", there, said)
+		}
+	})
+
 	t.Run("a lift takes the restore asks of its fingerprint on every node", func(t *testing.T) {
 		asksKV, wantsKV := &memKV{values: map[string][]byte{}}, &memKV{values: map[string][]byte{}}
 		p := newPasser(buckets{delivery.AssignerAsksBucket: asksKV, delivery.AssignerWantsBucket: wantsKV}, time.Now())