Skip to content

fix(outbox): skip the outstanding count when no limit is set - #4564

Merged
iancooper merged 6 commits into
BrighterCommand:masterfrom
guilhermeappel:fix/4554-skip-outstanding-count-without-limit
Oct 9, 2026
Merged

iancooper merged 6 commits into
BrighterCommand:masterfrom
guilhermeappel:fix/4554-skip-outstanding-count-without-limit

Conversation

@guilhermeappel

@guilhermeappel guilhermeappel commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Description

With the default AddProducers settings, MaxOutStandingMessages is -1 (no limit) and MaxOutStandingCheckInterval is zero, so CheckOutstandingMessages queued a background count after every Post and clear even though the count is never compared with a limit. Each count takes the process-wide semaphore and asks the outbox for the count, which for the in-memory outbox sorts every retained entry.

CheckOutstandingMessages now returns straight away when the limit is -1. Only -1 is skipped, the same value CheckOutboxOutstandingLimit treats as no limit: 0 is a real limit and still counts, and other negative values behave as before.

The XML docs for MaxOutStandingCheckInterval said it is ignored when the limit is 0, which is a real limit, and described the interval as measured since the last insertion. I corrected them on IAmProducersConfiguration, ProducersConfiguration and the maxOutStandingCheckInterval parameter of OutboxProducerMediator.

This is the first of the three PRs I described on issue #4554. The next one records the time before queuing the check and keeps at most one check in flight per mediator, and the last one drops the sort and the copy from the in-memory count. Once this is in, I'll bring PR #4552 up to date on top of it.

Related Issues

Part of issue #4554.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the Contributing Guide
  • I have checked the documentation for relevant guidance
  • I have added/updated XML documentation for any public API changes
  • I have added/updated tests as appropriate
  • My changes follow the existing code style and conventions

Additional Notes

  • Commits: the docs fix first, then two control tests that pass on the current code (with a limit of 100 and with a limit of 0 the mediator must still count), then the fix with the tests that post 20 times with no limit and assert the outbox is never asked for the count. Those failed before the fix with 20 counts each. The release note comes last.
  • I checked the control tests against two mutations of CheckOutstandingMessages that I didn't commit: returning at the top fails both, and returning when the limit is <= 0 fails the limit-0 one.
  • I put the release note at the end of the Master section rather than the top, so it doesn't conflict with the entries PR fix: TimedOutboxSweeper starvation and publish-confirm floods (#4560) #4562 and PR fix(service-activator)!: coordinate consumer startup and shutdown #4563 add there.
  • The Telemetry page in the docs shows a count.outstanding_messages span under the clear operation. With the defaults it no longer appears after this change. I can update the Docs repo once this ships.
  • The Core suite on net10.0 passes (1647 passed, 7 skipped) with this branch merged into current master.
  • Please merge with a merge commit rather than squash, as CONTRIBUTING asks.

The XML docs on IAmProducersConfiguration said the interval is ignored
when the limit is 0, the ones on ProducersConfiguration said it is
ignored for -1 or 0, and both described it as measured since the last
insertion into the outbox.

A limit of 0 is a real limit, so the interval still applies to it: once
a check has counted an outstanding message at least that old, the next
add to the outbox throws OutboxLimitReachedException. The interval is
the minimum time between background checks, measured from the previous
check, and a check starts after messages are cleared from the outbox,
which Post does straight after the deposit. It is also the minimum age
for a message to count as outstanding, as it is passed to the outbox as
dispatchedSince.

Correct the docs on both, and on the maxOutStandingCheckInterval
parameter of OutboxProducerMediator. What happens with -1 is left to the
fix later in this series, which stops the count for it.
Add CountingOutbox, an InMemoryOutbox that counts, thread-safely, how
often the mediator asks it for the outstanding message count, and a
builder that wires a CommandProcessor around it with an explicit limit
and check interval.

The two tests are positive controls for skipping the count when there is
no limit: with a limit of 100, and with a limit of 0 (a real limit, not
"no limit"), posting must still make the mediator count outstanding
messages. They pass on the current code. Each was seen to fail under a
temporary mutation of CheckOutstandingMessages, never committed:

- returning at the top: both tests fail
- returning at the top when the limit is <= 0: the limit-0 test fails,
  the limit-100 test passes
With the default AddProducers settings (MaxOutStandingMessages = -1 and
MaxOutStandingCheckInterval = TimeSpan.Zero) CheckOutstandingMessages
queued a background count after every Post and Clear, although with no
limit the count is never compared with a limit. Each count took the
process-wide semaphore and queried the outbox: a sort of every retained
entry for the in-memory outbox, a SELECT per Post for a relational one.

Return at the top of CheckOutstandingMessages when the limit is -1. Only
-1 returns early: 0 is a real limit and still counts, and values below
-1 keep their current behaviour. The XML docs for the interval now say
that it is ignored for -1, as the outbox is not consulted.

The new tests post 20 times through Post and PostAsync with a limit of
-1 and an interval of zero and assert that the outbox is never asked for
the count. They failed before the fix (20 counts each). A control
mediator with a limit gives the background work time to run before the
assertion.

Part of issue BrighterCommand#4554
Add a note under Master for the change to the outstanding message check
when MaxOutStandingMessages is -1, including the span and the debug log
lines that change.

Part of issue BrighterCommand#4554
@iancooper

Copy link
Copy Markdown
Member

Thanks @guilhermeappel I'm really excited about this, as across this set of changes, we should be able to address some problems we note at scale

@iancooper

Copy link
Copy Markdown
Member

Thanks @guilhermeappel

@iancooper
iancooper merged commit cc8d6e1 into BrighterCommand:master Oct 9, 2026
31 of 32 checks passed
iancooper pushed a commit that referenced this pull request Oct 10, 2026
The note that PR #4564 added for issue #4554 only covers the case with
no limit. Extend it with what changes when a limit is set: each
mediator keeps at most one check queued or running, and the time of
the last check is recorded when the check is queued. Retitle the entry
to match.

Also correct the semaphore the checks wait on: the note called it
process-wide, but it is a static field of the generic mediator, so it
is shared by every mediator with the same message and transaction
types.

Part of issue #4554
iancooper pushed a commit that referenced this pull request Oct 10, 2026
CheckOutstandingMessages returns before queuing a check when the limit
is -1, so OutstandingMessagesCheck never runs with no limit. The comment
that said the outbox's default ceiling applies with no limit has been
out of date since PR #4564 added the skip at -1.

Part of issue #4554
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 - Done Bug .NET Pull requests that update .net code V10.X

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants