[fix][test] Await reader reconnect after seek() to fix flaky TopicReaderTest assertions - #26141
Merged
Merged
Conversation
…derTest assertions Three reader tests assert reader.isConnected() immediately after seek() plus reading the messages a second time: testReaderIsAbleToSeekWithTimeOnBeginningOfTopic, testMultiReaderIsAbleToSeekWithTimeOnBeginningOfTopic and testReaderIsAbleToSeekWithMessageIdOnMiddleOfTopic. seek() triggers an asynchronous reconnect of the underlying consumer(s) -- the broker disconnects the consumer on cursor reset so it reconnects at the new position -- and for a multi-topic reader isConnected() requires all partition consumers to be connected at that instant, so the check can be transiently false right after the post-seek reads. Await the condition with Awaitility instead of asserting it instantly. The race is latent but surfaced frequently after apache#25338 migrated TopicReaderTest to SharedPulsarBaseTest: a shared, busier broker widens the seek->reconnect window. Assisted-by: Claude Code (Opus 4.8)
merlimat
approved these changes
Jul 2, 2026
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
2 participants
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.
Motivation
TopicReaderTest.testMultiReaderIsAbleToSeekWithTimeOnBeginningOfTopicfails intermittently in the"Pulsar CI Flaky" suite with
java.lang.AssertionError: expected [true] but found [false]atassertTrue(reader.isConnected())— observed on two unrelated PRs(run 28608140872 and
run 28612964177).
The assertion runs immediately after
seek()plus reading the messages a second time.seek()triggersan asynchronous reconnect of the underlying consumer(s): the broker disconnects the consumer on cursor
reset so it reconnects at the new position. For a multi-topic reader,
MultiTopicsConsumerImpl.isConnected()requires all partition consumers to be connected at thatinstant, so it can be transiently
falseright after the post-seek reads.The assertion has always been racy, but it started failing frequently after #25338 migrated the class
to
SharedPulsarBaseTest: a shared, busier broker widens the seek→reconnect window, so the latent racenow surfaces regularly.
Modifications
Await
reader.isConnected()with Awaitility instead of asserting it instantly, in the three tests thatshare the "seek() then assert isConnected()" pattern:
testReaderIsAbleToSeekWithTimeOnBeginningOfTopictestMultiReaderIsAbleToSeekWithTimeOnBeginningOfTopictestReaderIsAbleToSeekWithMessageIdOnMiddleOfTopicThis is a test-only change; no production code is modified.
Note: #17407 (
testMultiReaderIsAbleToSeekWithTimeOnMiddleOfTopic) is a different flake — that testhas no
isConnected()assertion — and is not addressed here.Verifying this change
This change is a test-only rework. The three affected tests plus the other seek-with-time reader tests
pass locally with the flaky test group enabled
(
-PtestGroups=flaky -PexcludedTestGroups=quarantine).Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes