Skip to content

🧪 test(storage): share one capture subscriber - #2191

Open
gaborbernat wants to merge 1 commit into
mainfrom
fix/webhook-capture-flake-2188
Open

🧪 test(storage): share one capture subscriber#2191
gaborbernat wants to merge 1 commit into
mainfrom
fix/webhook-capture-flake-2188

Conversation

@gaborbernat

@gaborbernat gaborbernat commented Sep 4, 2026

Copy link
Copy Markdown
Member

test_webhook_queue_scan_skips_and_cleans_damaged_rows failed on some runs and not others, a different one of its three cases each time, on an unchanged tree. The count of the cleanup warning came back as zero, and the scan does emit it on every pass that finds damage, so something dropped the event on the way rather than the scan skipping it.

A callsite decides its interest the first time any thread executes it, and every thread reads that decision from then on. 🔍 While one dispatcher is alive, tracing resolves that question against the registering thread's own default rather than against the registry, so a thread holding no subscriber resolves it against nothing, caches Interest::never(), and silences the callsite for whichever case was asserting on it. A subscriber that lives only as long as one test is absent whenever a neighbour asks first, and which case loses that race is what varied between runs.

One subscriber for the whole binary cannot be the one missing at that moment. Events route to a capture belonging to the thread that raised them, so the cases still read only their own output while sharing the decision each callsite makes once. Serialising them would have hidden the interleaving rather than removing it, and it would leave the next test that captures logs in this crate to rediscover the same thing.

The assertion holds as it was, one warning across two scans, which is what proves the second scan found nothing left to clean. Nothing in WebhookQueueCleanup::log changed.

The new test drives the interleaving rather than waiting for it to come around. A thread holding no subscriber reaches a callsite first, and the event a subscribed thread raises at that callsite still reaches its capture. Swapping the shared subscriber back for a per-test one makes that capture come back empty, which is the failure this fixes.

Closes #2188

@gaborbernat gaborbernat added the bug Something isn't working label Sep 4, 2026
The three webhook queue-scan cases assert on captured log output, and one of them
failed on some runs and not others, a different case each time, with the count of
the cleanup warning reading zero on an unchanged tree.

A callsite decides its interest the first time any thread executes it, and every
thread reads that decision afterwards. While one dispatcher is alive, tracing
resolves that question against the registering thread's own default rather than
the registry, so a thread holding no subscriber resolves it against nothing,
caches never, and silences the callsite for the tests asserting on it. A
subscriber that lives only as long as one test is absent whenever a neighbour
asks first.

One subscriber for the binary cannot be the one missing at that moment, and
routing by thread keeps each case reading only its own events. Serialising the
cases would have hidden the interleaving rather than removing it, and the
assertion stays as it was: one warning across two scans, which is what proves the
second scan found nothing left to clean.

The new test drives the interleaving rather than waiting for it. A thread with no
subscriber reaches a callsite first, and the event a subscribed thread raises
there still lands in its capture. With a per-test subscriber that capture comes
back empty.
@gaborbernat
gaborbernat force-pushed the fix/webhook-capture-flake-2188 branch from 95da23e to 0984a82 Compare September 4, 2026 11:13
@codspeed-hq

codspeed-hq Bot commented Sep 4, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 29 untouched benchmarks
⏩ 133 skipped benchmarks1


Comparing fix/webhook-capture-flake-2188 (0984a82) with main (d75b16a)

Open in CodSpeed

Footnotes

  1. 133 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛 Flaky: webhook queue scan test loses its log capture

1 participant