[jira] [Updated] (CURATOR-338) SessionFailRetryLoop doesn't work correctly

["Rohit Agarwal (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/CURATOR-338?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Rohit Agarwal updated CURATOR-338:
----------------------------------
    Description: 
According to the documentation at https://curator.apache.org/apidocs/org/apache/curator/SessionFailRetryLoop.html
the canonical usage of {{SessionFailRetryLoop}} is:
{code}
 SessionFailRetryLoop    retryLoop = client.newSessionFailRetryLoop(mode);
 retryLoop.start();
 try
 {
     while ( retryLoop.shouldContinue() )
     {
         try
         {
             // do work
         }
         catch ( Exception e )
         {
             retryLoop.takeException(e);
         }
     }
 }
 finally
 {
     retryLoop.close();
 }
{code}

Let's say there is an exception which can be retried (i.e. it's not session expiry.) If my understanding of the code is correct, it will never be retried.

This is because when you call {{shouldContinue()}} for the first time, {{isDone}} is set to true (https://github.com/apache/curator/blob/apache-curator-2.11.0/curator-client/src/main/java/org/apache/curator/SessionFailRetryLoop.java#L204)
So, the next time {{shouldContinue()}} will return false, unless {{isDone}} is set to {{false}} in the interim. However, {{isDone}} is only updated when the session has expired and the mode is retry https://github.com/apache/curator/blob/apache-curator-2.11.0/curator-client/src/main/java/org/apache/curator/SessionFailRetryLoop.java#L241

  was:
According to the documentation at https://curator.apache.org/apidocs/org/apache/curator/SessionFailRetryLoop.html
the canonical usage of {{SessionFailRetryLoop}} is:
{code}
 SessionFailRetryLoop    retryLoop = client.newSessionFailRetryLoop(mode);
 retryLoop.start();
 try
 {
     while ( retryLoop.shouldContinue() )
     {
         try
         {
             // do work
         }
         catch ( Exception e )
         {
             retryLoop.takeException(e);
         }
     }
 }
 finally
 {
     retryLoop.close();
 }
{code}

Let's say there is an exception which can be retried (i.e. it's not session expiry.) However, it will never be retried.

This is because when the first time you call {{shouldContinue()}} for the first time, {{isDone}} is set to true (https://github.com/apache/curator/blob/apache-curator-2.11.0/curator-client/src/main/java/org/apache/curator/SessionFailRetryLoop.java#L204)
So, the next time {{shouldContinue()}} will return false, unless {{isDone}} is set to {{false}} in the interim. However, {{isDone}} is only updated when the session has expired and the mode is retry https://github.com/apache/curator/blob/apache-curator-2.11.0/curator-client/src/main/java/org/apache/curator/SessionFailRetryLoop.java#L241


> SessionFailRetryLoop doesn't work correctly
> -------------------------------------------
>
>                 Key: CURATOR-338
>                 URL: https://issues.apache.org/jira/browse/CURATOR-338
>             Project: Apache Curator
>          Issue Type: Bug
>    Affects Versions: 2.11.0
>            Reporter: Rohit Agarwal
>            Assignee: Jordan Zimmerman
>
> According to the documentation at https://curator.apache.org/apidocs/org/apache/curator/SessionFailRetryLoop.html
> the canonical usage of {{SessionFailRetryLoop}} is:
> {code}
>  SessionFailRetryLoop    retryLoop = client.newSessionFailRetryLoop(mode);
>  retryLoop.start();
>  try
>  {
>      while ( retryLoop.shouldContinue() )
>      {
>          try
>          {
>              // do work
>          }
>          catch ( Exception e )
>          {
>              retryLoop.takeException(e);
>          }
>      }
>  }
>  finally
>  {
>      retryLoop.close();
>  }
> {code}
> Let's say there is an exception which can be retried (i.e. it's not session expiry.) If my understanding of the code is correct, it will never be retried.
> This is because when you call {{shouldContinue()}} for the first time, {{isDone}} is set to true (https://github.com/apache/curator/blob/apache-curator-2.11.0/curator-client/src/main/java/org/apache/curator/SessionFailRetryLoop.java#L204)
> So, the next time {{shouldContinue()}} will return false, unless {{isDone}} is set to {{false}} in the interim. However, {{isDone}} is only updated when the session has expired and the mode is retry https://github.com/apache/curator/blob/apache-curator-2.11.0/curator-client/src/main/java/org/apache/curator/SessionFailRetryLoop.java#L241



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)