From 5d21186f22a8948a5d87af7269b3abed9eeed813 Mon Sep 17 00:00:00 2001
From: Kristoffer Dalby <kristoffer@tailscale.com>
Date: Thu, 22 Oct 2026 00:02:17 +0000
Subject: [PATCH] state: release persistMu before the NodeStore delete

Holding it serialised concurrent deletes into one NodeStore batch and
peer map build each, capping deletion at one batch tick per node.
---
 hscontrol/state/persist_test.go | 53 +++++++++++++++++++++++++++++++++
 hscontrol/state/state.go        |  9 +++---
 2 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/hscontrol/state/persist_test.go b/hscontrol/state/persist_test.go
index f0a7fbb41..ad781a00b 100644
--- a/hscontrol/state/persist_test.go
+++ b/hscontrol/state/persist_test.go
@@ -628,6 +628,59 @@ func TestDeleteNodeReturnsRemovalOnPolicyFailure(t *testing.T) {
 // write during pre-auth-key re-registration does not leave the NodeStore
 // holding a node key that was never persisted: a restart would reload the old
 // row and the client's current key would no longer resolve, locking it out.
+// TestConcurrentDeleteNodesShareNodeStoreBatch deletes two nodes at once with
+// a NodeStore that applies a batch only when it holds two writes. Holding
+// persistMu across the NodeStore write gave every delete a batch, and so a
+// peer map build, of its own; under churn that serialised deletion behind one
+// rebuild each.
+func TestConcurrentDeleteNodesShareNodeStoreBatch(t *testing.T) {
+	cfg := persistTestConfig(t.TempDir() + "/headscale.db")
+	cfg.Tuning.NodeStoreBatchSize = 2
+	cfg.Tuning.NodeStoreBatchTimeout = time.Hour
+
+	database, err := db.NewHeadscaleDatabase(cfg)
+	require.NoError(t, err)
+
+	user := database.CreateUserForTest("delete-batch-user")
+	nodes := database.CreateNodesForTest(user, 2, "delete-batch-node")
+
+	require.NoError(t, database.Close())
+
+	s, err := NewState(cfg)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = s.Close() })
+
+	var wg sync.WaitGroup
+
+	for _, n := range nodes {
+		node, ok := s.GetNodeByID(n.ID)
+		require.True(t, ok)
+
+		wg.Go(func() {
+			_, err := s.DeleteNode(node)
+			assert.NoError(t, err)
+		})
+	}
+
+	done := make(chan struct{})
+
+	go func() {
+		wg.Wait()
+		close(done)
+	}()
+
+	select {
+	case <-done:
+	case <-time.After(10 * time.Second):
+		t.Fatal("concurrent deletes must be able to share a NodeStore batch")
+	}
+
+	for _, n := range nodes {
+		_, ok := s.GetNodeByID(n.ID)
+		assert.False(t, ok, "deleted node must be gone from the NodeStore")
+	}
+}
+
 func TestPreAuthKeyReauthRevertsNodeStoreOnDBFailure(t *testing.T) {
 	dbPath := t.TempDir() + "/headscale.db"
 	cfg := persistTestConfig(dbPath)
diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go
index d465375f7..7536b778b 100644
--- a/hscontrol/state/state.go
+++ b/hscontrol/state/state.go
@@ -627,17 +627,18 @@ func (s *State) DeleteNode(node types.NodeView) ([]change.Change, error) {
 	s.persistMu.Lock()
 
 	err := s.db.DeleteNode(node.AsStruct())
-	if err != nil {
-		s.persistMu.Unlock()
+	s.persistMu.Unlock()
 
+	if err != nil {
 		return nil, err
 	}
 
 	// The database is the durable source of truth. Only remove the in-memory
 	// node after its row is gone so a failed database write cannot make a live
-	// node look deleted until the next restart.
+	// node look deleted until the next restart. persistMu stays out of this
+	// write: a persist that still finds the node updates no row, and holding
+	// it would give every concurrent delete a NodeStore batch of its own.
 	s.nodeStore.DeleteNode(node.ID())
-	s.persistMu.Unlock()
 
 	s.ipAlloc.FreeIPs(node.IPs())