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][test] Fix the flaky tests of ManagedLedgerImplUtilsTest #22611

Merged
merged 1 commit into from
Apr 29, 2024

Conversation

codelipenghui
Copy link
Contributor

Motivation

If the trim ledger happened before get the ManagedLedgerImplUtils.asyncGetLastValidPosition(), the returned message ID will not be the message ID returned to the writer because messages are deleted by the trimming task.

2024-04-29T09:29:18,911 - INFO  - [test-OrderedScheduler-0-0:PulsarMockBookKeeper@123] - Creating ledger 4
2024-04-29T09:29:18,911 - INFO  - [test-OrderedScheduler-0-0:ManagedLedgerImpl@1529] - [testReverseFindPositionOneByOne] Created new ledger 4
2024-04-29T09:29:18,919 - INFO  - [test-OrderedScheduler-1-0:OpAddEntry@231] - [testReverseFindPositionOneByOne] Closing ledger 4 for being full
2024-04-29T09:29:18,920 - INFO  - [bookkeeper-ml-scheduler-OrderedScheduler-1-0:ManagedLedgerImpl$17@2686] - [testReverseFindPositionOneByOne] End TrimConsumedLedgers. ledgers=1 totalSize=65
2024-04-29T09:29:18,921 - INFO  - [bookkeeper-ml-scheduler-OrderedScheduler-1-0:ManagedLedgerImpl$17@2693] - [testReverseFindPositionOneByOne] Removing ledger 3 - size: 55

java.lang.AssertionError:
Expected :3:4
Actual   :4:-1
<Click to see difference>


	at org.testng.Assert.fail(Assert.java:99)
	at org.testng.Assert.failNotEquals(Assert.java:1037)
	at org.testng.Assert.assertEqualsImpl(Assert.java:140)
	at org.testng.Assert.assertEquals(Assert.java:122)
	at org.testng.Assert.assertEquals(Assert.java:617)
	at org.apache.bookkeeper.mledger.util.ManagedLedgerImplUtilsTest.testGetLastValidPosition(ManagedLedgerImplUtilsTest.java:69)
	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.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
	at org.testng.internal.TestInvoker.invokeMethod(TestInvoker.java:599)
	at org.testng.internal.TestInvoker.invokeTestMethod(TestInvoker.java:174)
	at org.testng.internal.MethodRunner.runInSequence(MethodRunner.java:46)
	at org.testng.internal.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:822)
	at org.testng.internal.TestInvoker.invokeTestMethods(TestInvoker.java:147)
	at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:128)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.testng.TestRunner.privateRun(TestRunner.java:764)
	at org.testng.TestRunner.run(TestRunner.java:585)
	at org.testng.SuiteRunner.runTest(SuiteRunner.java:384)
	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:378)
	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:337)
	at org.testng.SuiteRunner.run(SuiteRunner.java:286)
	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:53)
	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:96)
	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1218)
	at org.testng.TestNG.runSuitesLocally(TestNG.java:1140)
	at org.testng.TestNG.runSuites(TestNG.java:1069)
	at org.testng.TestNG.run(TestNG.java:1037)
	at com.intellij.rt.testng.IDEARemoteTestNG.run(IDEARemoteTestNG.java:65)
	at com.intellij.rt.testng.RemoteTestNGStarter.main(RemoteTestNGStarter.java:105)

Modifications

Set retention policy to the managed ledger to avoid the data deletion before the verification of the test. Otherwise, the test will not cover the newly added method.

Verifying this change

It's a test change

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

Documentation

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

Copy link
Member

@dao-jun dao-jun left a comment

Choose a reason for hiding this comment

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

lgtm

@codelipenghui codelipenghui added this to the 3.3.0 milestone Apr 29, 2024
@codelipenghui codelipenghui merged commit 264722f into apache:master Apr 29, 2024
50 of 54 checks passed
codelipenghui added a commit that referenced this pull request Apr 29, 2024
codelipenghui added a commit that referenced this pull request Apr 29, 2024
codelipenghui added a commit that referenced this pull request Apr 29, 2024
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request May 13, 2024
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request May 16, 2024
hanmz pushed a commit to hanmz/pulsar that referenced this pull request Feb 12, 2025
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.

3 participants