[Flaky-test] BacklogQuotaManagerTest#testConsumerBacklogEvictionTimeQuota #14000
Conversation
This file contains 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
|
@gaozhangmin:Thanks for your contribution. For this PR, do we need to update docs? |
gaozhangmin
changed the title
|
@gaozhangmin:Thanks for providing doc info! |
github-actions
bot
added
doc-not-needed
There was a problem hiding this comment.
@gaozhangmin - great find! Thanks for looking into this flaky test. I have a couple suggestions about additional assertions that might also be valuable.
pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BacklogQuotaManagerTest.java
Outdated
Show resolved
Hide resolved
|
/pulsarbot run-failure-checks |
|
@Jason918 PTAL |
Jason918
requested changes
Feb 10, 2022
pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BacklogQuotaManagerTest.java
Outdated
Show resolved
Hide resolved
gaozhangmin
force-pushed
the
flakyTest-testConsumerBacklogEvictionTimeQuota
branch
from
c8e854a to
2578c85
Compare
|
/pulsarbot run-failure-checks |
michaeljmarshall
approved these changes
Feb 10, 2022
|
/pulsarbot run-failure-checks |
nicoloboschi
approved these changes
Feb 11, 2022
There was a problem hiding this comment.
LGTM
Seen also today
2022-02-11T01:54:21.9467754Z [ERROR] testConsumerBacklogEvictionTimeQuota(org.apache.pulsar.broker.service.BacklogQuotaManagerTest) Time elapsed: 6.373 s <<< FAILURE!
2022-02-11T01:54:21.9468391Z java.lang.AssertionError: expected [4] but found [5]
2022-02-11T01:54:21.9468784Z at org.testng.Assert.fail(Assert.java:99)
2022-02-11T01:54:21.9471540Z at org.testng.Assert.failNotEquals(Assert.java:1037)
2022-02-11T01:54:21.9471998Z at org.testng.Assert.assertEqualsImpl(Assert.java:140)
2022-02-11T01:54:21.9472925Z at org.testng.Assert.assertEquals(Assert.java:122)
2022-02-11T01:54:21.9473422Z at org.testng.Assert.assertEquals(Assert.java:797)
2022-02-11T01:54:21.9473867Z at org.testng.Assert.assertEquals(Assert.java:807)
2022-02-11T01:54:21.9474520Z at org.apache.pulsar.broker.service.BacklogQuotaManagerTest.testConsumerBacklogEvictionTimeQuota(BacklogQuotaManagerTest.java:480)
2022-02-11T01:54:21.9475288Z at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
2022-02-11T01:54:21.9475861Z at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
2022-02-11T01:54:21.9476643Z at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
2022-02-11T01:54:21.9477241Z at java.base/java.lang.reflect.Method.invoke(Method.java:566)
2022-02-11T01:54:21.9477700Z at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
2022-02-11T01:54:21.9478377Z at org.testng.internal.InvokeMethodRunnable.runOne(InvokeMethodRunnable.java:45)
2022-02-11T01:54:21.9478873Z at org.testng.internal.InvokeMethodRunnable.call(InvokeMethodRunnable.java:73)
2022-02-11T01:54:21.9479353Z at org.testng.internal.InvokeMethodRunnable.call(InvokeMethodRunnable.java:11)
2022-02-11T01:54:21.9479857Z at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
2022-02-11T01:54:21.9480332Z at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
2022-02-11T01:54:21.9480920Z at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
2022-02-11T01:54:21.9481313Z at java.base/java.lang.Thread.run(Thread.java:829)
|
@merlimat This can be merged also. |
|
@gaozhangmin - thanks for fixing this flaky test! |
This was referenced Feb 28, 2022
nicoloboschi
pushed a commit
to datastax/pulsar
that referenced
this pull request
Mar 2, 2022
…uota (apache#14000) * fix flaky test * change to getCurrentLedgerEntries Co-authored-by: gavingaozhangmin <[email protected]> ### Motivation Fixes apache#13952 ### Modifications MAX_ENTRIES_PER_LEDGER = 5, it will be uncertain that the num of entries in last ledger is 4. Use ManagedLedgerImpl. getNumberOfEntries to get the num of entries in last opened ledger (cherry picked from commit 0ff2b8c)
Nicklee007
pushed a commit
to Nicklee007/pulsar
that referenced
this pull request
Apr 20, 2022
…uota (apache#14000) * fix flaky test * change to getCurrentLedgerEntries Co-authored-by: gavingaozhangmin <[email protected]> ### Motivation Fixes apache#13952 ### Modifications MAX_ENTRIES_PER_LEDGER = 5, it will be uncertain that the num of entries in last ledger is 4. Use ManagedLedgerImpl. getNumberOfEntries to get the num of entries in last opened ledger
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.
Motivation
Fixes #13952
Modifications
MAX_ENTRIES_PER_LEDGER = 5, it will be uncertain that the num of entries in last ledger is 4.
Use ManagedLedgerImpl. getNumberOfEntries to get the num of entries in last opened ledger
Documentation
Check the box below or label this PR directly (if you have committer privilege).
Need to update docs?
doc-required(If you need help on updating docs, create a doc issue)
no-need-doc(Please explain why)
doc(If this PR contains doc changes)