Give the controller lease back at the stop signal and say a takeover once, so a controller delivery raises no urgent alarm (issue 906, ADR 0558)

The 2026-10-26 replacements showed the lease was given back, but last: the
shutdown took 10 s, the gate had closed at half the key's age, and the release
recorded the epoch released only through the gate, so the next holder found
it expired and S12 said it, urgent, for an hour.

Now the stop signal gives the key back first and records the epoch released
whenever the delete succeeded; a candidate reads the key's delete marker on
the bus to tell a handover from an expiry; and a real takeover (lost or
expired) is raised and cleared once by the controller that took it, instead
of an hour of S12 observation.

Research 072 piece moved: the lease's takeover read from its own key on the
bus and said as an event (03-domains, mesh-controller row; 04-findings 2,
the watchdogs' age reading).
Quick fix weighed: drop the gate check from the release record only; it
leaves the release last in the shutdown and the hour-long alarm for a real
takeover.
This commit is contained in:
2026-10-27 00:49:13 +01:00
parent 2dafbe60e9
commit fbaa463a38
8 changed files with 262 additions and 36 deletions
+13 -3
View File
@@ -67,14 +67,24 @@ func (i *Inventory) HighestEpoch(ctx context.Context) (uint64, error) {
}
// TookEpoch records an epoch taken, and ends every earlier one still open as found expired: the lease
// was free to take, so whoever held it last neither holds it nor said it let go. Answers the epochs it
// ended that way, newest first.
func (i *Inventory) TookEpoch(ctx context.Context, e Epoch) ([]Epoch, error) {
// was free to take, so whoever held it last neither holds it nor said it let go. givenBack is the bus's word
// that the key was given back (the lease's own delete marker, novox/hq issue 906): then the newest earlier
// epoch still open is ended as released — its holder gave the key back and had not yet recorded it — and only
// the ones before it as expired. Answers the epochs it ended as expired, newest first.
func (i *Inventory) TookEpoch(ctx context.Context, e Epoch, givenBack bool) ([]Epoch, error) {
tx, err := i.store.Pool().Begin(ctx)
if err != nil {
return nil, err
}
defer func() { _ = tx.Rollback(ctx) }()
if givenBack {
if _, err := tx.Exec(ctx,
`update controller_epoch set ended = now(), how = $2
where epoch = (select max(epoch) from controller_epoch where ended is null and epoch < $1)`,
int64(e.Epoch), EpochReleased); err != nil {
return nil, err
}
}
rows, err := tx.Query(ctx,
`update controller_epoch set ended = now(), how = $2
where ended is null and epoch < $1
+37 -6
View File
@@ -139,6 +139,9 @@ type Lease struct {
lostWhy error
lost chan struct{}
taken time.Time
// givenBack is whether the key this instance took had been given back by its last holder — the bus's
// delete marker on it — rather than left to expire (novox/hq issue 906).
givenBack bool
// update writes the key at a revision: the bucket's own, or a test's bus that does not answer.
update func(ctx context.Context, key string, value []byte, last uint64) (uint64, error)
}
@@ -271,6 +274,7 @@ func (l *Lease) TryTake(ctx context.Context) (uint64, error) {
} else if found {
return 0, &heldBy{h}
}
givenBack := l.wasGivenBack(ctx)
if err := l.aboveTheFloor(ctx); err != nil {
return 0, err
}
@@ -307,13 +311,37 @@ func (l *Lease) TryTake(ctx context.Context) (uint64, error) {
"back to be taken again", revision, floor)
}
l.mu.Lock()
l.held, l.epoch, l.revision, l.taken = true, revision, revision, now
l.held, l.epoch, l.revision, l.taken, l.givenBack = true, revision, revision, now, givenBack
l.renewedAt(anchor)
l.mu.Unlock()
l.o.Say("took the controller lease at epoch %d (%s)", revision, h.Instance)
return revision, nil
}
// wasGivenBack is whether the free key was given back: its last message on the bus is the delete marker a
// holder's Release writes. A key that expired has no message left — the bucket's age removed it — and one that
// cannot be read is answered as not given back, so a takeover nobody can prove was a handover is said as one
// that was not (novox/hq issue 906).
func (l *Lease) wasGivenBack(ctx context.Context) bool {
if l.stream == nil {
return false
}
last, err := l.stream.GetLastMsgForSubject(ctx, "$KV."+l.kv.Bucket()+"."+Key)
if err != nil || last == nil {
return false
}
op := last.Header.Get("KV-Operation")
return op == "DEL" || op == "PURGE"
}
// GivenBack is whether the key this instance took had been given back by its last holder, not left to expire:
// what the takeover was, read from the bus (novox/hq issue 906).
func (l *Lease) GivenBack() bool {
l.mu.Lock()
defer l.mu.Unlock()
return l.givenBack
}
// floor is the highest epoch issued, zero without a floor.
func (l *Lease) floor(ctx context.Context) (uint64, error) {
if l.o.Floor == nil {
@@ -588,9 +616,11 @@ func (l *Lease) lose(why error) error {
return l.lostWhy
}
// Release gives the lease back, so the next candidate takes it at once rather than after its age.
// Only the key this instance last wrote is deleted: one that moved is somebody else's.
func (l *Lease) Release(ctx context.Context) {
// Release gives the lease back, so the next candidate takes it at once rather than after its age, and answers
// the epoch it gave back — zero when it gave nothing back: it held none, had lost it, or the bus would not
// delete the key. Only the key this instance last wrote is deleted: one that moved is somebody else's. The
// gate closes before the key goes, so nothing this instance starts after overlaps its successor.
func (l *Lease) Release(ctx context.Context) uint64 {
l.mu.Lock()
held, revision, epoch := l.held && l.lostWhy == nil, l.revision, l.epoch
l.held = false
@@ -600,7 +630,7 @@ func (l *Lease) Release(ctx context.Context) {
}
l.mu.Unlock()
if !held {
return
return 0
}
releasing, cancel := context.WithTimeout(context.WithoutCancel(ctx), 5*time.Second)
defer cancel()
@@ -619,7 +649,8 @@ func (l *Lease) Release(ctx context.Context) {
if err != nil {
l.o.Say("the controller lease (epoch %d) could not be given back, so the next controller waits for "+
"it to expire: %v", epoch, err)
return
return 0
}
l.o.Say("gave the controller lease back (epoch %d)", epoch)
return epoch
}
+17 -3
View File
@@ -133,7 +133,12 @@ func TestTheSecondControllerWaitsAndTakesAHigherEpochOnHandover(t *testing.T) {
// a gives it back: b takes it at once, not after the key's age, at a higher epoch.
stop()
handedOver := time.Now()
a.Release(t.Context())
if given := a.Release(t.Context()); given != epochA {
t.Fatalf("a's release answers epoch %d given back, want %d", given, epochA)
}
if again := a.Release(t.Context()); again != 0 {
t.Fatalf("a second release gives epoch %d back again", again)
}
select {
case epochB := <-took:
if epochB <= epochA {
@@ -142,6 +147,9 @@ func TestTheSecondControllerWaitsAndTakesAHigherEpochOnHandover(t *testing.T) {
if waited := time.Since(handedOver); waited > testTTL {
t.Fatalf("b took the lease %s after a gave it back: it waited out the key's age", waited)
}
if !b.GivenBack() {
t.Fatal("b does not read from the bus that a gave the key back (novox/hq issue 906)")
}
case <-time.After(2 * testTTL):
t.Fatal("b never took the lease a gave back")
}
@@ -182,6 +190,9 @@ func TestAHolderThatStopsRenewingStopsActingBeforeItIsTakenOver(t *testing.T) {
if epochB <= epochA {
t.Fatalf("b took epoch %d after a's %d", epochB, epochA)
}
if b.GivenBack() {
t.Fatal("b reads a key that expired as given back (novox/hq issue 906)")
}
// And a, the moment it tries to renew, knows it lost: the key moved.
if err := a.Renew(t.Context()); err == nil {
t.Fatal("a renewed a lease b holds")
@@ -226,8 +237,11 @@ func TestARenewalRefusedIsALossAtOnce(t *testing.T) {
if _, err := a.Epoch(); !errors.Is(err, ErrNotHeld) {
t.Fatalf("a acts after its renewal was refused: %v", err)
}
// And giving back a lease it lost deletes nothing of the one who holds it now.
a.Release(t.Context())
// And giving back a lease it lost deletes nothing of the one who holds it now, and answers no epoch given
// back, so none is recorded released (novox/hq issue 906).
if given := a.Release(t.Context()); given != 0 {
t.Fatalf("a says it gave back epoch %d of a lease it lost", given)
}
if h, found, err := Current(t.Context(), kv); err != nil || !found || h.Instance != "intruder" {
t.Fatalf("after a gave back what it lost, the key holds %+v (%v, %v)", h, found, err)
}