[ https://issues.apache.org/jira/browse/CURATOR-644?focusedWorklogId=804703&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-804703 ]
ASF GitHub Bot logged work on CURATOR-644: ------------------------------------------ Author: ASF GitHub Bot Created on: 30/Aug/22 08:20 Start Date: 30/Aug/22 08:20 Worklog Time Spent: 10m Work Description: 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: ``` @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 latch0 = new LeaderLatch(client, latchPath, "0"); latch0.start(); waitForALeader(Collections.singletonList(latch0), new Timing()); LeaderLatch latch1 = new LeaderLatch(client, latchPath, "1"); LeaderLatch latch2 = new LeaderLatch(client, latchPath, "2"); latch1.debugCheckLeaderShipLatch = new CountDownLatch(1); latch1.start(); timing.sleepABit(); latch2.start(); timing.sleepABit(); latch0.close(); latch1.debugCheckLeaderShipLatch.countDown(); waitForALeader(Arrays.asList(latch1, latch2), new Timing()); latch1.close(); latch2.close(); } } ``` This test case describes the situation for 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. ########## 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: ``` @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 latch0 = new LeaderLatch(client, latchPath, "0"); latch0.start(); waitForALeader(Collections.singletonList(latch0), new Timing()); LeaderLatch latch1 = new LeaderLatch(client, latchPath, "1"); LeaderLatch latch2 = new LeaderLatch(client, latchPath, "2"); latch1.debugCheckLeaderShipLatch = new CountDownLatch(1); latch1.start(); timing.sleepABit(); latch2.start(); timing.sleepABit(); latch0.close(); latch1.debugCheckLeaderShipLatch.countDown(); waitForALeader(Arrays.asList(latch1, latch2), new Timing()); latch1.close(); latch2.close(); } } ``` This test case describes the situation for 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. Issue Time Tracking ------------------- Worklog Id: (was: 804703) Time Spent: 2h 10m (was: 2h) > CLONE - Race conditions in LeaderLatch after reconnecting to ensemble > --------------------------------------------------------------------- > > Key: CURATOR-644 > URL: https://issues.apache.org/jira/browse/CURATOR-644 > Project: Apache Curator > Issue Type: Bug > Affects Versions: 4.2.0 > Reporter: Ken Huang > Assignee: Jordan Zimmerman > Priority: Minor > Time Spent: 2h 10m > Remaining Estimate: 0h > > Clone from CURATOR-504. > We use LeaderLatch in a lot of places in our system and when ZooKeeper > ensemble is unstable and clients are reconnecting to logs are full of > messages like the following: > {{{}[2017-08-31 > 19:18:34,562][ERROR][org.apache.curator.framework.recipes.leader.LeaderLatch] > Can't find our node. Resetting. Index: -1 {{}}}} > According to the > [implementation|https://github.com/apache/curator/blob/4251fe328908e5fca37af034fabc190aa452c73f/curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderLatch.java#L529-L536], > this can happen in two cases: > * When internal state `ourPath` is null > * When the list of latches does not have the expected one. > I believe we hit the first condition because of races that occur after client > reconnects to ZooKeeper. > * Client reconnects to ZooKeeper and LeaderLatch gets the event and calls > reset method which set the internal state (`ourPath`) to null, removes old > latch and creates a new one. This happens in thread > "Curator-ConnectionStateManager-0". > * Almost simultaneously, LeaderLatch gets another even NodeDeleted > ([here|https://github.com/apache/curator/blob/4251fe328908e5fca37af034fabc190aa452c73f/curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderLatch.java#L543-L554]) > and tries to re-read the list of latches and check leadership. This happens > in the thread "main-EventThread". > Therefore, sometimes there is a situation when method `checkLeadership` is > called when `ourPath` is null. -- This message was sent by Atlassian Jira (v8.20.10#820010)