This is an automated email from the ASF dual-hosted git repository. randgalt pushed a commit to branch CURATOR-559-fix-nested-retry-loops-reopen in repository https://gitbox.apache.org/repos/asf/curator.git
The following commit(s) were added to refs/heads/CURATOR-559-fix-nested-retry-loops-reopen by this push: new 0f9260e CURATOR-559 - background thread retries are spoiling the test. Try to work around this 0f9260e is described below commit 0f9260eb89ce2378877ca7a805e85a04a7c3f135 Author: randgalt <randg...@apache.org> AuthorDate: Mon Apr 20 17:14:41 2020 -0500 CURATOR-559 - background thread retries are spoiling the test. Try to work around this --- .../connection/TestThreadLocalRetryLoop.java | 44 ++++++++++++++-------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/curator-recipes/src/test/java/org/apache/curator/connection/TestThreadLocalRetryLoop.java b/curator-recipes/src/test/java/org/apache/curator/connection/TestThreadLocalRetryLoop.java index 686109c..f7c997d 100644 --- a/curator-recipes/src/test/java/org/apache/curator/connection/TestThreadLocalRetryLoop.java +++ b/curator-recipes/src/test/java/org/apache/curator/connection/TestThreadLocalRetryLoop.java @@ -6,9 +6,9 @@ * 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 - * + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> * 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 @@ -16,8 +16,10 @@ * specific language governing permissions and limitations * under the License. */ + package org.apache.curator.connection; +import org.apache.curator.RetryLoop; import org.apache.curator.RetryPolicy; import org.apache.curator.RetrySleeper; import org.apache.curator.framework.CuratorFramework; @@ -25,6 +27,7 @@ import org.apache.curator.framework.CuratorFrameworkFactory; import org.apache.curator.framework.state.ConnectionState; import org.apache.curator.retry.RetryNTimes; import org.apache.curator.test.compatibility.CuratorTestBase; +import org.apache.curator.utils.ThreadUtils; import org.apache.zookeeper.KeeperException; import org.testng.Assert; import org.testng.annotations.Test; @@ -37,6 +40,7 @@ import java.util.concurrent.atomic.AtomicInteger; public class TestThreadLocalRetryLoop extends CuratorTestBase { private static final int retryCount = 4; + private static final String backgroundThreadNameBase = "ignore-curator-background-thread"; @Test(description = "Check for fix for CURATOR-559") public void testRecursingRetry() throws Exception @@ -79,7 +83,7 @@ public class TestThreadLocalRetryLoop extends CuratorTestBase private CuratorFramework newClient(AtomicInteger count) { RetryPolicy retryPolicy = makeRetryPolicy(count); - return CuratorFrameworkFactory.newClient(server.getConnectString(), 100, 100, retryPolicy); + return CuratorFrameworkFactory.builder().connectString(server.getConnectString()).connectionTimeoutMs(100).sessionTimeoutMs(100).retryPolicy(retryPolicy).threadFactory(ThreadUtils.newThreadFactory(backgroundThreadNameBase)).build(); } private void prep(CuratorFramework client, AtomicInteger count) throws Exception @@ -88,7 +92,8 @@ public class TestThreadLocalRetryLoop extends CuratorTestBase client.create().forPath("/test"); CountDownLatch lostLatch = new CountDownLatch(1); client.getConnectionStateListenable().addListener((__, newState) -> { - if (newState == ConnectionState.LOST) { + if ( newState == ConnectionState.LOST ) + { lostLatch.countDown(); } }); @@ -99,15 +104,21 @@ public class TestThreadLocalRetryLoop extends CuratorTestBase private Void doOperation(CuratorFramework client) throws Exception { - try - { - client.checkExists().forPath("/hey"); - Assert.fail("Should have thrown an exception"); - } - catch ( KeeperException ignore ) - { - // correct - } + RetryLoop.callWithRetry(client.getZookeeperClient(), () -> { + for ( int i = 0; i < 5; ++i ) // simulate a bunch of calls in the same thread/call chain + { + try + { + client.checkExists().forPath("/hey"); + Assert.fail("Should have thrown an exception"); + } + catch ( KeeperException ignore ) + { + // correct + } + } + return null; + }); return null; } @@ -118,7 +129,10 @@ public class TestThreadLocalRetryLoop extends CuratorTestBase @Override public boolean allowRetry(int retryCount, long elapsedTimeMs, RetrySleeper sleeper) { - count.incrementAndGet(); + if ( !Thread.currentThread().getName().contains(backgroundThreadNameBase) ) // if it does, it's Curator's background thread - don't count these + { + count.incrementAndGet(); + } return super.allowRetry(retryCount, elapsedTimeMs, sleeper); } };