[fix][test] Fix flaky ExtensibleLoadManagerImplTest.initializeState by recovering wedged channel ownership#25977
Merged
Conversation
…y recovering wedged channel ownership
The initializeState @BeforeMethod times out (ConditionTimeoutException after
120s) when it runs right after a role-churning test such as
testRoleChangeIdempotency, whose direct playLeader()/playFollower() calls can
leave the channel system topic (loadbalancer-service-unit-state) owned by a
broker that no longer serves it ("not served by this instance, redo the lookup"),
with the channel producer stuck in escalating reconnect backoff. The existing
monitor()+unload retry cannot heal this because monitor() only reconciles broker
roles, which are already consistent in this state.
When the fast unload retry does not recover, force a clean channel owner via
leader re-election: closing the current owner's LeaderElectionService makes the
other broker win the election, and its playLeader() re-creates and re-serves the
channel topic while the ownership change makes clients redo their stale lookups.
Assisted-by: Claude Code (Opus 4.8)
Member
Author
|
might not be needed after #25976 |
…itializestate # Conflicts: # pulsar-broker/src/test/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManagerImplBaseTest.java
merlimat
approved these changes
Jun 8, 2026
lhotari
added a commit
that referenced
this pull request
Jun 10, 2026
…y recovering wedged channel ownership (#25977)
lhotari
added a commit
that referenced
this pull request
Jun 10, 2026
…y recovering wedged channel ownership (#25977)
priyanshu-ctds
pushed a commit
to datastax/pulsar
that referenced
this pull request
Jun 15, 2026
…y recovering wedged channel ownership (apache#25977) (cherry picked from commit 476881f)
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
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.
Builds on #25976, which first hardened
initializeStateagainst channel disruption by force-servingthe channel topic (admin lookup +
getStats) on each unload attempt. This PR adds a strongerfallback for the case that force-serve alone cannot recover. (#25976 was merged while this PR was
open and is now merged into this branch.)
Motivation
ExtensibleLoadManagerImplTest.initializeState(a@BeforeMethod) intermittently fails withorg.awaitility.core.ConditionTimeoutException: Assertion condition null within 2 minutes.Root cause (confirmed from the failing run's test-report XML and thread dump): the failing
initializeStateruns immediately after a role-churning test such astestRoleChangeIdempotency,whose direct
playLeader()/playFollower()calls can leave the channel system topicpersistent://pulsar/system/loadbalancer-service-unit-statein an owns-but-doesn't-serve state.Clients looking it up get
"... not served by this instance:localhost:<port>. Please redo the lookup."(logged ~120× over the 120s window) while the channel's
ServiceUnitStatereports a differentowner, and the channel's internal producer falls into escalating reconnect backoff (observed up to
61s) and never recovers.
monitor()only reconciles broker roles (already consistent here), so theunload's channel publish keeps failing for the full window and Awaitility times out. (The
Assertion condition nullwording is just Awaitility reporting that only ignored / non-assertionexceptions occurred; the thread dump confirmed no thread was actually stuck.)
The force-serve heal from #25976 handles the common case (an owner is recorded but the bundle is not
loaded). This PR adds a stronger fallback for the harder wedge where the lookup keeps resolving a
broker that refuses to serve.
Modifications
ExtensibleLoadManagerImplBaseTest.initializeStatenow:awaitTestNamespaceUnloaded(30)— which drivesmonitor(), force-servesthe channel topic (the lookup +
getStatsheal from [fix][test] Fix flaky ExtensibleLoadManagerImplTest by re-serving the channel topic in initializeState #25976), and unloads the test namespace.recoverChannelOwnership()andretries the unload for 60s.
recoverChannelOwnership()forces a clean channel owner via leader re-election: it closes thecurrent channel owner's
LeaderElectionServiceso the other broker wins the election; that broker'splayLeader()re-creates and re-serves the channel topic, and the ownership change makes clients redotheir stale lookups. This reuses the exact LES-bounce pattern already used by
makePrimaryAsLeader().In the common (healthy) case the fast path succeeds immediately and the recovery never runs, so
behavior is unchanged.
A now-stale comment in
ExtensibleLoadManagerImplTest(describing theinitializeStatebackstop) isupdated to match.
Verifying this change
This change is already covered by existing tests (
ExtensibleLoadManagerImplTest, run in the flakygroup with
-PexcludedTestGroups=''). Verified locally:testRoleChangeIdempotency+testHandleNoChannelOwnerpass (bothserviceUnitStateTableViewClassNameparametrizations).initializeStateinvocation (LES bounce before each test method); all selected tests still passed, confirming the recovery is benign in a healthy cluster.spotlessJavaCheckandcheckstyleTestare green.The original wedge is timing-dependent and was not reproduced locally; confidence rests on the
artifact-confirmed root cause plus the above.
Does this pull request potentially affect one of the following parts:
This change is test-only (no production code changes).