You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by ra...@apache.org on 2020/04/21 12:47:56 UTC

[curator] branch CURATOR-559-fix-nested-retry-loops-reopen updated (0f9260e -> 713c859)

This is an automated email from the ASF dual-hosted git repository.

randgalt pushed a change to branch CURATOR-559-fix-nested-retry-loops-reopen
in repository https://gitbox.apache.org/repos/asf/curator.git.


 discard 0f9260e  CURATOR-559 - background thread retries are spoiling the test. Try to work around this
    omit 320b6e1  CURATOR-559 - more attempts to keep tests from failing. Make sure count is zeroed after server is stopped.
     add 1c56fc6  CURATOR-567 - TestCleanState.closeAndTestClean has meet is desired goal. Start removing it where it causes test flakiness
     add 1d0c6f2  Merge branch 'CURATOR-567-remove-test-clean-state-where-flaky'
     add 4fb8f2c  CURATOR-567 - At this point, TestCleanState is so flakey we should just turn it off for now. It's not serving much purpose anyway.
     add 629486f  Merge branch 'CURATOR-567-remove-test-clean-state-where-flaky'
     new 3d76a31  CURATOR-559 - more attempts to keep tests from failing. Make sure count is zeroed after server is stopped.
     new 713c859  CURATOR-559 - background thread retries are spoiling the test. Try to work around this

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (0f9260e)
            \
             N -- N -- N   refs/heads/CURATOR-559-fix-nested-retry-loops-reopen (713c859)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../main/java/org/apache/curator/utils/DebugUtils.java   |  1 +
 .../apache/curator/framework/imps/TestCleanState.java    |  4 +++-
 .../curator/connection/TestThreadLocalRetryLoop.java     |  7 +++----
 .../framework/recipes/shared/TestSharedCount.java        | 16 +++++++---------
 4 files changed, 14 insertions(+), 14 deletions(-)


[curator] 02/02: CURATOR-559 - background thread retries are spoiling the test. Try to work around this

Posted by 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

commit 713c859a97496cf36649c93bdc36a8154f581bfa
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       | 37 +++++++++++++++-------
 1 file changed, 25 insertions(+), 12 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..e920bd8 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
@@ -18,6 +18,7 @@
  */
 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 +26,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 +39,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 +82,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 +91,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 +103,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 +128,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);
             }
         };


[curator] 01/02: CURATOR-559 - more attempts to keep tests from failing. Make sure count is zeroed after server is stopped.

Posted by 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

commit 3d76a317b68839b83e298fdd48ead1a95e5f8fe8
Author: randgalt <ra...@apache.org>
AuthorDate: Sun Apr 19 14:54:04 2020 -0500

    CURATOR-559 - more attempts to keep tests from failing. Make sure count is zeroed after server is stopped.
---
 .../org/apache/curator/connection/TestThreadLocalRetryLoop.java    | 7 ++++---
 1 file changed, 4 insertions(+), 3 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 56362e6..686109c 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
@@ -44,7 +44,7 @@ public class TestThreadLocalRetryLoop extends CuratorTestBase
         AtomicInteger count = new AtomicInteger();
         try (CuratorFramework client = newClient(count))
         {
-            prep(client);
+            prep(client, count);
             doOperation(client);
             Assert.assertEquals(count.get(), retryCount + 1);    // Curator's retry policy has been off by 1 since inception - we might consider fixing it someday
         }
@@ -58,7 +58,7 @@ public class TestThreadLocalRetryLoop extends CuratorTestBase
         AtomicInteger count = new AtomicInteger();
         try (CuratorFramework client = newClient(count))
         {
-            prep(client);
+            prep(client, count);
             for ( int i = 0; i < threadQty; ++i )
             {
                 executorService.submit(() -> doOperation(client));
@@ -82,7 +82,7 @@ public class TestThreadLocalRetryLoop extends CuratorTestBase
         return CuratorFrameworkFactory.newClient(server.getConnectString(), 100, 100, retryPolicy);
     }
 
-    private void prep(CuratorFramework client) throws Exception
+    private void prep(CuratorFramework client, AtomicInteger count) throws Exception
     {
         client.start();
         client.create().forPath("/test");
@@ -94,6 +94,7 @@ public class TestThreadLocalRetryLoop extends CuratorTestBase
         });
         server.stop();
         Assert.assertTrue(timing.awaitLatch(lostLatch));
+        count.set(0);   // in case the server shutdown incremented the count
     }
 
     private Void doOperation(CuratorFramework client) throws Exception