amadan

carlos / platform Public

lease: prove the dark-then-returns failover race (returning host must lose)

Issue #53 point 3: a host that goes dark (stops renewing, never releases —
kill -9 / power cut) must not silently reclaim its lease after a peer has
taken over. The conditional-write lock already guarantees it; this adds the
proof for that specific sequence.

- acquireTTL(ctx, bucket, prefix, region, ttl) is Acquire with the TTL as a
  seam; Acquire delegates with LeaseTTL. No exported surface change.
- startRenewer now ticks at leaser.TTL/3 rather than the LeaseTTL constant.
  Identical in production (leaser.TTL is always LeaseTTL there), but it is
  what makes a short-TTL lease actually renewed — without it a test lease
  lapses whether or not the holder went dark, and the new test would pass
  against a renewer that never ran.
- TestDarkHostCannotReclaimAfterFailover walks the sequence: A holds and goes
  dark; a peer is refused while A's lock is still in force (dark != expired);
  after expiry B takes over; A returns and must lose with a *HeldError
  carrying a live expiry.

Live-gated on CARLOS_TEST_S3_BUCKET like the rest of the package; the live
run is PENDING and must be executed before merge.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell 5df73eb3ef80543242cddc2c28d6c8c4d121b72a parent 8fe7d5e
2 files changed, +105 −2
  • internal/lease/lease.go +23 −2
  • internal/lease/lease_test.go +82 −0
