Skip to content

Complete a tracked session only when ALL conditions are satisfied (GH-3824) - #3828

Merged
jeremydmiller merged 1 commit into
mainfrom
gh-3763/tracked-session-all-conditions
Aug 4, 2026
Merged

Complete a tracked session only when ALL conditions are satisfied (GH-3824)#3828
jeremydmiller merged 1 commit into
mainfrom
gh-3763/tracked-session-all-conditions

Conversation

@jeremydmiller

Copy link
Copy Markdown
Member

Closes #3824.

TrackedSession.IsCompleted() short-circuited on the first satisfied condition instead of requiring all of them:

if (_conditions.Any(x => x.IsCompleted())) return true;     // ANY
...
return !_conditions.Any() || _conditions.All(x => x.IsCompleted());   // ALL — unreachable

With a single condition Any and All are identical, which is why this survived: nearly every session registers just one.

Wolverine.Tracking is public test-support API, so this is not only our problem — a user chaining two WaitForMessageToBeReceivedAt calls gets a session that returns when either host handles the message, then asserts against handlers still running. A test that passes locally and fails under load, pointing nowhere near its cause.

There was already a workaround implying someone hit this and routed around it rather than fixing it: WaitForExecutionOf<T> deliberately merges repeat calls into a single condition object, and its doc comment says so — "Multiple calls combine into a single condition that requires every registered count to be reached." That is only necessary because multiple conditions don't AND together.

How it surfaced, and a refuted hypothesis

Wolverine.RabbitMQ.Tests.end_to_end carried [Trait("Category", "Flaky")], excluding 20 tests from CI. Its two persistent failures each chain three WaitForMessageToBeReceivedAt calls for a fan-out exchange.

The in-file triage had every symptom right — both pass alone, fail in-class, fail in ~500ms on a null ColorHistory nowhere near the 30s timeout — and then inferred the cause: that the wait is satisfied by MessageFailed, so a message that arrived and then failed ended the session early. It also recorded that the next step was to dump the session rather than infer.

Doing that dump refutes the inference outright:

PROBE status=Completed exceptions=0
Sent ColorChosen to rabbitmq://exchange/exchange-...
Received ColorChosen at .../messages-...-20e23   (node 5321b23d)
Received ColorChosen at .../messages-...-21e23   (node 7d6a0afc)
Received ColorChosen at .../messages-...-22e23   (node ee1d1ae4)
Started execution   (node ee1d1ae4)
Finished execution  (node ee1d1ae4)
marked as successful (node ee1d1ae4)

Nothing failed. The message succeeded — at exactly one of the three receivers, whose condition alone satisfied Any. Alone on an idle machine all three finish inside the same millisecond; in-class under load the other two lag and their ColorHistory is still null at assertion time.

Verification — baselined, not assumed

Because this changes shared test infrastructure, the full RabbitMQ project was run on both sides:

main this branch
end_to_end.use_fan_out_exchange FAIL pass
end_to_end.use_direct_exchange_with_binding_key FAIL pass
ConventionalRouting…send_from_one_node_to_another… FAIL FAIL
multi_tenancy_through_virtual_hosts.send_message_to_a_specific_tenant FAIL FAIL
total 4 / 491 2 / 491

The fix removes exactly its two targets and adds nothing. The two survivors are the known tracked-debt pair, failing identically on both sides.

  • CoreTests: 2246 passed, 0 failed
  • end_to_end class alone: 20/20 on three consecutive runs
  • 163 WaitForMessageToBeReceivedAt call sites repo-wide; exactly one file chains more than one
  • One documentation sample mixes WaitForMessageToBeReceivedAt with WaitForExecutionOf; its comments describe wanting both, so the fix makes it match its own documentation
  • Full wolverine.slnx Release build clean, 0 warnings

Reviewers: the failure mode to watch for on CI is a timeout, not an assertion — a session that now correctly waits for something that never arrives.

Also in here

The Category=Flaky tag comes off end_to_end (20 tests back into CIRabbitMQ), and the three Azure Service Bus "Needs its own issue" notes now point at the issues finally filed for them — #3825, #3826, #3827. Those were diagnosed product defects sitting behind Flaky tags with nothing tracking them.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WHAuhdWS3XeAk16swV9G8m

…-3824)

TrackedSession.IsCompleted() short-circuited on the first satisfied condition
rather than requiring all of them:

    if (_conditions.Any(x => x.IsCompleted())) return true;
    ...
    return !_conditions.Any() || _conditions.All(x => x.IsCompleted());

which made the All(...) on the last line unreachable whenever any single
condition was satisfied. With one condition ANY and ALL are identical, which is
why this survived -- nearly every session registers just one.

Wolverine.Tracking is public test-support API, so this is not only our problem:
a user chaining two WaitForMessageToBeReceivedAt calls gets a session that
returns when EITHER host handles the message and then asserts against handlers
that may still be running. There was already a workaround implying someone hit
this and routed around it -- WaitForExecutionOf<T> deliberately merges repeat
calls into one condition object, and its doc comment says so in as many words.

Found via Wolverine.RabbitMQ.Tests.end_to_end, tagged Category=Flaky and
therefore excluded from CI. Its two persistent failures each chain three
WaitForMessageToBeReceivedAt calls for a fan-out exchange, so the session
returned as soon as the FIRST of the three receivers handled the message.

The recorded triage had the symptoms right (both pass alone, fail in-class, fail
in ~500ms on a null ColorHistory nowhere near the 30s timeout) but inferred the
cause: that WaitForMessageToBeReceivedAt is satisfied by MessageFailed, so a
message that arrived and then failed ended the session. Dumping the session --
the next step that triage itself prescribed -- refutes that: status=Completed,
ZERO exceptions, message marked successful, at exactly one of three receivers.

Measured, not assumed:

  full Wolverine.RabbitMQ.Tests on main   4 failed / 491
  full Wolverine.RabbitMQ.Tests on branch 2 failed / 491

The two that remain are the known tracked-debt flakes
(ConventionalRouting.send_from_one_node_to_another_all_with_conventional_routing
and multi_tenancy_through_virtual_hosts.send_message_to_a_specific_tenant),
failing identically on both sides. CoreTests: 2246 passed, 0 failed. Of 163
WaitForMessageToBeReceivedAt call sites, exactly one file chains more than one.

The Category=Flaky tag comes off end_to_end, returning 20 tests to CIRabbitMQ.

Also points the three Azure Service Bus "needs its own issue" notes at the
issues now filed for them: GH-3825, GH-3826, GH-3827.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WHAuhdWS3XeAk16swV9G8m
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TrackedSession completes on ANY satisfied condition, not ALL — multi-condition tracked sessions return early

1 participant