daemon/libnetwork/networkdb: compare Lamport times when invalidating queued broadcasts

[corhere]

• Related to: daemon/libnetwork/networkdb: carry network attachments in bulk syncs #53437

Summary

memberlist.TransmitLimitedQueue asks a broadcast whether it invalidates one already queued, and both of NetworkDB's answers ignored Lamport time. Nothing keeps queue order and Lamport order together, so "the last one queued wins" is not the same as "the newest one wins".

Every path which queues an event does so after releasing the lock it mutated state under:

• CreateEntry, UpdateEntry and DeleteEntry each Unlock() before calling sendTableEvent.
• handleTableMessage and handleNodeMessage queue their relay after handleTableEvent / handleNodeEvent has returned.

Two writers to one key can therefore take the lock in Lamport order and reach the queue in the opposite order. The node's own state is right either way — the Lamport check ran under the lock — so what is affected is what it tells everyone else.

tableEventMessage

Matched on (table, network, key) alone, so a straggler evicted the fresher event and put a superseded value on the wire in its place, until anti-entropy corrected it a sync interval later. Now carries the event's Lamport time and refuses to supersede anything fresher. Equal times still collapse, so duplicate relays of one event do not accumulate.

nodeEventMessage

Invalidated nothing at all, so a peer which flaps filled every other node's queue with its churn — a join and a leave per cycle, of which only the newest tells a receiver anything, since handleNodeEvent discards any event no fresher than what it already holds.

Invalidation could not simply be switched on, because the type was doing two jobs. sendNodeEvent waits to learn that this node's own announcement went out and Finished() is how it is told, but memberlist calls Finished() on whatever it invalidates — so an invalidatable own message would release that wait as though the event had been broadcast when it had in fact been dropped. Only relayed messages carried no notify channel, which made the distinction implicit and easy to lose.

So the type is split:

• ownNodeEventMessage — this node's own join or leave, and a memberlist.UniqueBroadcast, which the queue neither deduplicates nor offers up for invalidation. The hazard becomes structural rather than guarded. There are only ever a handful in flight; sendNodeEvent is called on cluster join, rejoin and leave.
• relayedNodeEventMessage — a peer's event being passed on, superseding an event for the same node which is no fresher than itself.

memberlist.NamedBroadcast would have given O(1) deduplication instead of the Invalidates scan, but its deduplication is unconditionally last-one-wins and would drop the fresher event — the very bug being fixed here.

Testing

Each change is pinned by a test that fails without it, and the queue-level tests drive the real TransmitLimitedQueue rather than calling Invalidates directly:

reverted to	test that fails
(table, network, key) matching	staler_must_not_evict_fresher, TestTableEventQueueKeepsFreshest (queue dropped the fresher event: [stale])
own message not a UniqueBroadcast, with the unchecked type assertion this file's other types use	TestOwnNodeEventSurvivesRelayedEvents panics inside the queue

Two notes for reviewers, since the tests do not make either obvious:

• TestOwnNodeEventSurvivesRelayedEvents passes if the UniqueBroadcast marker is dropped but the checked type assertion is kept — the two mechanisms are independent, and either alone prevents the invalidation. The var _ memberlist.UniqueBroadcast = (*ownNodeEventMessage)(nil) assertion is what pins the marker.
• relayedNodeEventMessage.Invalidates uses a checked type assertion because two message types now share nDB.nodeBroadcasts. The queue does not in fact offer a UniqueBroadcast up for invalidation, but with the unchecked form the marker becomes load-bearing for memory safety rather than just for correctness.

Full package green, plain and under -race.

Release notes (optional)

Fix a node gossiping a superseded value for a Swarm service discovery entry after concurrent updates to the same key.
Reduce gossip traffic generated by a node that repeatedly disconnects and rejoins the cluster.

A picture of a cute animal (not mandatory but encouraged)

🦔

Created with: Claude Code (Opus 5)

[thaJeztah]

=== FAIL: daemon/logger/awslogs TestNewAWSLogsClientUserAgentHandler (unknown)
fatal error: concurrent map read and map write

goroutine 51 [running]:
internal/runtime/maps.fatal({0x7ff7eb20f322?, 0x140?})
	C:/go/src/runtime/panic.go:1181 +0x18
net/http.Header.sortedKeyValues(0xdc1f5b73ce0, 0xdc1f5a1e6f0)
	C:/go/src/net/http/header.go:174 +0x1d3
net/http.Header.writeSubset(0xdc1f5b73ce0, {0x7ff7eb246140?, 0xdc1f5c10000?}, 0xdc1f5a1e6f0, 0x0)
	C:/go/src/net/http/header.go:195 +0xc5
net/http.(*Request).write(0xdc1f5b67a40, {0x7ff7eb246140, 0xdc1f5c10000}, 0x0, 0xdc1f5c06060, 0x0)
	C:/go/src/net/http/request.go:715 +0x82b
net/http.(*persistConn).writeLoop(0xdc1f5b67b80)
	C:/go/src/net/http/transport.go:2655 +0x174
created by net/http.(*Transport).dialConn in goroutine 46
	C:/go/src/net/http/transport.go:1995 +0x16fe

I think there were some PRs for this, but we had to check if it's only the test, or an actual issue.

[Copilot]

Pull request overview

This PR fixes NetworkDB’s use of memberlist.TransmitLimitedQueue invalidation so that queued broadcasts respect Lamport time ordering, preventing newer state from being displaced by older events when concurrent writers reach the broadcast queue out of order.

Changes:

• Add Lamport time to tableEventMessage queue entries and invalidate only when the new event is at least as fresh as the queued one.
• Split node broadcasts into ownNodeEventMessage (marked UniqueBroadcast to avoid unsafe invalidation/early Finished()) and relayedNodeEventMessage (Lamport-aware invalidation for peer churn).
• Add queue-level tests that exercise the real TransmitLimitedQueue behavior for both table and node events.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
daemon/libnetwork/networkdb/delegate.go	Propagates Lamport time into queued rebroadcast messages and switches node rebroadcast to the relayed-message type.
daemon/libnetwork/networkdb/broadcast.go	Implements Lamport-aware invalidation for table/node relays and splits own vs relayed node broadcasts to preserve sendNodeEvent semantics.
daemon/libnetwork/networkdb/broadcast_test.go	Adds targeted tests that validate queue behavior (freshest survives, churn collapses, own-node broadcasts aren’t invalidated).

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[thaJeztah]

LGTM