Dark Mode

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[fix][broker] Fix flaky testReplicatorsInflightTaskListIsEmptyAfterReplicationFinished#24590

Merged
lhotari merged 4 commits intoapache:masterfrom
poorbarcode:flaky/testReplicatorsInflightTaskListIsEmptyAfterReplicationFinished
Jul 30, 2025
Merged

[fix][broker] Fix flaky testReplicatorsInflightTaskListIsEmptyAfterReplicationFinished#24590
lhotari merged 4 commits intoapache:masterfrom
poorbarcode:flaky/testReplicatorsInflightTaskListIsEmptyAfterReplicationFinished

Conversation

Copy link
Contributor

poorbarcode commented Jul 30, 2025

Fixes #24583

Motivation & Modifications

fix the flaky test testReplicatorsInflightTaskListIsEmptyAfterReplicationFinished

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: x

poorbarcode added this to the 4.1.0 milestone Jul 30, 2025
poorbarcode self-assigned this Jul 30, 2025
poorbarcode added the ready-to-test label Jul 30, 2025
github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jul 30, 2025
poorbarcode added release/4.0.7 release/3.0.14 release/3.3.9 and removed doc-not-needed Your PR changes do not impact docs labels Jul 30, 2025
github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jul 30, 2025
lhotari reviewed Jul 30, 2025
poorbarcode requested review from BewareMyPower, Demogorgon314 and lhotari July 30, 2025 13:46
BewareMyPower previously requested changes Jul 30, 2025
Copy link
Contributor

BewareMyPower left a comment

Choose a reason for hiding this comment

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

There is one point that is not explained by the comments.

If the test in #24583 (comment) is added based on commit 671994f (the latest commit before #24189), the test will pass.

But if the test is added to master branch, it will always fail.

BewareMyPower dismissed their stale review July 30, 2025 14:05

Got the reason

...neWayReplicatorTest.java

Co-authored-by: Yunze Xu
poorbarcode requested a review from BewareMyPower July 30, 2025 14:08
Copy link
Contributor

BewareMyPower commented Jul 30, 2025

I got the reason now. After cluster A created a topic, the creation of the replicator producer will trigger the topic creation of cluster B. i.e. the following code has a great chance to fail:

final String topic = "persistent://" + replicatedNamespace + "/topic";
admin1.topics().createNonPartitionedTopic(topic);
Thread.sleep(1000);
admin2.topics().createNonPartitionedTopic(topic);

#24189 somehow speeds up the creation of the replication producer. BTW, I found the issue also from my improvements in topic loading. It should be the same reason.

Regarding the following test:

@Test
public void test() throws Exception {
final String topic1 = "persistent://" + replicatedNamespace + "/tp1";
admin1.topics().createPartitionedTopic(topic1, 2);
admin1.topicPolicies().setMessageTTL(topic1, 100);

final String topic2 = "persistent://" + replicatedNamespace + "/tp2";
admin2.topics().createNonPartitionedTopic(topic2);
admin2.topics().createSubscription(topic2, "sub", MessageId.earliest);

admin1.topics().createNonPartitionedTopic(topic2);
}

Question 1: why removing admin1.topicPolicies().setMessageTTL(topic1, 100) will make the test pass

Because after that, admin1.topics().createNonPartitionedTopic(topic2); will trigger the loading on topic __change_events, including the reader creation and the read loop, which could cost some time.

Question 2: why removing admin2.topics().createSubscription(topic2, "sub", MessageId.earliest); will make the test pass

Because creating a subscription will cost some time as well.

3pacccccc reacted with thumbs up emoji

BewareMyPower approved these changes Jul 30, 2025
lhotari approved these changes Jul 30, 2025
Copy link

codecov-commenter commented Jul 30, 2025 *
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests.
Project coverage is 74.29%. Comparing base (bbc6224) to head (887850a).
Report is 1250 commits behind head on master.

Additional details and impacted files

@@ Coverage Diff @@
## master #24590 +/- ##
============================================
+ Coverage 73.57% 74.29% +0.72%
- Complexity 32624 33027 +403
============================================
Files 1877 1880 +3
Lines 139502 146419 +6917
Branches 15299 16785 +1486
============================================
+ Hits 102638 108783 +6145
- Misses 28908 28995 +87
- Partials 7956 8641 +685
Flag Coverage D
inttests 26.80% (+2.22%)
systests 23.35% (-0.98%)
unittests 73.78% (+0.93%)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 1123 files with indirect coverage changes

New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

lhotari changed the title [fix][broker]Fix flaky test testReplicatorsInflightTaskListIsEmptyAfterReplicationFinished [fix][broker] Fix flaky test testReplicatorsInflightTaskListIsEmptyAfterReplicationFinished Jul 30, 2025
lhotari changed the title [fix][broker] Fix flaky test testReplicatorsInflightTaskListIsEmptyAfterReplicationFinished [fix][broker] Fix flaky testReplicatorsInflightTaskListIsEmptyAfterReplicationFinished Jul 30, 2025
lhotari merged commit 3ea8c26 into apache:master Jul 30, 2025
53 of 54 checks passed
lhotari pushed a commit that referenced this pull request Jul 30, 2025
...plicationFinished (#24590)

Co-authored-by: Yunze Xu
(cherry picked from commit 3ea8c26)
lhotari added the cherry-picked/branch-3.0 label Jul 30, 2025
lhotari pushed a commit that referenced this pull request Jul 30, 2025
...plicationFinished (#24590)

Co-authored-by: Yunze Xu
(cherry picked from commit 3ea8c26)
lhotari pushed a commit that referenced this pull request Jul 30, 2025
...plicationFinished (#24590)

Co-authored-by: Yunze Xu
(cherry picked from commit 3ea8c26)
gaozhangmin pushed a commit to gaozhangmin/pulsar that referenced this pull request Aug 13, 2025
poorbarcode added a commit to poorbarcode/pulsar that referenced this pull request Aug 14, 2025
ganesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Aug 20, 2025
...plicationFinished (apache#24590)

Co-authored-by: Yunze Xu
(cherry picked from commit 3ea8c26)
(cherry picked from commit ff86be1)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Aug 20, 2025
...plicationFinished (apache#24590)

Co-authored-by: Yunze Xu
(cherry picked from commit 3ea8c26)
(cherry picked from commit ff86be1)
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Aug 20, 2025
...plicationFinished (apache#24590)

Co-authored-by: Yunze Xu
(cherry picked from commit 3ea8c26)
(cherry picked from commit b8670b5)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Aug 26, 2025
...plicationFinished (apache#24590)

Co-authored-by: Yunze Xu
(cherry picked from commit 3ea8c26)
(cherry picked from commit b8670b5)
KannarFr pushed a commit to CleverCloud/pulsar that referenced this pull request Sep 22, 2025
walkinggo pushed a commit to walkinggo/pulsar that referenced this pull request Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

lhotari lhotari approved these changes

BewareMyPower BewareMyPower approved these changes

Demogorgon314 Awaiting requested review from Demogorgon314

Projects

None yet

Milestone

4.1.0

Development

Successfully merging this pull request may close these issues.

[Bug] OneWayReplicatorUsingGlobalPartitionedTest.testReplicatorsIn flightTaskListIsEmptyAfterReplicationFinished failed on master branch

4 participants