You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/02/21 08:09:59 UTC
[GitHub] [pulsar] MarvinCai opened a new pull request #9659: [FlakyTest]Try to fix flaky test ReplicatorTest.testReplicatorOnPartitionedTopic.
MarvinCai opened a new pull request #9659:
URL: https://github.com/apache/pulsar/pull/9659
Fixes #9457
### Motivation
Not sure why state is not cleaned up but exception is indicating metadata about namespace the test trying to create already exist.
### Modifications
Append System.nanoTime to the namespace used to avoid being affect by previous state. This seems working fine for other test cases in the same test.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] MarvinCai commented on pull request #9659: [FlakyTest]Try to fix flaky test ReplicatorTest.testReplicatorOnPartitionedTopic.
Posted by GitBox <gi...@apache.org>.
MarvinCai commented on pull request #9659:
URL: https://github.com/apache/pulsar/pull/9659#issuecomment-782830400
> Usually that error is due to the test retry policy and it hides the real error.
>
> This fix cannot be enough to fix the flakyness.
>
> Do you have the real error in the test? That is the logs of the first run that failed?
@eolivelli Sample stack trace is
```
org.apache.pulsar.client.admin.PulsarAdminException$ConflictException: Namespace already exists
at org.apache.pulsar.client.admin.internal.BaseResource.getApiException(BaseResource.java:219)
at org.apache.pulsar.client.admin.internal.BaseResource$1.failed(BaseResource.java:129)
at org.glassfish.jersey.client.JerseyInvocation$1.failed(JerseyInvocation.java:839)
at org.glassfish.jersey.client.JerseyInvocation$1.completed(JerseyInvocation.java:820)
at org.glassfish.jersey.client.ClientRuntime.processResponse(ClientRuntime.java:229)
at org.glassfish.jersey.client.ClientRuntime.access$200(ClientRuntime.java:62)
at org.glassfish.jersey.client.ClientRuntime$2.lambda$response$0(ClientRuntime.java:173)
at org.glassfish.jersey.internal.Errors$1.call(Errors.java:248)
at org.glassfish.jersey.internal.Errors$1.call(Errors.java:244)
at org.glassfish.jersey.internal.Errors.process(Errors.java:292)
at org.glassfish.jersey.internal.Errors.process(Errors.java:274)
at org.glassfish.jersey.internal.Errors.process(Errors.java:244)
at org.glassfish.jersey.process.internal.RequestScope.runInScope(RequestScope.java:288)
at org.glassfish.jersey.client.ClientRuntime$2.response(ClientRuntime.java:173)
at org.apache.pulsar.client.admin.internal.http.AsyncHttpConnector.lambda$apply$1(AsyncHttpConnector.java:210)
at java.util.concurrent.CompletableFuture.uniWhenComplete(CompletableFuture.java:774)
at java.util.concurrent.CompletableFuture$UniWhenComplete.tryFire(CompletableFuture.java:750)
at java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:488)
at java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:1975)
at org.apache.pulsar.client.admin.internal.http.AsyncHttpConnector.lambda$retryOperation$3(AsyncHttpConnector.java:251)
at java.util.concurrent.CompletableFuture.uniWhenComplete(CompletableFuture.java:774)
at java.util.concurrent.CompletableFuture$UniWhenComplete.tryFire(CompletableFuture.java:750)
at java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:488)
at java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:1975)
at org.asynchttpclient.netty.NettyResponseFuture.loadContent(NettyResponseFuture.java:222)
at org.asynchttpclient.netty.NettyResponseFuture.done(NettyResponseFuture.java:257)
at org.asynchttpclient.netty.handler.AsyncHttpClientHandler.finishUpdate(AsyncHttpClientHandler.java:241)
at org.asynchttpclient.netty.handler.HttpHandler.handleChunk(HttpHandler.java:114)
at org.asynchttpclient.netty.handler.HttpHandler.handleRead(HttpHandler.java:143)
at org.asynchttpclient.netty.handler.AsyncHttpClientHandler.channelRead(AsyncHttpClientHandler.java:78)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:103)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
at io.netty.channel.CombinedChannelDuplexHandler$DelegatingChannelHandlerContext.fireChannelRead(CombinedChannelDuplexHandler.java:436)
at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:324)
at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:296)
at io.netty.channel.CombinedChannelDuplexHandler.channelRead(CombinedChannelDuplexHandler.java:251)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:163)
at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:714)
at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:650)
at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:576)
at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:493)
at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:989)
at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
at java.lang.Thread.run(Thread.java:748)
Caused by: javax.ws.rs.ClientErrorException: HTTP 409 Conflict
at org.glassfish.jersey.client.JerseyInvocation.createExceptionForFamily(JerseyInvocation.java:938)
at org.glassfish.jersey.client.JerseyInvocation.convertToException(JerseyInvocation.java:921)
at org.glassfish.jersey.client.JerseyInvocation.access$500(JerseyInvocation.java:77)
... 54 more
```
Make the namespace name unique for different execution/retry should able to get rid of this 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] MarvinCai commented on pull request #9659: [FlakyTest]Try to fix flaky test ReplicatorTest.testReplicatorOnPartitionedTopic.
Posted by GitBox <gi...@apache.org>.
MarvinCai commented on pull request #9659:
URL: https://github.com/apache/pulsar/pull/9659#issuecomment-785559999
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] 315157973 commented on a change in pull request #9659: [FlakyTest]Try to fix flaky test ReplicatorTest.testReplicatorOnPartitionedTopic.
Posted by GitBox <gi...@apache.org>.
315157973 commented on a change in pull request #9659:
URL: https://github.com/apache/pulsar/pull/9659#discussion_r581491759
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/BrokerTestUtil.java
##########
@@ -0,0 +1,28 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.broker;
+
+/**
+ * Holds util methods used in test.
+ */
+public class BrokerTestUtil {
+ // Generate unique namespace name for different test run.
+ public static String newUniqueNamespace(String prefix) { return prefix + "-" + System.nanoTime(); }
Review comment:
How about UUID
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui merged pull request #9659: [FlakyTest]Try to fix flaky test ReplicatorTest.testReplicatorOnPartitionedTopic.
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #9659:
URL: https://github.com/apache/pulsar/pull/9659
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] MarvinCai commented on pull request #9659: [FlakyTest]Try to fix flaky test ReplicatorTest.testReplicatorOnPartitionedTopic.
Posted by GitBox <gi...@apache.org>.
MarvinCai commented on pull request #9659:
URL: https://github.com/apache/pulsar/pull/9659#issuecomment-785619821
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] 315157973 commented on a change in pull request #9659: [FlakyTest]Try to fix flaky test ReplicatorTest.testReplicatorOnPartitionedTopic.
Posted by GitBox <gi...@apache.org>.
315157973 commented on a change in pull request #9659:
URL: https://github.com/apache/pulsar/pull/9659#discussion_r580198482
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java
##########
@@ -805,7 +805,7 @@ public void testReplicatorOnPartitionedTopic(boolean isPartitionedTopic) throws
log.info("--- Starting ReplicatorTest::{} --- ", methodName);
- final String namespace = "pulsar/partitionedNs-" + isPartitionedTopic;
+ final String namespace = "pulsar/partitionedNs-" + isPartitionedTopic + "-" + System.nanoTime();;
Review comment:
1）I searched the code, but I didn’t see a namespace with the same name in other places
2）There are two ";"
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] MarvinCai commented on a change in pull request #9659: [FlakyTest]Try to fix flaky test ReplicatorTest.testReplicatorOnPartitionedTopic.
Posted by GitBox <gi...@apache.org>.
MarvinCai commented on a change in pull request #9659:
URL: https://github.com/apache/pulsar/pull/9659#discussion_r580805247
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java
##########
@@ -805,7 +805,7 @@ public void testReplicatorOnPartitionedTopic(boolean isPartitionedTopic) throws
log.info("--- Starting ReplicatorTest::{} --- ", methodName);
- final String namespace = "pulsar/partitionedNs-" + isPartitionedTopic;
+ final String namespace = "pulsar/partitionedNs-" + isPartitionedTopic + "-" + System.nanoTime();;
Review comment:
fixed the duplicate ";"
It could be namespace left from other test run, the zookeeper used for test will have a temp dir to store data with same name. It could be really hard to debug in github action VM but make namespace unique across different test run should solve the issue for us.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #9659: [FlakyTest]Try to fix flaky test ReplicatorTest.testReplicatorOnPartitionedTopic.
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #9659:
URL: https://github.com/apache/pulsar/pull/9659#issuecomment-783338057
We could use something like
`BrokerTestBase#newTopicName`
making it a `static` Utility method (and possibly extracting it to a separate file
https://github.com/apache/pulsar/blob/master/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BrokerTestBase.java#L93
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] MarvinCai commented on pull request #9659: [FlakyTest]Try to fix flaky test ReplicatorTest.testReplicatorOnPartitionedTopic.
Posted by GitBox <gi...@apache.org>.
MarvinCai commented on pull request #9659:
URL: https://github.com/apache/pulsar/pull/9659#issuecomment-783957484
@eolivelli Updated per you comment.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] MarvinCai commented on a change in pull request #9659: [FlakyTest]Try to fix flaky test ReplicatorTest.testReplicatorOnPartitionedTopic.
Posted by GitBox <gi...@apache.org>.
MarvinCai commented on a change in pull request #9659:
URL: https://github.com/apache/pulsar/pull/9659#discussion_r582440633
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/BrokerTestUtil.java
##########
@@ -0,0 +1,28 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.broker;
+
+/**
+ * Holds util methods used in test.
+ */
+public class BrokerTestUtil {
+ // Generate unique namespace name for different test run.
+ public static String newUniqueNamespace(String prefix) { return prefix + "-" + System.nanoTime(); }
Review comment:
I think that's better, updated to UUID
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] MarvinCai commented on pull request #9659: [FlakyTest]Try to fix flaky test ReplicatorTest.testReplicatorOnPartitionedTopic.
Posted by GitBox <gi...@apache.org>.
MarvinCai commented on pull request #9659:
URL: https://github.com/apache/pulsar/pull/9659#issuecomment-785639760
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] MarvinCai commented on a change in pull request #9659: [FlakyTest]Try to fix flaky test ReplicatorTest.testReplicatorOnPartitionedTopic.
Posted by GitBox <gi...@apache.org>.
MarvinCai commented on a change in pull request #9659:
URL: https://github.com/apache/pulsar/pull/9659#discussion_r581316239
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/BrokerTestUtil.java
##########
@@ -0,0 +1,28 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.broker;
+
+/**
+ * Holds util methods used in test.
+ */
+public class BrokerTestUtil {
+ // Generate unique namespace name for different test run.
+ public static String newUniqueNamespace(String prefix) { return prefix + "-" + System.nanoTime(); }
Review comment:
I not sure if just an AtomicLong is enough, if it start from 0 then there's still good chance different test run can have same name, if we initialize with some value like currentTimeMillis() or nanoTime() then I don't see it makes much benefit with the AtomicLong?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on a change in pull request #9659: [FlakyTest]Try to fix flaky test ReplicatorTest.testReplicatorOnPartitionedTopic.
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #9659:
URL: https://github.com/apache/pulsar/pull/9659#discussion_r581036106
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/BrokerTestUtil.java
##########
@@ -0,0 +1,28 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.broker;
+
+/**
+ * Holds util methods used in test.
+ */
+public class BrokerTestUtil {
+ // Generate unique namespace name for different test run.
+ public static String newUniqueNamespace(String prefix) { return prefix + "-" + System.nanoTime(); }
Review comment:
what about using a static `AtomicLong` variable and use `incrementAndGet` ?
what about naming this method `newUniqueName` ? as we are not using it only for namespaces but also for topic names
or we could have `newNamespaceName` and `newTopicName`
nit: probably formatting is not following current coding style
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org