Skip to content

[fix][test] Make KeyShared draining-hashes delivery test deterministic - #26602

Open
Dream95 wants to merge 2 commits into
apache:masterfrom
Dream95:fix_KeySharedSubscriptionTest
Open

Dream95 wants to merge 2 commits into
apache:masterfrom
Dream95:fix_KeySharedSubscriptionTest

Conversation

@Dream95

@Dream95 Dream95 commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Fixes #26568

Main Issue: #xyz

PIP: #xyz

Motivation

CI fails in KeySharedSubscriptionTest.resetDefaultNamespace with:
Topic has 8005 clients connected Including 8004 consumers, 0 producers, and 0 replicators.

Example: https://github.com/apache/pulsar/actions/runs/34130276934/job/101987362625

The leftover clients come from testMessageDeliveredFromDrainingHashes. That test used addConsumersUntilOwnerChanged, which kept creating Key_Shared consumers until consistent hashing moved testMessageKey off the original owner.
With subscriptionKeySharedConsistentHashingReplicaPoints=100 and a large hash space, a randomly named consumer often does not steal that hash. The loop can create thousands of consumers (8004 in the failure). The test then cannot finish cleanup before @AfterMethod, so resetDefaultNamespace cannot delete the topic.

Modifications

Replace addConsumersUntilOwnerChanged with addConsumerThatTakesOverHash. It picks a consumer name (takeover-N) that will own the target hash, subscribes that one consumer, and fails if the owner did not change.
findConsumerNameThatTakesHash builds a local ConsistentHashingStickyKeyConsumerSelector with the live replica-point count and selector.getKeyHashRange().getEnd().
Cleanup only has that extra consumer to close. findOwnerName uses selector.select(hash).

Verifying this change

  • Make sure that the change passes the CI checks.

This change is already covered by existing tests, such asKeySharedSubscriptionTest.testMessageDeliveredFromDrainingHashes.
Ran that method locally 500 times. All passed.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

@merlimat
merlimat requested a review from lhotari September 19, 2026 16:01

@lhotari lhotari left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reducing the consumer churn in this test. The new probe still has an input for which takeover is impossible, so the test is not yet deterministic. Please fix the initial consumer name and bound the search.

String name = "takeover-" + i;
if (!existingNames.add(name)) {
continue;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[INTENT MISMATCH] Choose a suitable initial name and bound the takeover search

The initial consumer still gets a random five-character name. With the valid name 00807, the modern selector's replica point 78 (00807 + NUL + 0 + NUL + 78) and testMessageKey both hash to slot 21672 with the default 100 points and range 1..65535. The existing ring point always wins: putIfAbsent retains its owner, and select uses ceilingEntry. No takeover-N can therefore succeed. This loop never terminates, and existingNames keeps every attempted name.

The collision also affected the old loop, but moving the search into a probe does not resolve this case of the flaky test. Please give the initial consumer a fixed suitable name (or choose a compatible pair), bound the search, and cover the exact-point case.

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.

Flaky-test: KeySharedSubscriptionTest

2 participants