tisonkun commented on code in PR #430:
URL: https://github.com/apache/curator/pull/430#discussion_r967851944
##########
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:
Thanks! Integrated.
##########
curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderLatch.java:
##########
@@ -717,6 +720,7 @@ else if ( !oldValue && newValue )
private void setNode(String newValue) throws Exception
{
String oldPath = ourPath.getAndSet(newValue);
+ log.debug("setNode with oldPath: {}, newValue: {}", oldPath, newValue);
Review Comment:
Updated.
##########
curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderLatch.java:
##########
@@ -667,9 +670,9 @@ protected void handleStateChange(ConnectionState newState)
{
try
{
- if (
client.getConnectionStateErrorPolicy().isErrorState(ConnectionState.SUSPENDED)
|| !hasLeadership.get() )
Review Comment:
It's for backward compatibility that once we treat ConnectionLoss as
ErrorState. I agree with that since we don't give up the leadership (`reset`)
actively, we can remove the `if` condition (but still keep `getChildren`, of
course).
However, it can be a follow-up improvement that doesn't affect the
correctness. You're welcome to make the patch.
--
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
