change: coalesce policy recomputes that carry an origin node

The recompute render ignores the origin and always carries the node's
own entry, so a burst of tag writes cost every node one per write.
This commit is contained in:
Kristoffer Dalby committed 2026-10-30 14:55:37 +00:00
1 parent 78329ec0ea
commit 11ae125a9f
4 files changed
+28 -20

No files matched your search

+11 -2
View File
@@ -1135,8 +1135,17 @@ func TestBatcherCoalescesPolicyRecomputesPerTick(t *testing.T) {
// a sentinel. All land in one tick; the sentinel rides the same work
// item after the recompute(s), so its arrival marks the end of this
// tick's policy responses for a node.
for range policyChangesPerTick {
batcher.AddWork(change.PolicyChange())
for i := range policyChangesPerTick {
c := change.PolicyChange()
// Every other one carries an origin, as a tag write's
// recompute does (State.SetNodeTags). The mapper renders it
// like any other recompute, so it must coalesce like one.
if i%2 == 1 {
c.OriginNode = testData.Nodes[0].n.ID
}
batcher.AddWork(c)
}
batcher.AddWork(change.DERPMap())
+3 -4
View File
@@ -1266,10 +1266,9 @@ func (s *State) SetNodeTags(nodeID types.NodeID, tags []string) (types.NodeView,
c = change.NodeAdded(nodeID)
}
// Set OriginNode so the mapper knows to include self info for this node.
// When tags change, persistNodeAndRefreshPolicy returns PolicyChange which doesn't set OriginNode,
// so the mapper's self-update check fails and the node never sees its new tags.
// Setting OriginNode ensures the node gets a self-update with the new tags.
// Records the retagged node. Its new tags reach it either way: a policy
// recompute always renders the node's own entry, and NodeAdded already
// carries this origin.
c.OriginNode = nodeID
// The new tags may approve announced routes, as on Tailscale SaaS. This
+6 -6
View File
@@ -277,13 +277,13 @@ func FilterForNode(nodeID types.NodeID, rs []Change) []Change {
return result
}
// IsBroadcastPolicyChange reports whether r is a tailnet-wide policy recompute
// with no per-node payload. A recompute reads the current snapshot, so every
// such change is interchangeable and same-tick duplicates are redundant. A
// targeted or self-update ([Change.OriginNode]) recompute is per-node, so it is
// not one of these.
// IsBroadcastPolicyChange reports whether r is a tailnet-wide policy recompute.
// A recompute reads the current snapshot and always carries the node's own
// entry, so every such change is interchangeable and same-tick duplicates are
// redundant. [Change.OriginNode] does not make one per-node: the render never
// reads it. A targeted recompute is per-node, so it is not one of these.
func (r Change) IsBroadcastPolicyChange() bool {
return r.RequiresRuntimePeerComputation && !r.IsTargetedToNode() && r.OriginNode == 0
return r.RequiresRuntimePeerComputation && !r.IsTargetedToNode()
}
// DedupePolicyChanges keeps the first broadcast policy change in a tick and
+8 -8
View File
@@ -407,7 +407,7 @@ func TestChange_IsBroadcastPolicyChange(t *testing.T) {
want bool
}{
{name: "policy change", c: PolicyChange(), want: true},
{name: "self-update recompute", c: originUpdate, want: false},
{name: "self-update recompute", c: originUpdate, want: true},
{name: "targeted recompute", c: targeted, want: false},
{name: "online patch", c: NodeOnline(1), want: false},
{name: "full update", c: FullUpdate(), want: false},
@@ -422,9 +422,9 @@ func TestChange_IsBroadcastPolicyChange(t *testing.T) {
}
func TestDedupePolicyChanges(t *testing.T) {
// originRecompute is a runtime recompute carrying node-specific payload
// (OriginNode), so it is not the canonical broadcast PolicyChange and must
// never be coalesced away.
// originRecompute is a runtime recompute stamped with the node that caused
// it, as a tag write returns it. The mapper renders it like any other
// recompute, so it must coalesce like any other.
originRecompute := PolicyChange()
originRecompute.OriginNode = 7
@@ -463,14 +463,14 @@ func TestDedupePolicyChanges(t *testing.T) {
want: []Change{PolicyChange(), NodeAdded(5)},
},
{
name: "recompute carrying OriginNode is kept alongside the canonical one",
name: "recompute carrying OriginNode collapses into the canonical one",
changes: []Change{PolicyChange(), originRecompute, PolicyChange()},
want: []Change{PolicyChange(), originRecompute},
want: []Change{PolicyChange()},
},
{
name: "non-canonical recomputes are not collapsed",
name: "recomputes carrying OriginNode collapse to one",
changes: []Change{originRecompute, originRecompute},
want: []Change{originRecompute, originRecompute},
want: []Change{originRecompute},
},
{
name: "changes without any recompute are unchanged",