Cory SniderandClaude Opus 5
4f69b464f0
handleTableEvent refuses a table entry from a node it does not believe participates in the network. That check is a hard precondition for accepting an entry, but the state it consults -- which nodes are attached to which networks -- is disseminated only by NetworkEvent gossip, and that is a terminating epidemic: a node which learns an event relays it to GossipNodes peers for retransmitLimit transmissions and then goes quiet, so once every infected node has spent its budget the fact stops moving for good. Usually it reaches everyone. Occasionally it misses a node, and that node then rejects every entry that owner ever writes to the network, for the lifetime of the cluster. Nothing recovers it. bulkSyncTables hands the node those very entries every PushPullInterval and it rejects every one of them, because the sync carries the entries but not the membership they are predicated on. The only other channel is memberlist's push/pull, which is pairwise and random -- one peer per round -- so it diffuses an attachment held by a minority of the cluster slowly and with no guarantee at all. Send the attachment view for the networks being synced ahead of their entries, so a bulk sync converges the membership it needs rather than assuming it. These are ordinary network events, already dispatched from a bulk sync's compound payload, so handleNetworkEvent applies its usual Lamport-time check and a receiver which is already up to date does nothing. Ordering is load-bearing: handleCompound processes parts in order, so the attachments land before the entries which depend on them. Raising RetransmitMult or GossipNodes would only move the tail probability. Bulk sync is the right home for the repair because it already runs unconditionally on its own timer and findCommonNetworks scopes it to the peers which actually share the network -- exactly the nodes whose entry acceptance depends on the state being repaired. Found by TestNetworkDBAlwaysConverges, which caught one node permanently missing the three entries owned by one peer. Instrumenting the receiver showed all 54 deliveries of that peer's entries rejected for the owner not being attached, 8 of them arriving by bulk sync; the node had never received a single network event naming that attachment, while the other 15 had all applied it. Reproduced from the recorded failfile in 2 of 25 runs before this change and 0 of 121 after -- each run replaying the failfile and then generating a fresh 100-case property run. Signed-off-by: Cory Snider <csnider@mirantis.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
36 lines
1.0 KiB
Go
36 lines
1.0 KiB
Go
package networkdb
|
|
|
|
import (
|
|
"testing"
|
|
|
|
"github.com/hashicorp/memberlist"
|
|
"gotest.tools/v3/assert"
|
|
)
|
|
|
|
func TestNodeCanReceiveNetworkEvents(t *testing.T) {
|
|
for _, tc := range []struct {
|
|
name string
|
|
meta []byte
|
|
want bool
|
|
}{
|
|
{"no metadata at all", nil, false},
|
|
{"empty metadata, as every daemon before this sent", []byte{}, false},
|
|
{"advertises ltime-aware invalidation", []byte{nodeMetaLTimeInvalidation}, true},
|
|
{"advertises something later", []byte{nodeMetaLTimeInvalidation + 1}, true},
|
|
} {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
n := &node{Node: memberlist.Node{Meta: tc.meta}}
|
|
assert.Equal(t, n.canReceiveNetworkEvents(), tc.want)
|
|
})
|
|
}
|
|
}
|
|
|
|
// What this daemon advertises has to satisfy its own predicate, or a cluster of
|
|
// current daemons would never send each other attachments at all.
|
|
func TestNodeMetaAdvertisesOwnCapability(t *testing.T) {
|
|
d := &delegate{}
|
|
n := &node{Node: memberlist.Node{Meta: d.NodeMeta(memberlist.MetaMaxSize)}}
|
|
assert.Check(t, n.canReceiveNetworkEvents())
|
|
assert.Check(t, len(n.Meta) <= memberlist.MetaMaxSize)
|
|
}
|