[curator] branch CURATOR-559-fix-nested-retry-loops-reopen updated: CURATOR-559 - background thread retries are spoiling the test. Try to work around this

[ra...@apache.org]

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 <ra...@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);
             }
         };