amadan

carlos / platform Public

zdd-5 task 7: a newer ack supersedes a held one; activate releases only the delivered revision

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude a12b53c57f32e90ebf466f9d01e9c66f8cb2ef2f parent 8900810
2 files changed, +66 −2
  • internal/rollout/config.go +6 −2
  • internal/rollout/config_test.go +60 −0
diff --git a/internal/rollout/config.go b/internal/rollout/config.go
index b978997db..b84048bba 100644
--- a/internal/rollout/config.go
+++ b/internal/rollout/config.go
@@ -46,6 +46,10 @@ func configBackoff(n int) time.Duration {
// with today's hibernating acknowledgement: the config landed and nothing
// woke to run it (ConfigAck.Hibernating, "delivered on wake").
func (t *configTrack) deferAck(a appconfig.ConfigAck, write func(appconfig.ConfigAck)) func() {
+ // A newer delivery supersedes any ack still held for an older one, whether
+ // or not this one waits: releasing the old ack later would write it after
+ // the newer ack, with a revision that is no longer delivered (spec §8.1).
+ t.ack, t.ackWrite, t.ackRev = nil, nil, ""
if t.sleeping {
a.Hibernating, a.Restarted = true, false
return func() { write(a) }
@@ -58,7 +62,7 @@ func (t *configTrack) deferAck(a appconfig.ConfigAck, write func(appconfig.Confi
}
// activate records rev as active and returns the held ack when it was
-// waiting for exactly rev.
+// waiting for exactly rev and rev is still the delivered revision.
func (t *configTrack) activate(rev string) (func(), bool) {
if rev == "" {
return nil, false
@@ -67,7 +71,7 @@ func (t *configTrack) activate(rev string) (func(), bool) {
if rev == t.delivered {
t.fails, t.lastErr = 0, ""
}
- if t.ack == nil || t.ackRev != rev {
+ if t.ack == nil || t.ackRev != rev || rev != t.delivered {
return nil, false
}
a, write := *t.ack, t.ackWrite
diff --git a/internal/rollout/config_test.go b/internal/rollout/config_test.go
index 4431920ab..62c1eb22a 100644
--- a/internal/rollout/config_test.go
+++ b/internal/rollout/config_test.go
@@ -57,3 +57,63 @@ func TestASleepersAckGoesOutAtDeliveryAsHibernating(t *testing.T) {
t.Fatalf("ack = %+v, want Hibernating and not Restarted", got)
}
}
+
+// Spec §8.1: a revision is acknowledged only once active. A held ack that a
+// newer delivery bypassed must never be released afterwards, or it would be
+// written after the newer ack with a revision that is no longer delivered.
+func TestANewerAckSupersedesTheHeldOne(t *testing.T) {
+ t.Run("revert to the active revision", func(t *testing.T) {
+ var wrote []string
+ write := func(a appconfig.ConfigAck) { wrote = append(wrote, a.Host) }
+ tr := &configTrack{delivered: "c2", active: "c1"}
+ if now := tr.deferAck(appconfig.ConfigAck{Host: "held-c2"}, write); now != nil {
+ t.Fatal("c2's ack was not held while c1 is active")
+ }
+ // The config reverts to c1, which is the active revision.
+ tr.delivered = "c1"
+ now := tr.deferAck(appconfig.ConfigAck{Host: "c1"}, write)
+ if now == nil {
+ t.Fatal("c1's ack waited though c1 is active")
+ }
+ now()
+ // The in-flight generation finishes on c2.
+ if run, ok := tr.activate("c2"); run != nil || ok {
+ t.Fatal("the stale c2 ack was released after a newer ack went out")
+ }
+ if len(wrote) != 1 || wrote[0] != "c1" {
+ t.Fatalf("wrote %v, want only the c1 ack", wrote)
+ }
+ })
+ t.Run("sleep then hibernating ack", func(t *testing.T) {
+ var wrote []string
+ write := func(a appconfig.ConfigAck) { wrote = append(wrote, a.Host) }
+ tr := &configTrack{delivered: "c2", active: "c1"}
+ if now := tr.deferAck(appconfig.ConfigAck{Host: "held-c2"}, write); now != nil {
+ t.Fatal("c2's ack was not held while c1 is active")
+ }
+ // The instance sleeps and c3 is delivered to it.
+ tr.delivered, tr.sleeping = "c3", true
+ now := tr.deferAck(appconfig.ConfigAck{Host: "c3"}, write)
+ if now == nil {
+ t.Fatal("a sleeper's ack waited")
+ }
+ now()
+ if run, ok := tr.activate("c2"); run != nil || ok {
+ t.Fatal("the stale c2 ack was released after the Hibernating ack went out")
+ }
+ if len(wrote) != 1 || wrote[0] != "c3" {
+ t.Fatalf("wrote %v, want only the c3 ack", wrote)
+ }
+ })
+}
+
+// A held ack is released only for the revision that is delivered now, even
+// when delivered moved on without another ack arriving.
+func TestAHeldAckIsNotReleasedForARevisionNoLongerDelivered(t *testing.T) {
+ tr := &configTrack{delivered: "c2", active: "c1"}
+ tr.deferAck(appconfig.ConfigAck{Host: "a"}, func(appconfig.ConfigAck) { t.Fatal("wrote a stale ack") })
+ tr.delivered = "c3"
+ if run, ok := tr.activate("c2"); run != nil || ok {
+ t.Fatal("released an ack for c2 while c3 is delivered")
+ }
+}