From 7390012fd6bbad2c95c2e47ad779b21deedf6c23 Mon Sep 17 00:00:00 2001 From: CMGS Date: Tue, 3 Nov 2026 02:00:45 +0800 Subject: [PATCH 1/6] pool: a settled archive intent keeps a replicated claim's row settleIntent took any own row for the archive's and deleted it. For a hibernated replicated claim that row is the replica's: after one failed or lost Replace, the next archive pass deleted it, refusedReplace read the absent row as an adopter finishing, and a claim nobody adopted or released was destroyed. A restart mid-archive did the same at the first unfence. The row now stays, and one the archive moved goes back to the replica, so it is never absent while the claim is held. Off the claim path. --- docs/cluster.md | 3 +- sandboxd/pool/failover.go | 37 +++++++++++++--- sandboxd/pool/replica_test.go | 83 +++++++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 8 deletions(-) diff --git a/docs/cluster.md b/docs/cluster.md index 0eccef656..0a6d8a1fc 100644 --- a/docs/cluster.md +++ b/docs/cluster.md @@ -217,7 +217,8 @@ replication rounds write it, so the claim path is unchanged. restart that finds the intent and no row keeps the claim while that checkpoint is still in the store, since the row never committed, and drops it once the checkpoint is gone, since only a node that consumed the row deletes - it. + it. A replicated claim's row stays: one the archive moved goes back to the + replica, so the row is never absent while the claim is held. A put whose outcome is unknown leaves the intent in place, and a release, the next archive and the hourly sweep settle it by the same rule before they act on the claim. diff --git a/sandboxd/pool/failover.go b/sandboxd/pool/failover.go index aee2e90fc..31125a6a5 100644 --- a/sandboxd/pool/failover.go +++ b/sandboxd/pool/failover.go @@ -557,7 +557,7 @@ func (m *Manager) fenceArchived(ctx context.Context, claims map[string]*types.Sa sb.ReplicaPending = "" } case sb.ArchivePending != "" || ok && !rec.Consumed: - moved, err := m.settleIntent(ctx, id, sb.ArchivePending, rec, ok) + moved, err := m.settleIntent(ctx, sb, sb.ArchivePending, rec, ok) switch { case err != nil: logger.Warnf(ctx, "archive intent of %s left for the next restart: %v", id, err) @@ -580,20 +580,29 @@ func (m *Manager) fenceArchived(ctx context.Context, claims map[string]*types.Sa return true } -func (m *Manager) settleIntent(ctx context.Context, id, pending string, rec archives.Record, ok bool) (bool, error) { +func (m *Manager) settleIntent(ctx context.Context, sb *types.Sandbox, pending string, rec archives.Record, ok bool) (moved bool, err error) { if ok && rec.Owner == m.nodeID { - err := m.archives.Delete(ctx, id, rec.Epoch) + m.mu.Lock() + replica := sb.ReplicaCk + if replica != "" { + sb.ArchiveEpoch = rec.Epoch + } + m.mu.Unlock() + if replica != "" { + return false, m.restoreReplicaRow(ctx, sb, replica, rec) + } + err = m.archives.Delete(ctx, sb.ID, rec.Epoch) if err == nil { return false, nil } if !errors.Is(err, archives.ErrLost) { return false, err } - if rec, ok, err = m.archives.Get(ctx, id); err != nil { + if rec, ok, err = m.archives.Get(ctx, sb.ID); err != nil { return false, err } if ok && rec.Owner == m.nodeID { - return false, fmt.Errorf("archive row %s moved under its settle", id) + return false, fmt.Errorf("archive row %s moved under its settle", sb.ID) } } switch { @@ -602,13 +611,27 @@ func (m *Manager) settleIntent(ctx context.Context, id, pending string, rec arch case pending == "": return false, nil } - _, err := m.ckpts.ReadMeta(ctx, pending) + _, err = m.ckpts.ReadMeta(ctx, pending) if errors.Is(err, store.ErrNotFound) { return true, nil } return false, err } +func (m *Manager) restoreReplicaRow(ctx context.Context, sb *types.Sandbox, replica string, rec archives.Record) error { + if rec.Checkpoint == replica || rec.Consumed { + return nil + } + epoch, err := m.replaceReplica(ctx, sb, replica) + if err != nil { + return err + } + m.mu.Lock() + sb.ArchiveEpoch = epoch + m.mu.Unlock() + return nil +} + func (m *Manager) resolveIntent(ctx context.Context, sb *types.Sandbox) error { if m.archives == nil { return nil @@ -626,7 +649,7 @@ func (m *Manager) resolveIntent(ctx context.Context, sb *types.Sandbox) error { if err != nil { return err } - moved, err := m.settleIntent(ctx, sb.ID, pending, rec, ok) + moved, err := m.settleIntent(ctx, sb, pending, rec, ok) switch { case err != nil: return err diff --git a/sandboxd/pool/replica_test.go b/sandboxd/pool/replica_test.go index deed5a830..2cb84a74a 100644 --- a/sandboxd/pool/replica_test.go +++ b/sandboxd/pool/replica_test.go @@ -1140,6 +1140,89 @@ func TestReplicaAdoptionWakesTheClaimFromTheLastReplica(t *testing.T) { } } +func TestReplicaArchiveRetriedAfterARegistryFaultKeepsTheClaim(t *testing.T) { + for _, tt := range []struct { + name string + reg func(*failoverCluster) *hookRegistry + }{ + {"the replace failed", func(c *failoverCluster) *hookRegistry { return &hookRegistry{Registry: c.reg, failPut: true} }}, + {"the replace landed and its reply was lost", func(c *failoverCluster) *hookRegistry { + return &hookRegistry{Registry: c.reg, losePutReply: true} + }}, + } { + t.Run(tt.name, func(t *testing.T) { + c := newReplicaCluster(t) + a := c.node(t, "a", t.TempDir(), keepAnHour) + sb := mustClaim(t, a, testKey) + replicateNow(t, a, sb) + if err := a.Hibernate(t.Context(), sb.ID, Cred{Token: sb.Token}); err != nil { + t.Fatalf("hibernate: %v", err) + } + a.archives = tt.reg(c) + if err := a.archive(t.Context(), sb); err == nil { + t.Fatal("archive under the fault succeeded") + } + a.archives = c.reg + if err := a.archive(t.Context(), sb); err != nil { + t.Fatalf("retried archive: %v", err) + } + if _, held := a.byID(sb.ID); !held || sb.ArchiveCk == "" { + t.Fatalf("claim held=%v archive ck %q, want it archived", held, sb.ArchiveCk) + } + rec, ok, err := c.reg.Get(t.Context(), sb.ID) + if err != nil || !ok || rec.Consumed || rec.Checkpoint != sb.ArchiveCk || rec.Epoch != sb.ArchiveEpoch { + t.Errorf("row %+v ok=%v err=%v, want it live on %s at epoch %d", rec, ok, err, sb.ArchiveCk, sb.ArchiveEpoch) + } + }) + } +} + +func TestReplicaRestartMidArchiveKeepsTheClaim(t *testing.T) { + for _, tt := range []struct { + name string + landed bool + }{ + {"the archive row never landed", false}, + {"the archive row landed", true}, + } { + t.Run(tt.name, func(t *testing.T) { + c := newReplicaCluster(t) + dir := t.TempDir() + a := c.node(t, "a", dir, keepAnHour) + sb := mustClaim(t, a, testKey) + vm := lockedVMName(sb) + replicateNow(t, a, sb) + replica := sb.ReplicaCk + if err := a.Hibernate(t.Context(), sb.ID, Cred{Token: sb.Token}); err != nil { + t.Fatalf("hibernate: %v", err) + } + interruptArchive(t, a, sb, tt.landed) + eng := a.eng.(*fakeEngine) + + restarted := c.nodeOn(t, eng, "a", dir, keepAnHour) + if err := restarted.Reconcile(t.Context()); err != nil { + t.Fatalf("reconcile: %v", err) + } + rec, ok, err := c.reg.Get(t.Context(), sb.ID) + if err != nil || !ok || rec.Consumed || rec.Checkpoint != replica { + t.Errorf("row %+v ok=%v err=%v, want it live on the replica %s", rec, ok, err, replica) + } + c.renew(t, "a") + restarted.unfence(t.Context(), time.Now()) + held, kept := restarted.byID(sb.ID) + if !kept || eng.removed(vm) { + t.Fatalf("restart dropped a claim nobody adopted: held=%v VM removed=%v", kept, eng.removed(vm)) + } + if err = restarted.archive(t.Context(), held); err != nil { + t.Fatalf("archive after the restart: %v", err) + } + if rec, ok, err = c.reg.Get(t.Context(), sb.ID); err != nil || !ok || rec.Checkpoint != held.ArchiveCk { + t.Errorf("row %+v ok=%v err=%v, want it on %s", rec, ok, err, held.ArchiveCk) + } + }) + } +} + func restartWithoutVM(t *testing.T, c *failoverCluster, a *Manager, sb *types.Sandbox, deleteAfter int) *Manager { t.Helper() if err := a.FlushClaims(); err != nil { From f362713a3d7c1378998d18af945b6c0e9bb16760 Mon Sep 17 00:00:00 2001 From: CMGS Date: Tue, 3 Nov 2026 02:05:26 +0800 Subject: [PATCH 2/6] review: settleIntent takes the replica arm before its own row's delete --- sandboxd/pool/failover.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/sandboxd/pool/failover.go b/sandboxd/pool/failover.go index 31125a6a5..9626ef549 100644 --- a/sandboxd/pool/failover.go +++ b/sandboxd/pool/failover.go @@ -581,16 +581,13 @@ func (m *Manager) fenceArchived(ctx context.Context, claims map[string]*types.Sa } func (m *Manager) settleIntent(ctx context.Context, sb *types.Sandbox, pending string, rec archives.Record, ok bool) (moved bool, err error) { + m.mu.Lock() + replica := sb.ReplicaCk + m.mu.Unlock() + if ok && rec.Owner == m.nodeID && replica != "" { + return false, m.restoreReplicaRow(ctx, sb, replica, rec) + } if ok && rec.Owner == m.nodeID { - m.mu.Lock() - replica := sb.ReplicaCk - if replica != "" { - sb.ArchiveEpoch = rec.Epoch - } - m.mu.Unlock() - if replica != "" { - return false, m.restoreReplicaRow(ctx, sb, replica, rec) - } err = m.archives.Delete(ctx, sb.ID, rec.Epoch) if err == nil { return false, nil @@ -619,6 +616,9 @@ func (m *Manager) settleIntent(ctx context.Context, sb *types.Sandbox, pending s } func (m *Manager) restoreReplicaRow(ctx context.Context, sb *types.Sandbox, replica string, rec archives.Record) error { + m.mu.Lock() + sb.ArchiveEpoch = rec.Epoch + m.mu.Unlock() if rec.Checkpoint == replica || rec.Consumed { return nil } From 6dea44445a6c989897dbcf86f616f4d140432ae7 Mon Sep 17 00:00:00 2001 From: CMGS Date: Tue, 3 Nov 2026 08:23:18 +0800 Subject: [PATCH 3/6] pool: an archive whose claims write fails keeps a replicated claim's row The rollback deleted the row it had just moved from the replica to the archive checkpoint and zeroed the epoch, while the journal on disk still named the replica at the old epoch. After a restart the first unfence read the missing row as a finished adoption and removed the claim, its VM and its snapshot. A replicated claim now keeps the row's epoch and its archive intent, so the next settle moves the row back to the replica. --- sandboxd/pool/archive.go | 10 ++++++++-- sandboxd/pool/replica_test.go | 37 +++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/sandboxd/pool/archive.go b/sandboxd/pool/archive.go index 99eaf39a2..b185cba1a 100644 --- a/sandboxd/pool/archive.go +++ b/sandboxd/pool/archive.go @@ -153,8 +153,12 @@ func (m *Manager) archive(ctx context.Context, sb *types.Sandbox) error { m.mu.Lock() var rb claimSnapshot // A Release that landed meanwhile already deleted the claim; re-setting it would resurrect it - if m.claimed[sb.ID] == sb { + live := m.claimed[sb.ID] == sb + if live { sb.ArchiveCk, sb.ArchiveEpoch, sb.ArchivePending, sb.ReplicaCk = "", 0, prevPending, replica + if replica != "" { + sb.ArchiveEpoch = epoch + } sb.HibernateSnap, sb.VsockSocket, sb.VMName = snap, sock, vmName sb.Deadline = prevDeadline rb = m.store.set(sb) @@ -162,7 +166,9 @@ func (m *Manager) archive(ctx context.Context, sb *types.Sandbox) error { m.mu.Unlock() sb.Transition.Unlock() m.store.recommit(ctx, rb) - m.abandonArchive(ctx, sb, epoch, ck.ID) + if !live || replica == "" { + m.abandonArchive(ctx, sb, epoch, ck.ID) + } return fmt.Errorf("archive %s: persist claims: %w", sb.ID, saveErr) } m.counters.archives.Add(1) diff --git a/sandboxd/pool/replica_test.go b/sandboxd/pool/replica_test.go index 2cb84a74a..d73b373b7 100644 --- a/sandboxd/pool/replica_test.go +++ b/sandboxd/pool/replica_test.go @@ -4,6 +4,7 @@ import ( "encoding/json/v2" "errors" "net" + "path/filepath" "testing" "testing/synctest" "time" @@ -1223,6 +1224,42 @@ func TestReplicaRestartMidArchiveKeepsTheClaim(t *testing.T) { } } +func TestReplicaArchiveWhoseJournalWriteFailsKeepsTheClaimAcrossARestart(t *testing.T) { + c := newReplicaCluster(t) + dir := t.TempDir() + a := c.node(t, "a", dir, keepAnHour) + sb := mustClaim(t, a, testKey) + vm := lockedVMName(sb) + replicateNow(t, a, sb) + replica := sb.ReplicaCk + if err := a.Hibernate(t.Context(), sb.ID, Cred{Token: sb.Token}); err != nil { + t.Fatalf("hibernate: %v", err) + } + broken := filepath.Join(t.TempDir(), "gone", "claims.json") + a.archives = &hookRegistry{Registry: c.reg, beforePut: func() { a.store.path = broken }} + if err := a.archive(t.Context(), sb); err == nil { + t.Fatal("archive with an unwritable journal succeeded") + } + if rec, ok, err := c.reg.Get(t.Context(), sb.ID); err != nil || !ok || rec.Consumed || rec.Owner != "a" { + t.Errorf("row %+v ok=%v err=%v, want this node's live row", rec, ok, err) + } + eng := a.eng.(*fakeEngine) + + restarted := c.nodeOn(t, eng, "a", dir, keepAnHour) + if err := restarted.Reconcile(t.Context()); err != nil { + t.Fatalf("reconcile: %v", err) + } + rec, ok, err := c.reg.Get(t.Context(), sb.ID) + if err != nil || !ok || rec.Consumed || rec.Checkpoint != replica { + t.Errorf("row %+v ok=%v err=%v, want it live on the replica %s", rec, ok, err, replica) + } + c.renew(t, "a") + restarted.unfence(t.Context(), time.Now()) + if _, kept := restarted.byID(sb.ID); !kept || eng.removed(vm) { + t.Fatalf("restart dropped a claim nobody adopted: held=%v VM removed=%v", kept, eng.removed(vm)) + } +} + func restartWithoutVM(t *testing.T, c *failoverCluster, a *Manager, sb *types.Sandbox, deleteAfter int) *Manager { t.Helper() if err := a.FlushClaims(); err != nil { From a0ce37fc8144a25cb0725e72b058fd9cd2c6b547 Mon Sep 17 00:00:00 2001 From: CMGS Date: Tue, 3 Nov 2026 08:23:44 +0800 Subject: [PATCH 4/6] pool: a fenced node, or one that just lifted its fence, does not adopt After a registry outage longer than the lease every node is fenced and every lease expired. A client polling a node made it adopt while it was fenced, or right after its own lift, before the healthy owner had renewed; the owner then confirmed, saw a foreign row and removed its running VM. Adoption now waits for the fence and for one renewal interval plus one registry query after the lift. A claim this node already holds keeps its path. --- docs/cluster.md | 5 ++++- sandboxd/pool/failover.go | 3 +++ sandboxd/pool/lease.go | 8 ++++++- sandboxd/pool/lease_test.go | 42 +++++++++++++++++++++++++++++++++++ sandboxd/pool/pool.go | 1 + sandboxd/pool/replica_test.go | 1 + 6 files changed, 58 insertions(+), 2 deletions(-) diff --git a/docs/cluster.md b/docs/cluster.md index 0a6d8a1fc..1c03fabfc 100644 --- a/docs/cluster.md +++ b/docs/cluster.md @@ -187,7 +187,10 @@ replication rounds write it, so the claim path is unchanged. answers 404. A tombstone is never adopted. The next exec or file call wakes the claim from the store as usual. An owner with a live lease answers for itself, and a node that tries to take its row is refused with a 404. - A draining node never adopts. Adoption checks no tenant `max_claims` or node + A draining node never adopts, nor does a fenced node or one that lifted its + fence less than 8 seconds ago (one renewal interval plus one registry query), + so after a registry outage every healthy owner renews before a peer can take + its rows. Adoption checks no tenant `max_claims` or node `max_claims`: the claim already exists, and a refusal could strand it. - **Fencing.** Take, consume and delete are compare-and-sets on the row's epoch, and so is the write of a replica round and of the archive of a claim diff --git a/sandboxd/pool/failover.go b/sandboxd/pool/failover.go index 9626ef549..63a1f348c 100644 --- a/sandboxd/pool/failover.go +++ b/sandboxd/pool/failover.go @@ -32,12 +32,15 @@ func (m *Manager) adoptArchived(ctx context.Context, id, token string) (bool, er m.mu.Lock() draining := m.draining _, held := m.authed(id, token) + fenced := m.adoptFenced() m.mu.Unlock() switch { case draining: return false, nil case held: return true, nil + case fenced: + return false, nil } ctx = context.WithoutCancel(ctx) rec, ok, err := m.archives.Get(ctx, id) diff --git a/sandboxd/pool/lease.go b/sandboxd/pool/lease.go index 380d2b624..8c628fe28 100644 --- a/sandboxd/pool/lease.go +++ b/sandboxd/pool/lease.go @@ -20,6 +20,7 @@ import ( type fenceState struct { ch chan struct{} fenced bool + lifted time.Time } // CloseOnFence closes c when fenced closes; stop ends the watch. @@ -72,6 +73,11 @@ func (m *Manager) isFenced() bool { return m.fence.Load().fenced } +func (m *Manager) adoptFenced() bool { + st := m.fence.Load() + return st.fenced || time.Since(st.lifted) < adoptGrace +} + func (m *Manager) trip(ctx context.Context) { for { old := m.fence.Load() @@ -129,7 +135,7 @@ func (m *Manager) unfence(ctx context.Context, start time.Time) { // lift replaces the fenced state this renewal saw, and trips again when the window closed before the lift landed. func (m *Manager) lift(ctx context.Context, fenced *fenceState, start time.Time) { - if !m.fence.CompareAndSwap(fenced, &fenceState{ch: make(chan struct{})}) { + if !m.fence.CompareAndSwap(fenced, &fenceState{ch: make(chan struct{}), lifted: time.Now()}) { return } if time.Since(start) > leaseTTL-leaseMargin { diff --git a/sandboxd/pool/lease_test.go b/sandboxd/pool/lease_test.go index 022d3b2ef..e4d75710f 100644 --- a/sandboxd/pool/lease_test.go +++ b/sandboxd/pool/lease_test.go @@ -302,6 +302,48 @@ func TestLeaseAdoptionDuringUnfenceDropsTheClaimWithoutServingIt(t *testing.T) { }) } +func TestLeaseANodeLiftedAfterAnOutageAdoptsOnlyOnceEveryOwnerCouldRenew(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + c := newReplicaCluster(t) + a, b := c.node(t, "a", t.TempDir(), keepAnHour), c.node(t, "b", t.TempDir(), keepAnHour) + sb := mustClaim(t, a, testKey) + vm := lockedVMName(sb) + replicateNow(t, a, sb) + a.tripFence() + b.tripFence() + c.expire("a") + c.expire("b") + + if adopted, err := b.AdoptArchived(t.Context(), sb.ID, sb.Token); err != nil || adopted { + t.Fatalf("adoption by a fenced node: %v %v, want refused", adopted, err) + } + c.renew(t, "b") + b.unfence(t.Context(), time.Now()) + if b.isFenced() { + t.Fatal("the node stayed fenced after its renewal") + } + if adopted, err := b.AdoptArchived(t.Context(), sb.ID, sb.Token); err != nil || adopted { + t.Fatalf("adoption right after the lift: %v %v, want refused", adopted, err) + } + c.renew(t, "a") + a.unfence(t.Context(), time.Now()) + synctest.Wait() + if _, held := a.byID(sb.ID); !held || a.eng.(*fakeEngine).removed(vm) { + t.Fatalf("the healthy owner lost its claim: held=%v VM removed=%v", held, a.eng.(*fakeEngine).removed(vm)) + } + + c.expire("a") + time.Sleep(adoptGrace - time.Millisecond) + if adopted, err := b.AdoptArchived(t.Context(), sb.ID, sb.Token); err != nil || adopted { + t.Fatalf("adoption inside the grace: %v %v, want refused", adopted, err) + } + time.Sleep(time.Millisecond) + if adopted, err := b.AdoptArchived(t.Context(), sb.ID, sb.Token); err != nil || !adopted { + t.Errorf("adoption once the grace passed: %v %v, want adopted", adopted, err) + } + }) +} + func TestLeaseFencedNodeDoesNotAnswerAsOwnerOfReplicatedClaims(t *testing.T) { c := newReplicaCluster(t) a := c.node(t, "a", t.TempDir(), keepAnHour) diff --git a/sandboxd/pool/pool.go b/sandboxd/pool/pool.go index 783a7bb65..4953e52da 100644 --- a/sandboxd/pool/pool.go +++ b/sandboxd/pool/pool.go @@ -57,6 +57,7 @@ const ( leaseTTL = 15 * time.Second leaseRenew = 5 * time.Second leaseMargin = 3 * time.Second + adoptGrace = leaseRenew + leaseMargin // One failed boot is ordinary; only an unbroken run predicts the next failure refillFailStreak = 8 refillBackoffBase = 250 * time.Millisecond diff --git a/sandboxd/pool/replica_test.go b/sandboxd/pool/replica_test.go index d73b373b7..91e5773e6 100644 --- a/sandboxd/pool/replica_test.go +++ b/sandboxd/pool/replica_test.go @@ -860,6 +860,7 @@ func TestAReplicaRowWithNoClaimLeftStaysAdoptable(t *testing.T) { sb := mustClaim(t, a, testKey) replicateNow(t, a, sb) restarted := restartWithoutVM(t, c, a, sb, keepAnHour) + restarted.liftFence() restarted.sweepOrphanArchives(t.Context()) if adopted, err := restarted.AdoptArchived(t.Context(), sb.ID, sb.Token); err != nil || !adopted { t.Errorf("adopt inside retention: %t %v", adopted, err) From c52674d80fac7c8e74303478180de65f9a7b995f Mon Sep 17 00:00:00 2001 From: CMGS Date: Tue, 3 Nov 2026 08:25:49 +0800 Subject: [PATCH 5/6] archives: take every live row a node owns in one call The unfence pass confirms each replicated claim with its own Take, so its length grows with the claim count times the registry round trip. TakeOwned moves the owner's live rows at their expected epochs in one statement, with Take's compare-and-set on the epoch and no lease arm, since the owner is the caller. --- sandboxd/archives/archives.go | 2 + .../archives/archivestest/archivestest.go | 46 +++++++++++++++++++ sandboxd/archives/pg/pg.go | 28 +++++++++++ sandboxd/archives/pg/pg_test.go | 3 ++ 4 files changed, 79 insertions(+) diff --git a/sandboxd/archives/archives.go b/sandboxd/archives/archives.go index 23add9bc0..606208a69 100644 --- a/sandboxd/archives/archives.go +++ b/sandboxd/archives/archives.go @@ -37,6 +37,8 @@ type Registry interface { Replace(ctx context.Context, rec Record, expect int64) (int64, error) // Take moves the live row at epoch expect to owner at a new epoch and returns it; another node's row moves only once its owner's lease expired. Take(ctx context.Context, id string, expect int64, owner string) (int64, error) + // TakeOwned moves each live row owner holds at its epoch in expect to a new epoch in one call and returns the new epochs of the rows it moved. + TakeOwned(ctx context.Context, owner string, expect map[string]int64) (map[string]int64, error) // Consume turns the live row at epoch expect into a tombstone at a new epoch and returns it. Consume(ctx context.Context, id string, expect int64) (int64, error) // Delete removes the row only while it is at epoch. diff --git a/sandboxd/archives/archivestest/archivestest.go b/sandboxd/archives/archivestest/archivestest.go index 42b33463d..8db9782e6 100644 --- a/sandboxd/archives/archivestest/archivestest.go +++ b/sandboxd/archives/archivestest/archivestest.go @@ -83,6 +83,20 @@ func (m *Mem) Take(_ context.Context, id string, expect int64, owner string) (in }) } +func (m *Mem) TakeOwned(_ context.Context, owner string, expect map[string]int64) (map[string]int64, error) { + taken := map[string]int64{} + for id, epoch := range expect { + next, err := m.advance(id, epoch, func(rec *archives.Record) bool { return rec.Owner == owner }) + switch { + case err == nil: + taken[id] = next + case !errors.Is(err, archives.ErrLost): + return nil, err + } + } + return taken, nil +} + func (m *Mem) Consume(_ context.Context, id string, expect int64) (int64, error) { return m.advance(id, expect, func(rec *archives.Record) bool { rec.Consumed = true @@ -206,6 +220,7 @@ func Run(t *testing.T, open Opener) { t.Run("put fences other owners", func(t *testing.T) { testPutFences(t, open(t)) }) t.Run("take is a compare-and-set", func(t *testing.T) { testTake(t, open(t)) }) t.Run("one of concurrent takes wins", func(t *testing.T) { testTakeRace(t, open(t)) }) + t.Run("take owned moves only the owner's live rows at their epochs", func(t *testing.T) { testTakeOwned(t, open(t)) }) t.Run("consume leaves a tombstone", func(t *testing.T) { testConsume(t, open(t)) }) t.Run("delete only at its epoch", func(t *testing.T) { testDelete(t, open(t)) }) t.Run("epochs are never reused", func(t *testing.T) { testEpochsNeverRepeat(t, open(t)) }) @@ -306,6 +321,37 @@ func testTakeRace(t *testing.T, r archives.Registry) { } } +func testTakeOwned(t *testing.T, r archives.Registry) { + ctx := t.Context() + mine := mustPut(t, r, record("sb_mine", "a", time.Time{})) + stale := mustPut(t, r, record("sb_moved", "a", time.Time{})) + moved := mustPut(t, r, record("sb_moved", "a", time.Time{})) + other := mustPut(t, r, record("sb_other", "b", time.Time{})) + spent, err := r.Consume(ctx, "sb_spent", mustPut(t, r, record("sb_spent", "a", time.Time{}))) + if err != nil { + t.Fatalf("consume: %v", err) + } + taken, err := r.TakeOwned(ctx, "a", map[string]int64{"sb_mine": mine, "sb_moved": stale, "sb_other": other, "sb_spent": spent, "sb_none": 1}) + if err != nil || len(taken) != 1 || taken["sb_mine"] <= spent { + t.Fatalf("take owned: %v %v, want only sb_mine at a new epoch", taken, err) + } + put := record("sb_mine", "a", time.Time{}) + if got, _, _ := r.Get(ctx, "sb_mine"); got.Owner != "a" || got.Epoch != taken["sb_mine"] || got.Consumed || got.Checkpoint != put.Checkpoint || string(got.Claim) != string(put.Claim) { + t.Errorf("row after take owned: %+v, want a@%d with the row", got, taken["sb_mine"]) + } + for id, want := range map[string]int64{"sb_moved": moved, "sb_other": other, "sb_spent": spent} { + if got, _, _ := r.Get(ctx, id); got.Epoch != want { + t.Errorf("%s moved to epoch %d, want it left at %d", id, got.Epoch, want) + } + } + if _, ok, _ := r.Get(ctx, "sb_none"); ok { + t.Error("take owned inserted a row") + } + if taken, err = r.TakeOwned(ctx, "a", nil); err != nil || len(taken) != 0 { + t.Errorf("take owned of no ids: %v %v, want none", taken, err) + } +} + func testConsume(t *testing.T, r archives.Registry) { ctx := t.Context() first := mustPut(t, r, record("sb_1", "a", time.Time{})) diff --git a/sandboxd/archives/pg/pg.go b/sandboxd/archives/pg/pg.go index 15cf03fb7..6830bf3c4 100644 --- a/sandboxd/archives/pg/pg.go +++ b/sandboxd/archives/pg/pg.go @@ -78,6 +78,34 @@ func (r *Registry) Take(ctx context.Context, id string, expect int64, owner stri RETURNING epoch`, r.cell, id, expect, owner) } +func (r *Registry) TakeOwned(ctx context.Context, owner string, expect map[string]int64) (map[string]int64, error) { + ids, epochs := make([]string, 0, len(expect)), make([]int64, 0, len(expect)) + for id, epoch := range expect { + ids, epochs = append(ids, id), append(epochs, epoch) + } + qctx, cancel := context.WithTimeout(ctx, metastore.QueryTimeout) + defer cancel() + rows, err := r.pool.Query(qctx, `UPDATE sandboxd_archives a SET epoch = nextval('sandboxd_archives_epoch'), updated_at = now() + FROM unnest($2::text[], $3::bigint[]) AS t(id, epoch) + WHERE a.cell = $1 AND a.id = t.id AND a.epoch = t.epoch AND a.owner = $4 AND NOT a.consumed + RETURNING a.id, a.epoch`, r.cell, ids, epochs, owner) + if err != nil { + return nil, unavailable(err) + } + taken := make(map[string]int64, len(expect)) + var ( + id string + epoch int64 + ) + if _, err = pgx.ForEachRow(rows, []any{&id, &epoch}, func() error { + taken[id] = epoch + return nil + }); err != nil { + return nil, unavailable(err) + } + return taken, nil +} + func (r *Registry) Consume(ctx context.Context, id string, expect int64) (int64, error) { return r.advance(ctx, `UPDATE sandboxd_archives SET consumed = true, epoch = nextval('sandboxd_archives_epoch'), updated_at = now() WHERE cell = $1 AND id = $2 AND epoch = $3 AND NOT consumed diff --git a/sandboxd/archives/pg/pg_test.go b/sandboxd/archives/pg/pg_test.go index b3fe52721..b9216e48c 100644 --- a/sandboxd/archives/pg/pg_test.go +++ b/sandboxd/archives/pg/pg_test.go @@ -30,6 +30,9 @@ func TestCellsKeepTheirOwnRows(t *testing.T) { if _, err = c2.Take(ctx, "sb_1", epoch, "n2"); !errors.Is(err, archives.ErrLost) { t.Errorf("c2 takes c1's row: %v, want ErrLost", err) } + if taken, takeErr := c2.TakeOwned(ctx, "n1", map[string]int64{"sb_1": epoch}); takeErr != nil || len(taken) != 0 { + t.Errorf("c2 takes c1's row as its owner: %v %v", taken, takeErr) + } if _, err = c2.Consume(ctx, "sb_1", epoch); !errors.Is(err, archives.ErrLost) { t.Errorf("c2 consumes c1's row: %v, want ErrLost", err) } From 243a0ea443d7194a3f00d5bea20bc167af795430 Mon Sep 17 00:00:00 2001 From: CMGS Date: Tue, 3 Nov 2026 08:29:39 +0800 Subject: [PATCH 6/6] pool: unfence takes its own rows in one registry call and one journal write The confirmation ran one Take and one whole-file claims commit per replicated claim, serially. Past the 12 s window the lift re-tripped and the next renewal started over, so about 1200 claims at a 10 ms registry round trip never lifted. The rows this node holds now move in one TakeOwned and their epochs land in one commit; absent, foreign, consumed or moved rows keep the per-row path. --- docs/cluster.md | 8 +++-- sandboxd/pool/lease.go | 38 ++++++++++++++++++++++ sandboxd/pool/lease_test.go | 63 +++++++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 3 deletions(-) diff --git a/docs/cluster.md b/docs/cluster.md index 1c03fabfc..fafb124e5 100644 --- a/docs/cluster.md +++ b/docs/cluster.md @@ -363,9 +363,11 @@ checkpoint store and keeps each claim's registry row on the latest snapshot: the node, and the node destroys nothing. Claims of pools that do not replicate stay served. A node starts fenced and lifts the fence after its first renewal that is followed by a confirmation. The confirmation runs beside the renew loop, which keeps - renewing: for each replicated claim the node takes the row back for itself in - one compare-and-set, which the row lock serializes against adopters, and - retries once on a stale epoch. A claim whose row another node took, or took + renewing: the node takes back every live row it holds in one statement, a + compare-and-set on each row's epoch that the row locks serialize against + adopters, and records the new epochs in one journal write; a claim whose row + that statement did not move takes it in its own compare-and-set and retries + once on a stale epoch. A claim whose row another node took, or took and finished, is forgotten at once and its VM removed in the background; a claim whose row is still this node's but keeps refusing the write leaves the node fenced and is retried at the next renewal. The lift replaces the fenced diff --git a/sandboxd/pool/lease.go b/sandboxd/pool/lease.go index 8c628fe28..5f86a2b6d 100644 --- a/sandboxd/pool/lease.go +++ b/sandboxd/pool/lease.go @@ -113,7 +113,15 @@ func (m *Manager) unfence(ctx context.Context, start time.Time) { return } } + taken, err := m.takeOwnRows(ctx, held, rows) + if err != nil { + logger.Warnf(ctx, "replicated claims stay fenced: %v", err) + return + } for id, sb := range held { + if _, ok := taken[id]; ok { + continue + } keep, err := m.confirmOwnership(ctx, sb, rows[id]) if err != nil { logger.Warnf(ctx, "replicated claims stay fenced: %v", err) @@ -146,6 +154,36 @@ func (m *Manager) lift(ctx context.Context, fenced *fenceState, start time.Time) log.WithFunc("pool.unfence").Info(ctx, "node lease renewed; replicated claims are served again") } +func (m *Manager) takeOwnRows(ctx context.Context, held map[string]*types.Sandbox, rows map[string]archives.Record) (map[string]int64, error) { + expect := make(map[string]int64, len(rows)) + for id, rec := range rows { + if rec.Owner == m.nodeID && !rec.Consumed { + expect[id] = rec.Epoch + } + } + if len(expect) == 0 { + return nil, nil + } + taken, err := m.archives.TakeOwned(ctx, m.nodeID, expect) + if err != nil { + return nil, err + } + live := make([]*types.Sandbox, 0, len(taken)) + m.mu.Lock() + for id, epoch := range taken { + if sb := held[id]; m.claimed[id] == sb { + sb.ArchiveEpoch = max(sb.ArchiveEpoch, epoch) + live = append(live, sb) + } + } + js := m.store.set(live...) + m.mu.Unlock() + if err = m.store.commit(js); err != nil { + m.store.recommit(ctx, js) + } + return taken, nil +} + // confirmOwnership takes sb's row for this node in one compare-and-set; false means another node holds the claim. func (m *Manager) confirmOwnership(ctx context.Context, sb *types.Sandbox, rec archives.Record) (bool, error) { for attempt := range 3 { diff --git a/sandboxd/pool/lease_test.go b/sandboxd/pool/lease_test.go index e4d75710f..8615bde57 100644 --- a/sandboxd/pool/lease_test.go +++ b/sandboxd/pool/lease_test.go @@ -3,6 +3,7 @@ package pool import ( "context" "errors" + "fmt" "net" "os" "sync/atomic" @@ -492,6 +493,38 @@ func TestLeaseAdoptionWaitsOutTheFenceAndTheOldOwnerDropsItsCopy(t *testing.T) { } } +func TestLeaseUnfenceOfManyClaimsLiftsInsideTheWindowOverASlowRegistry(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + c := newReplicaCluster(t) + a := c.node(t, "a", t.TempDir(), keepAnHour) + sbs := make([]*types.Sandbox, 200) + for i := range sbs { + sb := &types.Sandbox{ID: fmt.Sprintf("sb_%016x", i), Token: fmt.Sprintf("%032x", i), Key: testKey, Tenant: "t", ReplicaCk: fmt.Sprintf("ck_%016x", i)} + epoch, err := c.reg.Put(t.Context(), archives.Record{ID: sb.ID, Owner: "a", TokenSHA256: "x", Checkpoint: sb.ReplicaCk, Claim: []byte(`{}`)}) + if err != nil { + t.Fatalf("put: %v", err) + } + sb.ArchiveEpoch = epoch + a.mu.Lock() + a.addClaimLocked(sb) + a.mu.Unlock() + sbs[i] = sb + } + a.archives = &slowRegistry{Registry: c.reg, rtt: 100 * time.Millisecond} + a.tripFence() + + a.unfence(t.Context(), time.Now()) + if a.isFenced() { + t.Fatalf("%d claims at a 100 ms round trip outlasted the %s window", len(sbs), leaseTTL-leaseMargin) + } + for _, sb := range sbs { + if rec, _, _ := c.reg.Get(t.Context(), sb.ID); rec.Owner != "a" || rec.Epoch != a.epochOf(sb) { + t.Fatalf("row %s at %s@%d, claim epoch %d, want this node's row at the claim's epoch", sb.ID, rec.Owner, rec.Epoch, a.epochOf(sb)) + } + } + }) +} + type lostTakeRegistry struct { archives.Registry } @@ -500,6 +533,25 @@ func (r *lostTakeRegistry) Take(context.Context, string, int64, string) (int64, return 0, archives.ErrLost } +func (r *lostTakeRegistry) TakeOwned(context.Context, string, map[string]int64) (map[string]int64, error) { + return nil, nil +} + +type slowRegistry struct { + archives.Registry + rtt time.Duration +} + +func (r *slowRegistry) Take(ctx context.Context, id string, expect int64, owner string) (int64, error) { + time.Sleep(r.rtt) + return r.Registry.Take(ctx, id, expect, owner) +} + +func (r *slowRegistry) TakeOwned(ctx context.Context, owner string, expect map[string]int64) (map[string]int64, error) { + time.Sleep(r.rtt) + return r.Registry.TakeOwned(ctx, owner, expect) +} + type cutRegistry struct { archives.Registry cut atomic.Bool @@ -521,6 +573,17 @@ func (r *cutRegistry) Take(ctx context.Context, id string, expect int64, owner s return r.Registry.Take(ctx, id, expect, owner) } +func (r *cutRegistry) TakeOwned(ctx context.Context, owner string, expect map[string]int64) (map[string]int64, error) { + if hook := r.beforeTake; hook != nil { + r.beforeTake = nil + hook() + } + if r.takeErr != nil { + return nil, nil + } + return r.Registry.TakeOwned(ctx, owner, expect) +} + func (r *cutRegistry) Renew(ctx context.Context, node string, ttl time.Duration) error { if r.cut.Load() { return archives.ErrUnavailable