XComp commented on code in PR #430:
URL: https://github.com/apache/curator/pull/430#discussion_r958163715


##########
curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatch.java:
##########
@@ -218,6 +218,30 @@ public void testWatchedNodeDeletedOnReconnect() throws 
Exception
         }
     }
 
+    @Test
+    public void testOurPathDeletedOnReconnect() throws Exception

Review Comment:
   What do you think of the following test (I updated the test a bit):
   ```
       @Test
       public void 
testLeadershipElectionWhenNodeDisappearsAfterChildrenAreRetrieved() throws 
Exception
       {
           final String latchPath = "/foo/bar";
           final Timing2 timing = new Timing2();
           try (CuratorFramework client = 
CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), 
timing.connection(), new RetryOneTime(1)))
           {
               client.start();
               LeaderLatch latchInitialLeader = new LeaderLatch(client, 
latchPath, "initial-leader");
               LeaderLatch latchCandidate0 = new LeaderLatch(client, latchPath, 
"candidate-0");
               LeaderLatch latchCandidate1 = new LeaderLatch(client, latchPath, 
"candidate-1");
   
               try
               {
                   latchInitialLeader.start();
   
                   // we want to make sure that the leader gets leadership 
before other instances joining the party
                   
waitForALeader(Collections.singletonList(latchInitialLeader), new Timing());
   
                   // candidate #0 will wait for the leader to go away - this 
should happen after the child nodes are retrieved by candidate #0
                   latchCandidate0.debugCheckLeaderShipLatch = new 
CountDownLatch(1);
   
                   latchCandidate0.start();
                   timing.sleepABit();
   
                   // no extract CountDownLatch needs to be set here because 
candidate #1 will rely on candidate #0
                   latchCandidate1.start();
                   timing.sleepABit();
   
                   // triggers the removal of the corresponding child node 
after candidate #0 retrieved the children
                   latchInitialLeader.close();
   
                   latchCandidate0.debugCheckLeaderShipLatch.countDown();
   
                   waitForALeader(Arrays.asList(latchCandidate0, 
latchCandidate1), new Timing());
   
                   assertTrue(latchCandidate0.hasLeadership() ^ 
latchCandidate1.hasLeadership());
               } finally
               {
                   for (LeaderLatch latchToClose : 
Arrays.asList(latchInitialLeader, latchCandidate0, latchCandidate1))
                   {
                       if ( latchToClose.getState() != LeaderLatch.State.CLOSED 
)
                       {
                           latchToClose.close();
                       }
                   }
               }
           }
       
   ```
   
   This test case describes the situation for 
[CURATOR-645](https://issues.apache.org/jira/browse/CURATOR-645). I checked in 
a local test run, that it will run forever when still using `reset()` in 
[LeaderLatch:633](https://github.com/apache/curator/blob/425598cb6bf6a5b227a5fdd293fe46c7978d6578/curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderLatch.java#L633)
 but would succeed when using `getChildren()`, instead.



-- 
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: dev-unsubscr...@curator.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to