diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index ead5c0a45..8104ad0ec 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -228,9 +228,9 @@ type NetmapUpdater interface { // rather than just full updates. type NetmapDeltaUpdater interface { // UpdateNetmapDelta is called with discrete changes to the network map. - // The mutation slice may contain [netmap.NodeMutationAdd] and - // [netmap.NodeMutationRemove] entries when peers were added or removed, - // alongside per-field patches. + // The mutation slice may contain [netmap.NodeMutationUpsert] entries when + // peers are inserted or replaced, and [netmap.NodeMutationRemove] entries + // when peers are removed, alongside per-field patches. // // The ok result is whether the implementation was able to apply the // mutations. It might return false if its internal state doesn't diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 9b4f6ae67..e9525889c 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -2319,21 +2319,21 @@ func (b *LocalBackend) UpdateNetmapDelta(muts []netmap.NodeMutation) (handled bo cn := b.currentNode() cn.UpdateNetmapDelta(muts) - // Dispatch Add/Remove per-peer to magicsock, and any per-field + // Dispatch Upsert/Remove per-peer to magicsock, and any per-field // patches via the existing UpdateNetmapDelta path. The per-peer // methods take c.mu themselves, so we can't call them from inside // magicsock.UpdateNetmapDelta which already holds c.mu. - peersAddedOrRemoved := false + peersUpsertedOrRemoved := false ms := b.MagicConn() for _, m := range muts { switch m := m.(type) { - case netmap.NodeMutationAdd: + case netmap.NodeMutationUpsert: ms.UpsertPeer(m.Node) - peersAddedOrRemoved = true - metricNetmapDeltaPeerAdded.Add(1) + peersUpsertedOrRemoved = true + metricNetmapDeltaPeerUpserted.Add(1) case netmap.NodeMutationRemove: ms.RemovePeer(m.NodeIDBeingMutated()) - peersAddedOrRemoved = true + peersUpsertedOrRemoved = true metricNetmapDeltaPeerRemoved.Add(1) default: metricNetmapDeltaPeerPatched.Add(1) @@ -2370,17 +2370,17 @@ func (b *LocalBackend) UpdateNetmapDelta(muts []netmap.NodeMutation) (handled bo return true } - // A single MapResponse can carry adds/removes (full Nodes) AND + // A single MapResponse can carry upserts/removes (full Nodes) AND // per-field patches in the same delta. Build one Notify that // reflects all of them; per-session stripping in [sendToLocked] // hides fields the watcher didn't opt in to (and promotes patches // into full Nodes for watchers that asked for PeerChanges but not // PeerPatches). - if peersAddedOrRemoved || mutationsAreWorthyOfTellingIPNBus(muts) { + if peersUpsertedOrRemoved || mutationsAreWorthyOfTellingIPNBus(muts) { notify = &ipn.Notify{} for _, m := range muts { switch m := m.(type) { - case netmap.NodeMutationAdd: + case netmap.NodeMutationUpsert: notify.PeersChanged = append(notify.PeersChanged, m.Node.AsStruct()) case netmap.NodeMutationRemove: notify.PeersRemoved = append(notify.PeersRemoved, m.NodeIDBeingMutated()) @@ -2516,7 +2516,7 @@ func ipnBusPeerChangedPatchFromNodeMutations(muts []netmap.NodeMutation) ([]*tai } for _, m := range muts { switch v := m.(type) { - case netmap.NodeMutationAdd, netmap.NodeMutationRemove: + case netmap.NodeMutationUpsert, netmap.NodeMutationRemove: // These go in PeersChanged / PeersRemoved, not as patches. continue case netmap.NodeMutationOnline: @@ -8609,11 +8609,11 @@ var ( // [mapSession.tryHandleIncrementally]. Useful as test signals that a // MapResponse landed on the incremental path with the expected // payload shape. - metricNetmapDeltaPeerAdded = clientmetric.NewCounter("localbackend_netmap_delta_peer_added") - metricNetmapDeltaPeerRemoved = clientmetric.NewCounter("localbackend_netmap_delta_peer_removed") - metricNetmapDeltaPeerPatched = clientmetric.NewCounter("localbackend_netmap_delta_peer_patched") - metricUpdatePacketFilter = clientmetric.NewCounter("localbackend_update_packet_filter") - metricUpdateUserProfiles = clientmetric.NewCounter("localbackend_update_user_profiles") + metricNetmapDeltaPeerUpserted = clientmetric.NewCounter("localbackend_netmap_delta_peer_upserted") + metricNetmapDeltaPeerRemoved = clientmetric.NewCounter("localbackend_netmap_delta_peer_removed") + metricNetmapDeltaPeerPatched = clientmetric.NewCounter("localbackend_netmap_delta_peer_patched") + metricUpdatePacketFilter = clientmetric.NewCounter("localbackend_update_packet_filter") + metricUpdateUserProfiles = clientmetric.NewCounter("localbackend_update_user_profiles") ) func (b *LocalBackend) stateEncrypted() opt.Bool { diff --git a/ipn/ipnlocal/node_backend.go b/ipn/ipnlocal/node_backend.go index 3c21ff2a8..0f7bce81e 100644 --- a/ipn/ipnlocal/node_backend.go +++ b/ipn/ipnlocal/node_backend.go @@ -675,7 +675,7 @@ func (nb *nodeBackend) UpdateNetmapDelta(muts []netmap.NodeMutation) (handled bo for _, m := range muts { switch m := m.(type) { - case netmap.NodeMutationAdd: + case netmap.NodeMutationUpsert: nid := m.Node.ID() mak.Set(&nb.peers, nid, m.Node) for _, ipp := range m.Node.Addresses().All() { diff --git a/tstest/largetailnet/delta_test.go b/tstest/largetailnet/delta_test.go index aba639f22..cb9958c93 100644 --- a/tstest/largetailnet/delta_test.go +++ b/tstest/largetailnet/delta_test.go @@ -108,19 +108,19 @@ func TestNetmapDeltaFastPath(t *testing.T) { // parallel here, hence tstest.Shard above). mFast := metricByName(t, "controlclient_map_response_handled_incrementally") mFull := metricByName(t, "controlclient_map_response_handled_full_rebuild") - mAdd := metricByName(t, "localbackend_netmap_delta_peer_added") + mUpsert := metricByName(t, "localbackend_netmap_delta_peer_upserted") mRem := metricByName(t, "localbackend_netmap_delta_peer_removed") mPatch := metricByName(t, "localbackend_netmap_delta_peer_patched") mFilter := metricByName(t, "localbackend_update_packet_filter") mUsers := metricByName(t, "localbackend_update_user_profiles") baseline := map[*clientmetric.Metric]int64{ mFast: mFast.Value(), mFull: mFull.Value(), - mAdd: mAdd.Value(), mRem: mRem.Value(), mPatch: mPatch.Value(), + mUpsert: mUpsert.Value(), mRem: mRem.Value(), mPatch: mPatch.Value(), mFilter: mFilter.Value(), mUsers: mUsers.Value(), } dumpMetrics := func(t *testing.T) { t.Helper() - for _, m := range []*clientmetric.Metric{mFast, mFull, mAdd, mRem, mPatch, mFilter, mUsers} { + for _, m := range []*clientmetric.Metric{mFast, mFull, mUpsert, mRem, mPatch, mFilter, mUsers} { t.Logf("metric %s = %d (baseline %d, delta %d)", m.Name(), m.Value(), baseline[m], m.Value()-baseline[m]) } } @@ -188,7 +188,7 @@ func TestNetmapDeltaFastPath(t *testing.T) { }) waitDelta(t, mFast, 1) - waitDelta(t, mAdd, 1) + waitDelta(t, mUpsert, 1) waitDelta(t, mFilter, 1) waitDelta(t, mUsers, 1) waitDelta(t, mFull, 0) diff --git a/types/netmap/nodemut.go b/types/netmap/nodemut.go index aa91d03ea..e899ee077 100644 --- a/types/netmap/nodemut.go +++ b/types/netmap/nodemut.go @@ -68,15 +68,17 @@ func (m NodeMutationLastSeen) Apply(n *tailcfg.Node) { n.LastSeen = new(m.LastSeen) } -// NodeMutationAdd is a NodeMutation that says a new peer has been added. -// Apply is a no-op: consumers of NodeMutationAdd must type-switch to handle -// adds by inserting Node into their peer map. -type NodeMutationAdd struct { +// NodeMutationUpsert is a NodeMutation that says a peer's full Node value +// should be inserted or replaced. +// +// Apply is a no-op: consumers of NodeMutationUpsert must type-switch to handle +// upserts by storing Node in their peer map. +type NodeMutationUpsert struct { Node tailcfg.NodeView } -func (m NodeMutationAdd) NodeIDBeingMutated() tailcfg.NodeID { return m.Node.ID() } -func (m NodeMutationAdd) Apply(*tailcfg.Node) {} +func (m NodeMutationUpsert) NodeIDBeingMutated() tailcfg.NodeID { return m.Node.ID() } +func (m NodeMutationUpsert) Apply(*tailcfg.Node) {} // NodeMutationRemove is a NodeMutation that says a peer has been removed. // Apply is a no-op: consumers of NodeMutationRemove must type-switch to handle @@ -132,9 +134,11 @@ func NodeMutationsFromPatch(p *tailcfg.PeerChange) (_ []NodeMutation, ok bool) { // by res. It returns ok=false if res contains any non-delta field as defined // by mapResponseContainsNonPatchFields. // -// Adds and removes (from res.PeersChanged / res.PeersRemoved) are emitted as -// NodeMutationAdd / NodeMutationRemove entries. Callers must type-switch to -// handle those alongside field mutations. +// Upserts and removes (from res.PeersChanged / res.PeersRemoved) are emitted +// as NodeMutationUpsert / NodeMutationRemove entries. A PeersChanged entry can +// be either a new peer or a full replacement for an existing peer that couldn't +// be represented as PeerChangedPatch. Callers must type-switch to handle those +// alongside field mutations. func MutationsFromMapResponse(res *tailcfg.MapResponse, now time.Time) (ret []NodeMutation, ok bool) { if now.IsZero() { now = time.Now() @@ -149,7 +153,7 @@ func MutationsFromMapResponse(res *tailcfg.MapResponse, now time.Time) (ret []No for _, n := range res.PeersChanged { // Any n still in PeersChanged after patchifyPeersChanged is a // truly-new (or replaced) peer. - ret = append(ret, NodeMutationAdd{Node: n.View()}) + ret = append(ret, NodeMutationUpsert{Node: n.View()}) } for _, p := range res.PeersChangedPatch { deltas, ok := NodeMutationsFromPatch(p) @@ -174,7 +178,7 @@ func MutationsFromMapResponse(res *tailcfg.MapResponse, now time.Time) (ret []No // mapResponseContainsNonPatchFields reports whether res contains any field // that can't be expressed as a per-peer NodeMutation (including the new -// NodeMutationAdd / NodeMutationRemove variants) or via the sibling narrow +// NodeMutationUpsert / NodeMutationRemove variants) or via the sibling narrow // setter methods on the map-session backend (e.g. UpdatePacketFilter). // // When this returns true, the caller must fall back to rebuilding and @@ -182,7 +186,7 @@ func MutationsFromMapResponse(res *tailcfg.MapResponse, now time.Time) (ret []No // handled incrementally. // // PeersChanged, PeersRemoved, and PacketFilter(s) are intentionally not in -// this list: new/removed peers ride NodeMutationAdd/Remove, packet +// this list: upserted/removed peers ride NodeMutationUpsert/Remove, packet // filter updates are delivered via the backend's UpdatePacketFilter // method, and UserProfile updates ride the backend's UpdateUserProfiles // method. diff --git a/types/netmap/nodemut_test.go b/types/netmap/nodemut_test.go index 965e9a367..406f494f9 100644 --- a/types/netmap/nodemut_test.go +++ b/types/netmap/nodemut_test.go @@ -55,7 +55,7 @@ func TestMapResponseContainsNonPatchFields(t *testing.T) { // The three legacy delta fields handled via NodeMutation patches. want = false case "PeersChanged", "PeersRemoved": - // Now carried as NodeMutationAdd / NodeMutationRemove entries. + // Now carried as NodeMutationUpsert / NodeMutationRemove entries. want = false case "PacketFilter", "PacketFilters": // Now delivered separately via PacketFilterUpdater. @@ -196,7 +196,7 @@ func TestMutationsFromMapResponse(t *testing.T) { mr: &tailcfg.MapResponse{ PeersChanged: []*tailcfg.Node{{ID: 7}}, }, - want: muts(NodeMutationAdd{Node: (&tailcfg.Node{ID: 7}).View()}), + want: muts(NodeMutationUpsert{Node: (&tailcfg.Node{ID: 7}).View()}), }, { name: "add-and-remove-mixed-with-patch", @@ -211,7 +211,7 @@ func TestMutationsFromMapResponse(t *testing.T) { want: muts( NodeMutationRemove{3}, NodeMutationDERPHome{5, 2}, - NodeMutationAdd{Node: (&tailcfg.Node{ID: 7}).View()}, + NodeMutationUpsert{Node: (&tailcfg.Node{ID: 7}).View()}, ), }, }