Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] [broker] fix flaky test PatternTopicsConsumerImplTest #21222

Conversation

poorbarcode
Copy link
Contributor

@poorbarcode poorbarcode commented Sep 21, 2023

Motivation

see: https://github.com/apache/pulsar/actions/runs/6257755998/job/16993960633?pr=21216

  Error:  org.apache.pulsar.client.impl.PatternTopicsConsumerImplTest.testAutoSubscribePatterConsumerFromBrokerWatcher  Time elapsed: 10.152 s  <<< FAILURE!
  org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a org.apache.pulsar.client.impl.PatternTopicsConsumerImplTest expected [4] but found [0] within 10 seconds.
  	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
  	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
  	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)
  	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:985)
  	at org.awaitility.core.ConditionFactory.untilAsserted(ConditionFactory.java:769)
  	at org.apache.pulsar.client.impl.PatternTopicsConsumerImplTest.testAutoSubscribePatterConsumerFromBrokerWatcher(PatternTopicsConsumerImplTest.java:617)
  	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
  	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
  	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
  	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
  	at org.testng.internal.invokers.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:139)
  	at org.testng.internal.invokers.InvokeMethodRunnable.runOne(InvokeMethodRunnable.java:47)
  	at org.testng.internal.invokers.InvokeMethodRunnable.call(InvokeMethodRunnable.java:76)
  	at org.testng.internal.invokers.InvokeMethodRunnable.call(InvokeMethodRunnable.java:11)
  	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
  	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
  	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
  	at java.base/java.lang.Thread.run(Thread.java:833)
  Caused by: java.lang.AssertionError: expected [4] but found [0]
  	at org.testng.Assert.fail(Assert.java:110)
  	at org.testng.Assert.failNotEquals(Assert.java:1577)
  	at org.testng.Assert.assertEqualsImpl(Assert.java:149)
  	at org.testng.Assert.assertEquals(Assert.java:131)
  	at org.testng.Assert.assertEquals(Assert.java:1418)
  	at org.testng.Assert.assertEquals(Assert.java:1382)
  	at org.testng.Assert.assertEquals(Assert.java:1428)
  	at org.apache.pulsar.client.impl.PatternTopicsConsumerImplTest.lambda$testAutoSubscribePatterConsumerFromBrokerWatcher$16(PatternTopicsConsumerImplTest.java:618)
  	at org.awaitility.core.AssertionCondition.lambda$new$0(AssertionCondition.java:53)
  	at org.awaitility.core.ConditionAwaiter$ConditionPoller.call(ConditionAwaiter.java:248)
  	at org.awaitility.core.ConditionAwaiter$ConditionPoller.call(ConditionAwaiter.java:235)
  	... 4 more

Root cause: If the new topic is created before the consumer's topic list watcher is connected, the watcher cannot accept the notifications of new topic creation.

Modifications

  • Make new topics creation start after the consumer's topic list watcher is connected
  • Since the config subscriptionPatternMaxLength's default value is 50, the pattern name in the test might be too long. So set subscriptionPatternMaxLength to a larger value.

Documentation

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

Matching PR in forked repository

PR in forked repository: x

@poorbarcode poorbarcode added this to the 3.2.0 milestone Sep 21, 2023
@poorbarcode poorbarcode self-assigned this Sep 21, 2023
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 21, 2023
@poorbarcode poorbarcode added ready-to-test and removed doc-not-needed Your PR changes do not impact docs labels Sep 21, 2023
@poorbarcode poorbarcode reopened this Sep 21, 2023
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 21, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #21222 (4d7378c) into master (66271e3) will increase coverage by 0.46%.
Report is 1 commits behind head on master.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #21222      +/-   ##
============================================
+ Coverage     72.73%   73.20%   +0.46%     
- Complexity    32465    32479      +14     
============================================
  Files          1875     1887      +12     
  Lines        139929   140118     +189     
  Branches      15413    15426      +13     
============================================
+ Hits         101781   102570     +789     
+ Misses        30067    29457     -610     
- Partials       8081     8091      +10     
Flag Coverage Δ
inttests 24.18% <ø> (+0.01%) ⬆️
systests 24.74% <ø> (?)
unittests 72.51% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 154 files with indirect coverage changes

@poorbarcode poorbarcode changed the title [fix] [broker] fix flaky test auto subscribe patter consumer from broker watcher [fix] [broker] fix flaky test PatternTopicsConsumerImplTest Sep 22, 2023
@Technoboy- Technoboy- merged commit be4bcac into apache:master Sep 23, 2023
70 of 74 checks passed
mattisonchao pushed a commit that referenced this pull request Oct 11, 2023
poorbarcode added a commit that referenced this pull request Oct 11, 2023
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Dec 20, 2023
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants