-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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 clear transaction buffer snapshot flaky test #14922
[fix][test] Fix clear transaction buffer snapshot flaky test #14922
Conversation
@Demogorgon314:Thanks for your contribution. For this PR, do we need to update docs? |
/pulsarbot rerun-failure-checks |
@Demogorgon314:Thanks for providing doc info! |
/pulsarbot rerun-failure-checks |
0b73840
to
a62fadd
Compare
a62fadd
to
902d57d
Compare
@codelipenghui @BewareMyPower The CI is passed. Please take a look if you have time, thanks : ) |
### Motivation ``` java.lang.AssertionError: Expected :true Actual :false <Click to see difference> at org.testng.Assert.fail(Assert.java:99) at org.testng.Assert.failNotEquals(Assert.java:1037) at org.testng.Assert.assertTrue(Assert.java:45) at org.testng.Assert.assertTrue(Assert.java:55) at org.apache.pulsar.broker.transaction.TopicTransactionBufferRecoverTest.checkSnapshotCount(TopicTransactionBufferRecoverTest.java:452) at org.apache.pulsar.broker.transaction.TopicTransactionBufferRecoverTest.clearTransactionBufferSnapshotTest(TopicTransactionBufferRecoverTest.java:427) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132) // ... ``` Currently, the clear transaction buffer snapshot is flaky, because we used `@Cleanup` annotation, the `@Cleanup` annotation will put the close producer method to the bottom of the test method, then the producer will close after deleting the topic. When we delete the topic, all relevant producers will receive a `CLOSE_PRODUCER` command, then the producer client will try to reconnect to the broker, so it will send the `PRODUCER` command again, then the transaction buffer snapshot will take a new snapshot, so the test will fail. ### Modifications Make the producer close before the topic deleting.
### Motivation ``` java.lang.AssertionError: Expected :true Actual :false <Click to see difference> at org.testng.Assert.fail(Assert.java:99) at org.testng.Assert.failNotEquals(Assert.java:1037) at org.testng.Assert.assertTrue(Assert.java:45) at org.testng.Assert.assertTrue(Assert.java:55) at org.apache.pulsar.broker.transaction.TopicTransactionBufferRecoverTest.checkSnapshotCount(TopicTransactionBufferRecoverTest.java:452) at org.apache.pulsar.broker.transaction.TopicTransactionBufferRecoverTest.clearTransactionBufferSnapshotTest(TopicTransactionBufferRecoverTest.java:427) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132) // ... ``` Currently, the clear transaction buffer snapshot is flaky, because we used `@Cleanup` annotation, the `@Cleanup` annotation will put the close producer method to the bottom of the test method, then the producer will close after deleting the topic. When we delete the topic, all relevant producers will receive a `CLOSE_PRODUCER` command, then the producer client will try to reconnect to the broker, so it will send the `PRODUCER` command again, then the transaction buffer snapshot will take a new snapshot, so the test will fail. ### Modifications Make the producer close before the topic deleting.
### Motivation ``` java.lang.AssertionError: Expected :true Actual :false <Click to see difference> at org.testng.Assert.fail(Assert.java:99) at org.testng.Assert.failNotEquals(Assert.java:1037) at org.testng.Assert.assertTrue(Assert.java:45) at org.testng.Assert.assertTrue(Assert.java:55) at org.apache.pulsar.broker.transaction.TopicTransactionBufferRecoverTest.checkSnapshotCount(TopicTransactionBufferRecoverTest.java:452) at org.apache.pulsar.broker.transaction.TopicTransactionBufferRecoverTest.clearTransactionBufferSnapshotTest(TopicTransactionBufferRecoverTest.java:427) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132) // ... ``` Currently, the clear transaction buffer snapshot is flaky, because we used `@Cleanup` annotation, the `@Cleanup` annotation will put the close producer method to the bottom of the test method, then the producer will close after deleting the topic. When we delete the topic, all relevant producers will receive a `CLOSE_PRODUCER` command, then the producer client will try to reconnect to the broker, so it will send the `PRODUCER` command again, then the transaction buffer snapshot will take a new snapshot, so the test will fail. ### Modifications Make the producer close before the topic deleting. (cherry picked from commit 4ad8b5b)
### Motivation ``` java.lang.AssertionError: Expected :true Actual :false <Click to see difference> at org.testng.Assert.fail(Assert.java:99) at org.testng.Assert.failNotEquals(Assert.java:1037) at org.testng.Assert.assertTrue(Assert.java:45) at org.testng.Assert.assertTrue(Assert.java:55) at org.apache.pulsar.broker.transaction.TopicTransactionBufferRecoverTest.checkSnapshotCount(TopicTransactionBufferRecoverTest.java:452) at org.apache.pulsar.broker.transaction.TopicTransactionBufferRecoverTest.clearTransactionBufferSnapshotTest(TopicTransactionBufferRecoverTest.java:427) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132) // ... ``` Currently, the clear transaction buffer snapshot is flaky, because we used `@Cleanup` annotation, the `@Cleanup` annotation will put the close producer method to the bottom of the test method, then the producer will close after deleting the topic. When we delete the topic, all relevant producers will receive a `CLOSE_PRODUCER` command, then the producer client will try to reconnect to the broker, so it will send the `PRODUCER` command again, then the transaction buffer snapshot will take a new snapshot, so the test will fail. ### Modifications Make the producer close before the topic deleting. (cherry picked from commit 4ad8b5b)
Motivation
Currently, the clear transaction buffer snapshot is flaky, because we used
@Cleanup
annotation, the@Cleanup
annotation will put the close producer method to the bottom of the test method, then the producer will close after deleting the topic.When we delete the topic, all relevant producers will receive a
CLOSE_PRODUCER
command, then the producer client will try to reconnect to the broker, so it will send thePRODUCER
command again, then the transaction buffer snapshot will take a new snapshot, so the test will fail.Modifications
Make the producer close before the topic deleting.
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)