You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "lhotari (via GitHub)" <gi...@apache.org> on 2023/05/29 12:58:23 UTC
[GitHub] [pulsar] lhotari opened a new pull request, #20431: [fix][test] Improve deleteNamespaceWithRetry
lhotari opened a new pull request, #20431:
URL: https://github.com/apache/pulsar/pull/20431
### Motivation
There are this type of flaky test failures:
https://github.com/apache/pulsar/actions/runs/5111571116/jobs/9188966524?pr=20428#step:10:1449
```
Error: Tests run: 83, Failures: 1, Errors: 0, Skipped: 50, Time elapsed: 629.104 s <<< FAILURE! - in org.apache.pulsar.broker.admin.AdminApi2Test
Error: org.apache.pulsar.broker.admin.AdminApi2Test.resetClusters Time elapsed: 5.296 s <<< FAILURE!
org.testcontainers.shaded.org.awaitility.core.ConditionTimeoutException: Condition with org.apache.pulsar.broker.auth.MockedPulsarServiceBaseTest was not fulfilled within 5 seconds.
at org.testcontainers.shaded.org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
at org.testcontainers.shaded.org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
at org.testcontainers.shaded.org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
at org.testcontainers.shaded.org.awaitility.core.ConditionFactory.until(ConditionFactory.java:985)
at org.testcontainers.shaded.org.awaitility.core.ConditionFactory.until(ConditionFactory.java:954)
at org.apache.pulsar.broker.auth.MockedPulsarServiceBaseTest.deleteNamespaceWithRetry(MockedPulsarServiceBaseTest.java:618)
at org.apache.pulsar.broker.auth.MockedPulsarServiceBaseTest.deleteNamespaceWithRetry(MockedPulsarServiceBaseTest.java:609)
at org.apache.pulsar.broker.admin.AdminApi2Test.resetClusters(AdminApi2Test.java:178)
at jdk.internal.reflect.GeneratedMethodAccessor413.invoke(Unknown Source)
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.MethodInvocationHelper.invokeMethodConsideringTimeout(MethodInvocationHelper.java:69)
at org.testng.internal.invokers.ConfigInvoker.invokeConfigurationMethod(ConfigInvoker.java:361)
at org.testng.internal.invokers.ConfigInvoker.invokeConfigurations(ConfigInvoker.java:296)
at org.testng.internal.invokers.TestInvoker.runConfigMethods(TestInvoker.java:823)
at org.testng.internal.invokers.TestInvoker.runAfterConfigurations(TestInvoker.java:792)
at org.testng.internal.invokers.TestInvoker.invokeMethod(TestInvoker.java:768)
at org.testng.internal.invokers.TestInvoker.invokeTestMethod(TestInvoker.java:221)
at org.testng.internal.invokers.MethodRunner.runInSequence(MethodRunner.java:50)
at org.testng.internal.invokers.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:969)
at org.testng.internal.invokers.TestInvoker.invokeTestMethods(TestInvoker.java:194)
at org.testng.internal.invokers.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:148)
at org.testng.internal.invokers.TestMethodWorker.run(TestMethodWorker.java:128)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
at org.testng.TestRunner.privateRun(TestRunner.java:829)
at org.testng.TestRunner.run(TestRunner.java:602)
at org.testng.SuiteRunner.runTest(SuiteRunner.java:437)
at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:431)
at org.testng.SuiteRunner.privateRun(SuiteRunner.java:391)
at org.testng.SuiteRunner.run(SuiteRunner.java:330)
at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:95)
at org.testng.TestNG.runSuitesSequentially(TestNG.java:1256)
at org.testng.TestNG.runSuitesLocally(TestNG.java:1176)
at org.testng.TestNG.runSuites(TestNG.java:1099)
at org.testng.TestNG.run(TestNG.java:1067)
at org.apache.maven.surefire.testng.TestNGExecutor.run(TestNGExecutor.java:155)
at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.executeSingleClass(TestNGDirectoryTestSuite.java:102)
at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.executeLazy(TestNGDirectoryTestSuite.java:117)
at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.execute(TestNGDirectoryTestSuite.java:86)
at org.apache.maven.surefire.testng.TestNGProvider.invoke(TestNGProvider.java:137)
at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:385)
at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:162)
at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:507)
at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:495)
```
Since the exception is swallowed, it's hard to find out the reason. This PR attempts to fix the issue.
### Modifications
- don't fail when the namespace is already deleted
- don't swallow the last exception
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc` <!-- Your PR contains doc changes. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #20431: [fix][test] Improve deleteNamespaceWithRetry
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #20431:
URL: https://github.com/apache/pulsar/pull/20431#discussion_r1209720907
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/MockedPulsarServiceBaseTest.java:
##########
@@ -615,13 +615,12 @@ public static void deleteNamespaceWithRetry(String ns, boolean force, PulsarAdmi
*/
public static void deleteNamespaceWithRetry(String ns, boolean force, PulsarAdmin admin,
Collection<PulsarService> pulsars) throws Exception {
- Awaitility.await().atMost(5, TimeUnit.SECONDS).until(() -> {
+ Awaitility.await().ignoreExceptions().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> {
Review Comment:
I added a minimum backoff of 500 milliseconds and added explicit logging. @tisonkun PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #20431: [fix][test] Improve deleteNamespaceWithRetry
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #20431:
URL: https://github.com/apache/pulsar/pull/20431#discussion_r1209347813
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/MockedPulsarServiceBaseTest.java:
##########
@@ -615,13 +615,12 @@ public static void deleteNamespaceWithRetry(String ns, boolean force, PulsarAdmi
*/
public static void deleteNamespaceWithRetry(String ns, boolean force, PulsarAdmin admin,
Collection<PulsarService> pulsars) throws Exception {
- Awaitility.await().atMost(5, TimeUnit.SECONDS).until(() -> {
+ Awaitility.await().ignoreExceptions().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> {
try {
// Maybe fail by race-condition with create topics, just retry.
admin.namespaces().deleteNamespace(ns, force);
- return true;
- } catch (Exception ex) {
- return false;
+ } catch (PulsarAdminException.NotFoundException ex) {
+ // topic was already deleted, ignore exception
Review Comment:
```suggestion
// namespace was already deleted, ignore exception
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on a diff in pull request #20431: [fix][test] Improve deleteNamespaceWithRetry
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #20431:
URL: https://github.com/apache/pulsar/pull/20431#discussion_r1209581272
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/MockedPulsarServiceBaseTest.java:
##########
@@ -615,13 +615,12 @@ public static void deleteNamespaceWithRetry(String ns, boolean force, PulsarAdmi
*/
public static void deleteNamespaceWithRetry(String ns, boolean force, PulsarAdmin admin,
Collection<PulsarService> pulsars) throws Exception {
- Awaitility.await().atMost(5, TimeUnit.SECONDS).until(() -> {
+ Awaitility.await().ignoreExceptions().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> {
Review Comment:
> don't swallow the last exception
From the wiki page linked, it says only the exception will be evaluated to `false`. How do you expose the exception with this method call differs from what stacktrace we get currently?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari merged pull request #20431: [fix][test] Improve deleteNamespaceWithRetry
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari merged PR #20431:
URL: https://github.com/apache/pulsar/pull/20431
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #20431: [fix][test] Improve deleteNamespaceWithRetry
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #20431:
URL: https://github.com/apache/pulsar/pull/20431#discussion_r1209688576
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/MockedPulsarServiceBaseTest.java:
##########
@@ -615,13 +615,12 @@ public static void deleteNamespaceWithRetry(String ns, boolean force, PulsarAdmi
*/
public static void deleteNamespaceWithRetry(String ns, boolean force, PulsarAdmin admin,
Collection<PulsarService> pulsars) throws Exception {
- Awaitility.await().atMost(5, TimeUnit.SECONDS).until(() -> {
+ Awaitility.await().ignoreExceptions().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> {
Review Comment:
you are right. I'll add explicit logging of exceptions. My assumption is that it's the `PulsarAdminException.NotFoundException` which caused this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org