diff --git a/internal/lease/lease.go b/internal/lease/lease.go
index 57c6d99a4..4827481ce 100644
--- a/internal/lease/lease.go
+++ b/internal/lease/lease.go
@@ -111,9 +111,20 @@ type renewer struct {
func startRenewer(leaser *lss3.Leaser, lease *litestream.Lease) *renewer {
ctx, cancel := context.WithCancel(context.Background())
rn := &renewer{leaser: leaser, lease: lease, cancel: cancel, done: make(chan struct{})}
+ // Renew at a third of the lease's OWN TTL, not the package constant: the
+ // leaser is the one that stamps expiry (leaser.TTL), so reading the
+ // interval off it keeps renewal and expiry in step for any TTL. In
+ // production leaser.TTL is always LeaseTTL, so this is LeaseTTL/3 as
+ // before; under a test's short TTL it is what makes "stopped renewing"
+ // distinguishable from "never renewed at all". The fallback only guards
+ // NewTicker's panic on a non-positive duration.
+ renewEvery := leaser.TTL / 3
+ if renewEvery <= 0 {
+ renewEvery = LeaseTTL / 3
+ }
go func() {
defer close(rn.done)
- t := time.NewTicker(LeaseTTL / 3)
+ t := time.NewTicker(renewEvery)
defer t.Stop()
for {
select {
@@ -175,6 +186,16 @@ type Held struct {
// signal the activator reacts to. Region may be empty; the AWS default chain
// (env/profile) resolves it.
func Acquire(ctx context.Context, bucket, prefix, region string) (*Held, error) {
+ return acquireTTL(ctx, bucket, prefix, region, LeaseTTL)
+}
+
+// acquireTTL is Acquire with the lease TTL as a seam. Production always
+// passes LeaseTTL; tests pass a short TTL so an expiry-and-takeover sequence
+// (the failover race) fits inside a test run instead of 15 minutes. The TTL
+// governs renewal spacing too — startRenewer ticks at leaser.TTL/3 — so a
+// short-TTL lease is renewed proportionally often and lapses proportionally
+// fast, which is what makes "the holder went dark" observable.
+func acquireTTL(ctx context.Context, bucket, prefix, region string, ttl time.Duration) (*Held, error) {
// The leaser wants a ready-made S3 client (unlike the replica client,
// which builds its own). Default AWS credential chain.
var opts []func(*awsconfig.LoadOptions) error
@@ -190,7 +211,7 @@ func Acquire(ctx context.Context, bucket, prefix, region string) (*Held, error)
leaser.SetClient(awss3.NewFromConfig(cfg))
leaser.Bucket = bucket
leaser.Path = prefix // lock lives at <prefix>/lock.json, colocated with the replica
- leaser.TTL = LeaseTTL
+ leaser.TTL = ttl
lease, err := leaser.AcquireLease(ctx)
if err != nil {
diff --git a/internal/lease/lease_test.go b/internal/lease/lease_test.go
index 299f5737a..ad090a6b8 100644
--- a/internal/lease/lease_test.go
+++ b/internal/lease/lease_test.go
@@ -233,3 +233,85 @@ func TestReplicateWithLease_MutualExclusion(t *testing.T) {
t.Fatalf("third close: %v", err)
}
}
+
+// TestDarkHostCannotReclaimAfterFailover proves the dark-then-returns half of
+// failover (issue #53): host A holds the lease and goes DARK — stops renewing
+// but never releases, the kill -9 / power-cut shape. Its lock.json stays in
+// S3 with the expiry already stamped on it, so:
+//
+// 1. while that expiry is still in the future a peer is refused — dark is
+// not the same as expired, and nobody may jump a live lease;
+// 2. once it lapses, host B claims it — the failover wake;
+// 3. A comes back and tries to pick up where it left off, and MUST lose,
+// loudly, with a *HeldError. A returning dark host silently reclaiming a
+// lease that has moved on is the split-brain this lock exists to prevent.
+//
+// The other half of failover — a fallback host's activator waking the
+// instance from the regional bucket — is the existing activator wake path and
+// is covered by the activator's own tests; nothing about it is new here.
+//
+// Runs on a 15s TTL through the acquireTTL seam; at the production LeaseTTL
+// the same sequence would take 15 minutes.
+func TestDarkHostCannotReclaimAfterFailover(t *testing.T) {
+ bucket, prefix, region := liveConfig(t)
+ ctx := context.Background()
+ const shortTTL = 15 * time.Second
+
+ a, err := acquireTTL(ctx, bucket, prefix, region, shortTTL)
+ if err != nil {
+ t.Fatalf("A initial acquire: %v", err)
+ }
+ acquiredAt := time.Now()
+ // A goes dark. rn.stop() cancels the renewal goroutine's context and
+ // waits for it to exit; it makes no S3 call of any kind, so lock.json
+ // survives untouched and simply runs out its stamped TTL. Without this
+ // line A would renew every shortTTL/3 and step 2 below could never
+ // happen — that is what makes the stop load-bearing.
+ a.rn.stop()
+
+ // Dark is not expired: A's lock is still in force, so B must be refused.
+ // (Both "hosts" run in this one process and therefore share the leaser's
+ // owner string, so the refusal itself — not the name in it — is the
+ // evidence here.)
+ if peer, err := acquireTTL(ctx, bucket, prefix, region, shortTTL); err == nil {
+ elapsed := time.Since(acquiredAt)
+ _ = peer.Release(ctx)
+ if elapsed >= shortTTL {
+ t.Fatalf("second acquire only returned %s after A acquired, past the %s TTL: this environment is too slow for the test's timing margins, which is not the same as a mutual-exclusion failure", elapsed, shortTTL)
+ }
+ t.Fatalf("B acquired after %s while A's un-expired %s lease was still held", elapsed, shortTTL)
+ }
+
+ time.Sleep(shortTTL + 5*time.Second) // let A's abandoned lease lapse
+
+ b, err := acquireTTL(ctx, bucket, prefix, region, shortTTL)
+ if err != nil {
+ t.Fatalf("B acquire after A's lease expired (the failover wake): %v", err)
+ }
+ bHeldFrom := time.Now()
+ defer b.Release(ctx)
+
+ // A returns from the dead and tries to resume.
+ if revenant, err := acquireTTL(ctx, bucket, prefix, region, shortTTL); err == nil {
+ elapsed := time.Since(bHeldFrom)
+ _ = revenant.Release(ctx)
+ if elapsed >= shortTTL {
+ // B's own lease had lapsed by then (B is not renewed here either),
+ // so A winning proves nothing. Say so rather than cry split brain.
+ t.Fatalf("A's re-acquire only returned %s after B took over, past the %s TTL: this environment is too slow for the test's timing margins", elapsed, shortTTL)
+ }
+ t.Fatal("returning dark host reclaimed the lease B now holds — split brain")
+ } else {
+ var held *HeldError
+ if !errors.As(err, &held) {
+ t.Fatalf("returning dark host got %v, want *HeldError (it must lose the race)", err)
+ }
+ // The refusal has to come from B's LIVE lease. An expiry already in
+ // the past would mean the acquire lost for some other reason and the
+ // test would be proving nothing.
+ if !held.ExpiresAt.After(time.Now()) {
+ t.Fatalf("returning host refused by an already-expired lease (expires %s); want B's live lease", held.ExpiresAt.Format(time.RFC3339))
+ }
+ t.Logf("returning dark host lost the race, as required: %v", err)
+ }
+}