Github user madrob commented on a diff in the pull request:

    https://github.com/apache/curator/pull/242#discussion_r153341281
  
    --- Diff: 
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
 ---
    @@ -973,6 +981,33 @@ void performBackgroundOperation(OperationAndData<?> 
operationAndData)
             }
         }
     
    +    @VisibleForTesting
    +    volatile long sleepAndQueueOperationSeconds = 1;
    +
    +    private void sleepAndQueueOperation(OperationAndData<?> 
operationAndData) throws InterruptedException
    +    {
    +        operationAndData.sleepFor(sleepAndQueueOperationSeconds, 
TimeUnit.SECONDS);
    +        if ( queueOperation(operationAndData) )
    +        {
    +            forcedSleepOperations.add(operationAndData);
    +        }
    +    }
    +
    +    private void unSleepBackgroundOperations()
    +    {
    +        Collection<OperationAndData<?>> drain = new 
ArrayList<>(forcedSleepOperations.size());
    +        forcedSleepOperations.drainTo(drain);
    +        log.debug("Clearing sleep for {} operations", drain.size());
    +        for ( OperationAndData<?> operation : drain )
    +        {
    +            operation.clearSleep();
    +            if ( backgroundOperations.remove(operation) )
    --- End diff --
    
    Ah, I'm with you now. Can you add a method comment that explains the nuance 
of the implementation here?
    
    If we're really concerned about ordering, then we could update clearSleep() 
to set expiration time to the current time instead of setting it to zero.


---

Reply via email to