Improve SQS tests resilience under concurrent load - #1678
Merged
Conversation
The test asserted the throughput mode from inside the polling callback, which races with the adaptation it checks: the mode is raised by the release that follows a partial fetch and lowered again by the next empty poll, so a poll can observe either value. It failed on CI, where it ran for the first time after the Abstract* surefire exclusion was lifted. Record the mode right after each release instead, on the thread that performed it, and assert which release caused which transition. Sampling for the raised mode is not an option either, since it only lasts until the next poll. (cherry picked from commit 62f46d7)
Several tests wait a fixed time for work to finish, with budgets that only hold when the machine is fast: 10 seconds for 100 messages that need at least a second of sleep, 5 seconds for a semaphore handed over by another thread, a 21 party barrier, and a drain loop polling for in flight messages. Under a busy CI runner they expire and the test fails while the code under test is correct. Widen those, which costs no time when they pass, and wait on the latches in the SNS tests directly rather than blocking on them inside an awaitility block, where the outcome depends on which of the two budgets runs out first. The assertions that check an event does not occur keep their short timeouts, since those are spent on every run.
A latch waited ten minutes and another a thousand seconds. Neither costs time while the test passes, but on failure they burn the job's budget instead of reporting it, and the work they wait on completes in seconds. Thirty seconds is well clear of a loaded runner.
…tion The test indexes the second 'receive' observation context, which only exists after the message is redelivered. Awaitility retries AssertionError only, so indexing an absent element left the wait with an ArrayIndexOutOfBoundsException rather than another attempt: the test failed on a loaded runner while the redelivery was still in flight.
Same reasoning as the deadlines commit, applied to the classes it did not reach: eighty two presence assertions still gave the work ten seconds, which holds on an idle machine and does not on a loaded runner. A latch returns as soon as it is counted down, so a green run takes exactly as long either way. The two assertions that check an event does not occur keep their short timeouts, since those are spent on every run.
tomazfernandes
requested review from
MatejNedic and
maciejwalkowiak
as code owners
August 9, 2026 16:37
tomazfernandes
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The SQS test suite runs its classes and methods concurrently against a single
Localstack container, so on a busy CI runner the work takes longer than it does
on an idle machine. Several tests give that work a fixed budget that only holds
in the fast case, and they may fail while the code under test is correct. This
fixes the ones that were observed failing and the pattern behind them.
Test code only. No production change, and no test is removed or weakened: every
assertion still checks the same condition.
Most of the suite waits on a latch with a fixed budget, and a latch returns as
soon as it is counted down, so a green run takes exactly as long whether the
budget is ten seconds or sixty. The budget is only a ceiling on how long we are
willing to wait before calling it a failure, and ten seconds is below what a
loaded runner needs.
Presence assertions move to sixty seconds. The two assertions that check an
event does not occur keep their short timeouts, since those are spent in full
on every green run.
One latch waited ten minutes and another a thousand seconds. Neither costs time
while the test passes, but on failure they burn the job's budget rather than
reporting it, and the work they wait on completes in seconds. Both are now
thirty seconds.
Result
Three consecutive runs green across all three JDKs.