TestNetworkDBAlwaysConverges was gated behind the slowtests build tag,
which nothing in CI sets, because it spent its time asleep: every wait
in NetworkDB is timer-driven, so a run that does almost no work still
takes minutes of wall-clock to let gossip rounds and anti-entropy sweeps
happen. A property test nobody can afford to run is not much of a
property test.
Run it inside a testing/synctest bubble so those waits cost nothing. A
bubble's clock only advances once every goroutine in it is durably
blocked, and a goroutine parked on a socket is not durably blocked, so
this first requires getting the sockets out of the way: add an
unexported Config.transport for tests to substitute a memberlist
transport, and implement one over channels and net.Pipe
(memcluster_test.go). memberlist ships a MockTransport, but its sends
block until the peer picks the message up, its Shutdown leaves the node
reachable, and its address maps are unsynchronized -- all three are
fatal inside a bubble, so this is a fresh implementation. The default
100 rapid checks now take about five seconds rather than tens of
minutes, so drop the build tag.
Measuring convergence latency is a separate job from property-testing
it, and the two are in tension: measuring precisely means quiescing the
cluster before each observation, and quiescing is exactly what removes
the interleavings the property test exists to explore. Keep them apart.
TestNetworkDBAlwaysConverges therefore never quiesces and makes no
timing claim at all. It could not honestly make one: it observes after
the fact, against a target that moved throughout the run, so roughly
three quarters of runs are already converged by the time anything looks.
Those observations are left-censored -- an upper bound, not a
measurement -- and discarding them is not neutral either, because a run
is censored precisely when convergence beat the state machine's trailing
sleeps, so fast convergences are censored preferentially.
TestNetworkDBConvergenceLatency gives up interleaving for a clean
measurement instead: quiesce, apply exactly one perturbation of a known
kind, start the clock before the change, and watch to full agreement
with a 1ms step. Targets are Lamport times rather than values -- a write
at ltime L has reached a node once that node holds the key at ltime >= L
-- which is immune to a later write superseding the value and stays well
defined if anything else is in flight. Perturbations cover entry create,
update and delete, network join and leave, and two shapes chosen to
exercise the anti-entropy path: writing to a network in the same breath
as joining it, and joining a network which already holds entries.
TestNetworkDBConvergenceUnderChurn measures the same tracked write with
continuous background traffic: junk writes from rotating nodes and one
node repeatedly leaving and rejoining. Convergence is required of the
nodes which do not flap, because a node outside the network legitimately
does not need the entry, and including it would make the target depend
on where in its cycle it happened to be. That makes it a weaker target
than the quiet measurements use, so each sample records the number of
nodes it waited for alongside the cluster size; without that column a
churn row sitting next to a quiet one reads as a like-for-like
comparison when it is not.
Attributing a sample to a mechanism cannot be done from its duration
without assuming the answer, so add an unexported
Config.tableEventObserver in the same spirit as Config.transport, called
when a table event received from a peer is applied, and told which node
applied it and whether it arrived by bulk sync. The receiving node has
to be part of that report because one observer is shared by a whole
cluster, and a node which rejoins re-acquires the entire table by bulk
sync as a matter of course, so a cluster-wide tally says nothing about
how the entry being measured travelled. The observer runs with the write
lock held, so it is documented as having to be non-blocking and
non-reentrant; the only caller counts.
Each sample records both counts rather than a single winning label,
because "arrived by bulk sync" is not "arrived by the anti-entropy
sweep". JoinNetwork bulk syncs before it returns and bulkSyncNode ships
the whole byNetwork index, so a node rejoining a network pushes
everything it holds at its peers -- deliveries an order of magnitude
sooner than the ~30s sweep, and indistinguishable from it on the
receiving side, which is the only side this observer sees. Collapsing to
one label reported "bulksync" for the overwhelming majority of churn
samples, at a couple of hundred milliseconds, which is not the mode such
a label is read as meaning.
Getting the pre-measurement quiesce right mattered more than expected. A
check comparing only the size of each node's participant set passes
immediately after the joins are issued, while every node still lists
only itself -- and until a node knows the writer participates in a
network, handleTableEvent rejects its table events outright, so a
measurement taken then is really timing join propagation as well. The
check also has to treat a node which is leaving as no longer joined:
LeaveNetwork keeps its thisNodeNetworks entry for reapNetworkInterval
while dropping it from networkNodes, so on presence alone a departing
node is held to a member set it has already left and can never satisfy.
What the numbers say. On a quiet cluster, gossip settles in 200ms at two
or three nodes rising to about 800ms at twenty-five, cleanly logarithmic
in cluster size and near-identical across create, update, delete, join
and leave (1280 samples). The tail is a different mechanism: one
write-after-join sample on ten nodes took 58.2s, roughly two
anti-entropy sweeps, and arrived by bulk sync accordingly. Under churn
the p50 rises to 400ms at ten nodes and 1.4s at twenty-five, with a p90
of 11s at twenty-five; more interesting than the tail is that bulk sync
becomes the *primary* delivery path even at five nodes once the
broadcast queues are deep, so gossip is not occasionally missing but
routinely being outrun.
Write-after-join measures as an ordinary gossip delivery one round
slower than a plain create, rather than falling through to anti-entropy
as one might expect -- but not because the join wins the race. Nothing
sequences the two: the join goes to the cluster-wide networkBroadcasts,
drained by memberlist's own gossip, and the write to the per-network
tableBroadcasts, drained by NetworkDB's gossip() to a different set of
peers. Instrumenting the nodePresent branch shows the write really is
rejected, twice at three nodes and twelve times at twenty-five, where a
plain create is rejected none. What recovers it is retransmission:
tableBroadcasts carries RetransmitMult 4, so a later copy lands once the
join has propagated. Worth keeping for that recovery path -- a change
which stopped retransmitting rejected table events would show up here as
a jump to tens of seconds, and in no other perturbation.
Two caveats on the recorded timings, which land in
testdata/convergence_virtual-<timestamp>-<pid>.csv -- one file per test
binary invocation, named after the style of rapid's fail files, so that
samples taken against different builds do not accumulate into one
undifferentiated pool. They are virtual time: comparable across runs and
machines and sensitive to a change in the algorithm rather than to machine
load, but they account for none of the network RTT, syscall or serialization
cost a real cluster pays, so they are a floor for any wall-clock figure and
never the figure itself. And the clock only advances on a timer, so work
which completes without waiting for one is free here however much real work
it does -- joining a network which already has entries costs no virtual time
at all, because JoinNetwork bulk syncs before it returns. A zero means "no
timer had to fire", which is a genuine and useful distinction, but it is not
"no time passed".
Each sample also records its poll count and asserts the in-memory
network dropped nothing, since a drop means memPacketBuffer was too
shallow and the measurement was inflated by retransmits a real cluster
would not have needed -- otherwise indistinguishable from NetworkDB
genuinely falling back to anti-entropy.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
