test: fix integration flakiness from shared-etcd cross-contamination - #312
Conversation
The nightly beast kept failing Watch_ShouldRecover_AfterServerRestart with 'Collection was empty' even after the revision-resume fix (#310). Root cause was test cross-contamination on the single shared etcd1, not the reconnect logic: 1. xUnit runs different test collections in PARALLEL and there was no assembly-wide guard. AuthClientIntegrationTests enables/disables cluster-wide auth on etcd1; any other test hitting etcd1 unauthenticated during that window fails with 'etcdserver: user name is empty' -> the watch delivers no events -> empty. 2. WatchResilienceTests pause/restart their etcd server, disrupting every other test that shares it (parallel or serial). Fix: - Add xunit.runner.json with parallelizeTestCollections=false so collections run sequentially and auth toggling can't overlap other etcd1 tests. - Give WatchResilienceTests a dedicated single-node etcd (etcd-resilience, port 2409) so pausing/restarting it never affects the shared etcd1 cluster tests. Verified: full integration suite 57/57 green; both resilience tests pass against the dedicated instance.
|
Summary dotnet-etcd 94.5% |
WatchResilienceTests failed its FIRST assertion ("Collection was empty")
about once every 20 beast iterations. The reconnect logic was not at
fault: the watch was never registered at all.
dotnet-etcd never waited for etcd's Created acknowledgement. CreateWatchAsync
awaited only RequestStream.WriteAsync, which means "the create request was
written to the socket" -- not "etcd registered the watcher". etcd's own proto
says so: "Since creating a watcher in etcd is not a synchronous operation".
Two consequences, both proven by tests that fail before this change:
* Watch() returned before the watch existed server-side, so a write issued
immediately after could be applied first and its event never delivered.
* If the stream died before ANY response arrived -- exactly what happens when
a watch is opened against an etcd that is still restarting -- NextRevision
was still 0, so HandleConnectionFailure re-registered with StartRevision=0,
meaning "watch from now". But "now" was already past the caller's write, so
the event was lost permanently and silently. The revision-resume added in
#310 only engages once NextRevision > 0, so it never covered this window.
WatchAsync now awaits the Created ack before returning. If the stream dies
while waiting, the reconnect re-sends the create and its ack completes the same
wait, so a create lost to a dying stream is retried rather than silently
downgraded to "from now".
Supporting fixes, each with a regression test:
* Register the watch before writing the create: the server can answer while
WriteAsync is still in flight, and TrackResumeRevision dropped responses for
watches not yet in _watches -- losing the created revision.
* Seed NextRevision from the caller's StartRevision (as etcd clientv3 does with
nextRev := initReq.rev). A Created ack carries the CURRENT cluster revision,
so without this a watch resuming from a checkpoint would skip its backlog.
* Ignore the ack header for a replay create, for the same reason.
* Run user callbacks off the receive loop, serialized on one chain. They ran
inline, so a callback that started another watch deadlocked: the loop it
blocked was the only thing that could deliver the new watch's ack.
* Serialize writes to the duplex stream (gRPC allows one pending write).
* Dispose the abandoned Watcher on reconnect; it was leaked, and a still-healthy
stream went on delivering every event a second time.
* Keep the 5s reconnect retry alive when a re-register fails, and surface
failures from the sync overloads as RpcException rather than AggregateException.
Tests: health-gate etcd-resilience (added in #312 but never waited for), poll
instead of sleeping on fixed timers, collect events in a ConcurrentQueue (they
were appended from the receive loop and read from the test thread), capture test
stdout in beast.sh, and fix beast.sh -f All under bash 3.2.
Full suite 417/417; 25x full-suite beast all green.
1 participant
Problem
The nightly Beast (flakiness) workflow kept failing
WatchResilienceTests.Watch_ShouldRecover_AfterServerRestartwithAssert.NotEmpty() Failure: Collection was empty— even after the revision-resume fix in #310.Root cause: test cross-contamination on the single shared
etcd1Not the reconnect logic. Confirmed by experiment:
AuthClientIntegrationTestsenables/disables cluster-wide auth onetcd1. Any other test hittingetcd1unauthenticated during that window fails withetcdserver: user name is empty— so the resilience watch receives no events → empty collection. (Verified: manually enabling auth makes the test fail immediately.)WatchResilienceTestspause/restart their etcd, breaking whatever test runs next onetcd1— parallel or serial (serializing alone surfacedEtcdClientIntegrationTestsHTTP/2 failures against a pausedetcd1).Fix
xunit.runner.jsonwithparallelizeTestCollections: false— collections run sequentially, so auth toggling can't overlap otheretcd1tests.etcd-resiliencecontainer (single node, port 2409) forWatchResilienceTests, so pausing/restarting it never touches the sharedetcd1cluster.Verification
etcd1untouched.Follow-up to #310 — together these resolve the recurring nightly Beast failures.