Repository navigation
LiveQueryHandoffDropTest flakes — the early→live handoff race may not be fully closed #2338
Description
MeshWeaver.Hosting.Test.LiveQueryHandoffDropTest.A_notification_snapshotted_before_Initial_and_delivered_after_still_reaches_the_query failed on PR #2328, which changes only markdown (ContinuousDeliveryContract.md, a new ReleaseEventBus.md, one What's New entry). It cannot touch a live query.
Why this is worth a ticket rather than a re-run
That test is the guard introduced by b74277304 — "fix: a live query's early→live handoff dropped a change notification permanently". So the thing that failed is the regression guard for that exact race, and it failed on a change that cannot have caused it.
Per the standing rule — "flakes are real races. Either fix the race or pin the failure with a smaller repro; do not retry" — making it green by re-running would hide the signal. The most likely reading is that b74277304's fix narrowed the window rather than closing it, and some interleaving still drops the notification.
Evidence
| observation | value |
|---|---|
| failed on | #2328 (docs-only), Run tests (shard 2) |
shard 2 on main |
success on the 3 most recent Build-and-Test runs |
| other docs-only PR (#2327) | 26 green, 0 failing |
| project marker | [CI] MeshWeaver.Hosting.Test exit=1 TESTFAIL |
So: green on main, green on a sibling docs PR, red once here — a timing-sensitive failure, not a regression.
Suggested next step
Rather than re-running, reproduce under load — the failure class here is documented as bulk-only, and DOTNET_PROCESSOR_COUNT=4 reproduces CI's contention locally. If it reproduces, the interleaving that still drops the notification should be pinnable deterministically; if it does not, the window is narrow enough that a repro test wants writing before the next occurrence eats someone's CI cycle.
Filed by the session that hit it, so the signal is not lost — I have not re-run it, and #2328 is otherwise 22 green / 0 other failures.
Activity
rbuergi commentedon Aug 26, 2026
See #2339 — the same file (
src/MeshWeaver.Data/DataExtensions.cs) hosts both this handoff and theMergeGuardthat is refusing concurrent writes there. Two stream-lifecycle commits landed on it recently (0f5d8b7d7changes when a sync stream announces its end;a496723b4moves when a source starts). That may explain two guards for previously-fixed races becoming reachable at once, on PRs that cannot touch either. Not a diagnosis — recorded so the two are investigated together rather than as independent flakes.rbuergi commentedon Aug 26, 2026
🚨 Now blocking a production fix. Third independent hit within the hour:
Connection.Orleansonly#2333 is the fix for the live outage — memex.systemorph.com has been unable to boot any current image for ~20 h (
MissingMethodExceptiononLanguageModelCatalogSource's 8-arg ctor from modules built 2026-08-25 01:17). This test failing is the only thing standing between that fix and merge.Raising priority rather than working around it: re-running to green would be the wrong call on principle and would leave the next PR — and the next prod fix — hitting the same wall.
Frequency looks materially higher than "occasional": three hits in roughly one hour, against
mainstaying green on the same shard across its last three runs. If contention on this runner class is what makes the window reachable, it is reachable often enough now to be a merge blocker rather than a nuisance.See the note above re: shared cause with #2339 — both guards live in
src/MeshWeaver.Data/DataExtensions.cs, and two recent stream-lifecycle commits landed there.rbuergi commentedon Aug 26, 2026
🚨 Correction — this is NOT a flake. The test verified, and the notification was genuinely dropped.
I previously described this as a flake and argued for re-running to unblock the prod fix. That was wrong, and the evidence is in which assertion fired.
The test has two failure modes, deliberately separated:
Assert.False(adapter.GateTimedOut, …)added.Wait(10s)@ line 216The failure is at line 216 — the second one. The gate assertion passed, so the scope-walk gate was released as intended and the Initial stayed pinned relative to the notification's fan-out snapshot. The run held its window and verified.
The test's own message states why waiting longer is not the answer:
Conclusion:
b74277304narrowed the early→live handoff window but did not close it. Under contention the notification is still lost, and the loss is permanent — a row that a point read returns immediately never reaches the live subscription.Do not re-run to green. That would ship a real data-loss defect. The gate added by
608f0335dis what makes this distinguishable at all — without it, this failure and an environmental timeout would look identical, and the natural reading would have been "flaky, retry".Frequency remains notable: three hits within an hour on PRs that cannot touch it (#2328, #2334, #2333). Given it is real, that is a defect reachable often enough to block merges — including the fix for the current production outage.
rbuergi commentedon Aug 26, 2026
Narrowing — three plausible windows checked and CLOSED. Recording so nobody re-walks them.
All line refs:
src/MeshWeaver.Hosting/Persistence/Query/StorageAdapterMeshQueryProvider.cs.1.
initialDone == truewhileliveBufferis still null → silent drop. The routing handler hasClosed. Both are assigned in one critical section (1418–1423), and
liveBuffer = changeBufferprecedesinitialDone = true. SoinitialDone⇒liveBuffer != null, and thatreturnis unreachable.2.
changeBufferis a plainSubject, so anything pushed before a downstream subscriber attaches is dropped. Closed. The consuming pipeline (changeBuffer.Select(_ => RunQuery()).Concat().Subscribe(...)) is subscribed at 1408, before the lock at 1418. The buffer already has its subscriber when it becomes reachable.3. Backlog drained into a buffer nobody reads yet. Closed by the same ordering — the backlog is pushed at 1445, well after the 1408 subscribe.
So the single-subscription redesign does hold at the routing layer: every notification lands in the backlog or the live buffer, never between them.
Which means the loss is further downstream — after
live.OnNext(n)succeeds. Remaining candidates, none verified:scopeFilters.Any(sf => PathMatcher.ShouldNotify(n.Path, sf.BasePath, sf.Scope))(1317);RunQuery()'s re-read does not observe the write (read-your-writes against the adapter);ProcessBatchobserves the row but does not emit anAddedchange for it.The test asserts on
added.Set(), which fires only on an emitted change — so a re-query that ran and found nothing looks identical to a notification that never arrived. Instrumenting betweenlive.OnNextandProcessBatchwould separate those three, and that is the next step I would take.Not a diagnosis. Stopping here rather than guessing, since the previous fix's comment shows this was already reasoned through carefully once and still left a hole.
rbuergi commentedon Aug 26, 2026
Closing as resolved by #2337 (merged) — and the hypothesis in this ticket was specifically disproven by measurement, which is worth recording rather than just closing.
This is the same test as #2319, filed at 08:46 today, a few hours before the fix landed. Both tickets reasoned the same way, and it is good reasoning: the test is the regression guard for
b74277304's early→live handoff race, it failed on a diff that cannot touch a live query, and the test's own docstring says "a permanent miss dressed as a timeout, which is why raising a budget could never fix it." I escalated #2319 twice on exactly that basis.It was cold-JIT, not the handoff race. The discriminating experiment:
A logic race fails sometimes when warm. This never does. The failures were the one-time JIT/type-load cost of
StorageAdapterMeshQueryProvider's reactive pipeline (ObserveQueryInternal/CollectMatched/FindMatchingNodes/ProcessBatchplus their generic Rx instantiations) exceeding the 10 s budget on the first call in a fresh process.So the handoff fix from
b74277304is correct and untouched. #2337 changed no production code and widened none of the three 10 s budgets — it pays the cold cost on throwaway objects before the timed assertions begin, removing a confound rather than relaxing an assertion. Verified onmain:WarmUpJitAndTypeLoadis present inLiveQueryHandoffDropTest.cs.The general lesson, since three of us reached the wrong conclusion from it: a test's docstring explains the defect it was WRITTEN for; it is not diagnostic of a later failure of that test. And a failure count tells you how often, never why. Frequency justified investigating — it never justified the diagnosis.
If this test flakes again after
e2080c4fa, that would be new information and worth a fresh ticket with the run link — the warm-up removes the known cost path, so a post-fix failure would not have this explanation.