Flaky test: ConfigPublishCoordinatorTimeoutTests.DeadlineElapsed_for_current_deployment_marks_TimedOut races its own 150 ms wall-clock deadline #544

New Issue

2026-08-06T07:13:51-04:00

dohertj2 commented

2026-08-06 07:13:51 -04:00

ConfigPublishCoordinatorTimeoutTests.DeadlineElapsed_for_current_deployment_marks_TimedOut is racy on its own terms and flakes under load.

Symptom. The test failed on 2 of ~11 full-suite runs during the ClusterClient→gRPC Phase 1A work (feat/grpc-node-command). It is green at the branch base and green in isolation; what changed is that task 1A.2 roughly doubled the ControlPlane test project, so the suite now runs it under more concurrent load.

Why it is racy, independent of any product change. The test arms a wall-clock deadline of ~150 ms and inserts the Deployment row after spawning the coordinator. Both halves are timing assumptions:

150 ms of wall clock is inside the noise band of a loaded test host (xunit parallel collections, EF in-memory warmup, JIT of the first coordinator activation).
The row is inserted after the coordinator is already ticking, so a slow insert can put the row in place after the deadline the test intends to measure has already been evaluated — or the tick can land before the row exists at all.

Nothing in the product is implicated: the coordinator's deadline path itself is covered by the same test when it wins the race, and the observed failures are the assertion on Status == TimedOut not yet being true / already having passed.

Fix belongs in the test, not the product. Either:

inject the clock the coordinator reads (preferred — makes the deadline deterministic and removes the wall-clock dependency entirely), or
insert the deployment row before spawning the coordinator and raise the deadline well clear of scheduler noise (seconds, not milliseconds).

Filed from the Phase 1A live gate (docs/plans/2026-08-06-clusterclient-to-grpc-tracking.md § Issues filed by this program).

`ConfigPublishCoordinatorTimeoutTests.DeadlineElapsed_for_current_deployment_marks_TimedOut` is racy on its own terms and flakes under load. **Symptom.** The test failed on 2 of ~11 full-suite runs during the ClusterClient→gRPC Phase 1A work (`feat/grpc-node-command`). It is green at the branch base and green in isolation; what changed is that task 1A.2 roughly doubled the ControlPlane test project, so the suite now runs it under more concurrent load. **Why it is racy, independent of any product change.** The test arms a wall-clock deadline of ~150 ms and inserts the `Deployment` row *after* spawning the coordinator. Both halves are timing assumptions: - 150 ms of wall clock is inside the noise band of a loaded test host (xunit parallel collections, EF in-memory warmup, JIT of the first coordinator activation). - The row is inserted after the coordinator is already ticking, so a slow insert can put the row in place after the deadline the test intends to measure has already been evaluated — or the tick can land before the row exists at all. Nothing in the product is implicated: the coordinator's deadline path itself is covered by the same test when it wins the race, and the observed failures are the assertion on `Status == TimedOut` not yet being true / already having passed. **Fix belongs in the test, not the product.** Either: 1. inject the clock the coordinator reads (preferred — makes the deadline deterministic and removes the wall-clock dependency entirely), or 2. insert the deployment row *before* spawning the coordinator and raise the deadline well clear of scheduler noise (seconds, not milliseconds). Filed from the Phase 1A live gate (`docs/plans/2026-08-06-clusterclient-to-grpc-tracking.md` § Issues filed by this program).

dohertj2 referenced this issue from a commit

2026-08-06 19:36:00 -04:00

test(controlplane): #544 de-flake deadline-timeout test — seed acks before spawn, un-hittable wall deadline

dohertj2 referenced this issue from a commit

2026-08-06 19:36:00 -04:00

Merge fix/issues-543-544-545: whitespace-key fail-closed backport (#543), de-flaked deadline test (#544), h2c accepted-posture record (#545)

dohertj2 commented

2026-08-06 19:37:00 -04:00

Fixed, merged to master @ 1df84703 and pushed (commit 722a222d, test-only).

Took option 2: the NodeDeploymentState row is now seeded before the coordinator spawns (closing the PreStart read race — if PreStart won, _expectedAcks was empty and DeadlineElapsed early-returned on 0 == 0, never reaching TimedOut), and the wall deadline is raised to 5 minutes (matching the file's other tests) so the real timer can never fire inside the 3 s assertion window — the test's explicit Tell(DeadlineElapsed) is the only deadline signal. The reviewer traced both races through the coordinator source and confirmed the test still exercises the real TimedOut path (0 acks ≠ 1 expected), not a tautology. The de-flaked test ran 5× consecutively green; class 3/3; it also passed inside the full-suite run on the merged SHA (exit 0, 9,333/0).

Option 1 (inject the clock) was deliberately not taken: it is a product change, and this issue scoped the fix to the test.

Side-observation for the record (out of scope, product behavior): ConfigPublishCoordinator seals a deployment whose expected-ack set is empty — exactly the state the race produced in-test. Whether an empty driver set should seal green may deserve its own issue.

Fixed, merged to `master` @ `1df84703` and pushed (commit `722a222d`, test-only). Took option 2: the `NodeDeploymentState` row is now seeded **before** the coordinator spawns (closing the PreStart read race — if PreStart won, `_expectedAcks` was empty and `DeadlineElapsed` early-returned on `0 == 0`, never reaching TimedOut), and the wall deadline is raised to 5 minutes (matching the file's other tests) so the real timer can never fire inside the 3 s assertion window — the test's explicit `Tell(DeadlineElapsed)` is the only deadline signal. The reviewer traced both races through the coordinator source and confirmed the test still exercises the real TimedOut path (0 acks ≠ 1 expected), not a tautology. The de-flaked test ran 5× consecutively green; class 3/3; it also passed inside the full-suite run on the merged SHA (exit 0, 9,333/0). Option 1 (inject the clock) was deliberately not taken: it is a product change, and this issue scoped the fix to the test. Side-observation for the record (out of scope, product behavior): `ConfigPublishCoordinator` seals a deployment whose expected-ack set is empty — exactly the state the race produced in-test. Whether an empty driver set should seal green may deserve its own issue.

dohertj2 closed this issue

2026-08-06 19:37:04 -04:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#544