state: return the replaced node from NodeStore updates

UpdateNodeDiff hands back the pre-update clone the writer already makes.

Updates #3531
This commit is contained in:
Kristoffer Dalby
2026-10-08 09:54:04 +00:00
committed by Kristoffer Dalby
parent aaf4ccd8c9
commit 9c764e5a99
3 changed files with 61 additions and 1 deletions
+1 -1
View File
@@ -1822,7 +1822,7 @@ func waitParkedOnWriteQueue(t *testing.T, fn string) {
stacks := string(buf[:runtime.Stack(buf, true)])
for g := range strings.SplitSeq(stacks, "\n\n") {
if strings.Contains(g, "[select") &&
strings.Contains(g, "(*NodeStore).UpdateNodes(") &&
strings.Contains(g, "(*NodeStore).updateNodes(") &&
strings.Contains(g, "."+fn+"(") {
return true
}
+38
View File
@@ -198,6 +198,9 @@ type work struct {
// prober applying multiple probe results at once) cannot have a
// partial snapshot published between the updates.
multiUpdates map[types.NodeID]UpdateNodeFunc
// pre, when non-nil, receives each updated node as it was before its
// update function ran.
pre map[types.NodeID]types.NodeView
}
// updateChanges reports whether an in-place update moved a peer-visibility
@@ -274,6 +277,27 @@ func (s *NodeStore) UpdateNode(nodeID types.NodeID, updateFn UpdateNodeFunc) (ty
return s.GetNode(nodeID)
}
// UpdateNodeDiff is [NodeStore.UpdateNode] that also returns, first, the node
// as it was before updateFn ran, so callers can tell what the update changed
// without cloning the node themselves. That view is invalid when the node does
// not exist.
func (s *NodeStore) UpdateNodeDiff(
nodeID types.NodeID,
updateFn UpdateNodeFunc,
) (types.NodeView, types.NodeView, bool) {
timer := prometheus.NewTimer(nodeStoreOperationDuration.WithLabelValues("update"))
defer timer.ObserveDuration()
pre := make(map[types.NodeID]types.NodeView, 1)
s.updateNodes(map[types.NodeID]UpdateNodeFunc{nodeID: updateFn}, pre)
nodeStoreOperations.WithLabelValues("update").Inc()
after, ok := s.GetNode(nodeID)
return pre[nodeID], after, ok
}
// UpdateNodes applies per-node update functions in a single atomic
// batch. The election that recomputes primary routes runs once, after
// every update has landed, so callers cannot observe an intermediate
@@ -282,6 +306,15 @@ func (s *NodeStore) UpdateNode(nodeID types.NodeID, updateFn UpdateNodeFunc) (ty
// would change the election outcome — e.g. the HA prober applying
// concurrent probe-timeout results.
func (s *NodeStore) UpdateNodes(updates map[types.NodeID]UpdateNodeFunc) {
s.updateNodes(updates, nil)
}
// updateNodes queues updates as one batch entry and waits for it, filling pre
// when it is non-nil.
func (s *NodeStore) updateNodes(
updates map[types.NodeID]UpdateNodeFunc,
pre map[types.NodeID]types.NodeView,
) {
timer := prometheus.NewTimer(nodeStoreOperationDuration.WithLabelValues("update_multi"))
defer timer.ObserveDuration()
@@ -292,6 +325,7 @@ func (s *NodeStore) UpdateNodes(updates map[types.NodeID]UpdateNodeFunc) {
w := work{
op: updateMulti,
multiUpdates: updates,
pre: pre,
result: make(chan struct{}),
}
@@ -507,6 +541,10 @@ func (s *NodeStore) applyBatch(batch []work) {
nodes[id] = n
if w.pre != nil {
w.pre[id] = pre.View()
}
relation, election := updateChanges(pre, &n)
relationChanged = relationChanged || relation
electionChanged = electionChanged || election
+22
View File
@@ -1653,6 +1653,28 @@ func TestUpdateNodeRecomputesPeersOnlyForRelationInputs(t *testing.T) {
}
}
// TestUpdateNodeDiff proves the update returns the node the writer replaced
// alongside the result, and an invalid before for a missing node.
func TestUpdateNodeDiff(t *testing.T) {
node := createTestNode(1, 1, "user1", "node1")
oldKey := node.NodeKey
store := NewNodeStore(types.Nodes{&node}, allowAllPeersFunc, TestBatchSize, TestBatchTimeout)
store.Start()
defer store.Stop()
newKey := key.NewNode().Public()
before, after, ok := store.UpdateNodeDiff(1, func(n *types.Node) { n.NodeKey = newKey })
require.True(t, ok)
assert.Equal(t, oldKey, before.NodeKey())
assert.Equal(t, newKey, after.NodeKey())
before, _, ok = store.UpdateNodeDiff(99, func(*types.Node) {})
assert.False(t, ok)
assert.False(t, before.Valid())
}
// TestListPeersExcludesSelf proves a node is never returned among its own
// peers, on both the snapshot path and the explicit peer-ID path.
//