[ https://issues.apache.org/jira/browse/CURATOR-644?focusedWorklogId=810239&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-810239 ]
ASF GitHub Bot logged work on CURATOR-644: ------------------------------------------ Author: ASF GitHub Bot Created on: 20/Sep/22 03:28 Start Date: 20/Sep/22 03:28 Worklog Time Spent: 10m Work Description: tisonkun commented on code in PR #430: URL: https://github.com/apache/curator/pull/430#discussion_r974852280 ########## curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderLatch.java: ########## @@ -198,9 +204,25 @@ public void close() throws IOException * @param closeMode allows the default close mode to be overridden at the time the latch is closed. * @throws IOException errors */ - public synchronized void close(CloseMode closeMode) throws IOException + public void close(CloseMode closeMode) throws IOException { - Preconditions.checkState(state.compareAndSet(State.STARTED, State.CLOSED), "Already closed or has not been started"); + internalClose(closeMode, true); + } + + private synchronized void internalClose(CloseMode closeMode, boolean failOnClosed) throws IOException + { + if (!state.compareAndSet(State.STARTED, State.CLOSED)) + { + if (failOnClosed) + { + throw new IllegalStateException("Already closed or has not been started"); + } + else + { + return; + } Review Comment: Make sense. Although, they're almost the same. So I just keep it as is now. Issue Time Tracking ------------------- Worklog Id: (was: 810239) Time Spent: 7h (was: 6h 50m) > 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: 7h > 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)