curator-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CURATOR-157) Avoid stack traces closing PathChildrenCache followed by closing CuratorFramework
Date Fri, 06 Feb 2015 16:02:34 GMT

    [ https://issues.apache.org/jira/browse/CURATOR-157?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14309357#comment-14309357
] 

ASF GitHub Bot commented on CURATOR-157:
----------------------------------------

Github user madrob commented on the pull request:

    https://github.com/apache/curator/pull/51#issuecomment-73259604
  
    Looks like I neglected to respond to this for a while. Here is why I think the logic is
faulty.
    
    T2: `processResult()` method entry
    T2: `synchronized (backgroundTaskMonitor)` acquire lock
    T2: `if (state.get().equals(State.CLOSED)) { }` evaluates false
    T1: `close()` method entry
    T1: `if ( state.compareAndSet(State.STARTED, State.CLOSED) )` evaluates true
    T1: `listeners.clear()`
    T1:  `client.getConnectionStateListenable().removeListener(connectionStateListener)`
    T1 suspends waiting for the lock
    T2: `processBackgroundResult(event)` gets called on a closed connection
    
    I think the solution is to put all of the `close()` method into the `synchronized` block,
but that is not a super appealing answer. Maybe it will be fine though.
    
    The other concern is that now we can only process a single background event at a time,
which seems like it could be a pretty high price to pay.


> Avoid stack traces closing PathChildrenCache followed by closing CuratorFramework
> ---------------------------------------------------------------------------------
>
>                 Key: CURATOR-157
>                 URL: https://issues.apache.org/jira/browse/CURATOR-157
>             Project: Apache Curator
>          Issue Type: Improvement
>            Reporter: Bruno Dumon
>            Assignee: Jordan Zimmerman
>         Attachments: LogProblemIllustration.java
>
>
> When closing PathChildrenCache, and immediately afterwards closing CuratorFramework,
some ERROR-level stack traces are logged.
> This was previously reported on the mailing list: http://curator.markmail.org/thread/bmfr62ekx5p2vv7f
> The cause is that the BackgroundCallback defined in PathChildrenCache.refresh() will,
when triggered, perform some more ZooKeeper operations.
> Thus one can get in sequences such as:
>  * operation with BackgroundCallback is submitted
>  * processResult of the BackgroundCallback is called
>  * PathChildrenCache is closed
>  * CuratorFramework is closed
>  * processResult, which is running on another thread, comes to the point it does operations
on ZooKeeper, which fail because ZooKeeper is closed.
> There is no real impact on the application, it is just for log-esthetical reasons that
I'd like to avoid it.
> In the more common case, the processResult will receive an IllegalStateException, which
could be easily catched and ignored in PathChildrenCache if the PathChildrenCache is closed:
> {noformat}
> 14/10/30 11:24:51 ERROR org.apache.curator.framework.imps.CuratorFrameworkImpl: Background
exception was not retry-able or retry gave up
> java.lang.IllegalStateException: instance must be started before calling this method
> 	at com.google.common.base.Preconditions.checkState(Preconditions.java:149)
> 	at org.apache.curator.framework.imps.CuratorFrameworkImpl.getData(CuratorFrameworkImpl.java:360)
> 	at org.apache.curator.framework.recipes.cache.PathChildrenCache.getDataAndStat(PathChildrenCache.java:545)
> 	at org.apache.curator.framework.recipes.cache.PathChildrenCache.processChildren(PathChildrenCache.java:668)
> 	at org.apache.curator.framework.recipes.cache.PathChildrenCache.access$200(PathChildrenCache.java:68)
> 	at org.apache.curator.framework.recipes.cache.PathChildrenCache$4.processResult(PathChildrenCache.java:490)
> 	at org.apache.curator.framework.imps.CuratorFrameworkImpl.sendToBackgroundCallback(CuratorFrameworkImpl.java:715)
> 	at org.apache.curator.framework.imps.CuratorFrameworkImpl.processBackgroundOperation(CuratorFrameworkImpl.java:502)
> 	at org.apache.curator.framework.imps.GetChildrenBuilderImpl$2.processResult(GetChildrenBuilderImpl.java:166)
> 	at org.apache.zookeeper.ClientCnxn$EventThread.processEvent(ClientCnxn.java:590)
> 	at org.apache.zookeeper.ClientCnxn$EventThread.run(ClientCnxn.java:498)
> {noformat}
> But sometimes it also fails with other async operations deeper down:
> {noformat}
> 14/10/30 11:24:51 ERROR org.apache.curator.framework.imps.CuratorFrameworkImpl: Background
exception was not retry-able or retry gave up
> java.lang.IllegalStateException: Client is not started
> 	at com.google.common.base.Preconditions.checkState(Preconditions.java:149)
> 	at org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:113)
> 	at org.apache.curator.framework.imps.CuratorFrameworkImpl.getZooKeeper(CuratorFrameworkImpl.java:474)
> 	at org.apache.curator.framework.imps.GetDataBuilderImpl.performBackgroundOperation(GetDataBuilderImpl.java:263)
> 	at org.apache.curator.framework.imps.OperationAndData.callPerformBackgroundOperation(OperationAndData.java:65)
> 	at org.apache.curator.framework.imps.CuratorFrameworkImpl.performBackgroundOperation(CuratorFrameworkImpl.java:789)
> 	at org.apache.curator.framework.imps.CuratorFrameworkImpl.processBackgroundOperation(CuratorFrameworkImpl.java:487)
> 	at org.apache.curator.framework.imps.GetDataBuilderImpl.forPath(GetDataBuilderImpl.java:275)
> 	at org.apache.curator.framework.imps.GetDataBuilderImpl.forPath(GetDataBuilderImpl.java:41)
> 	at org.apache.curator.framework.recipes.cache.PathChildrenCache.getDataAndStat(PathChildrenCache.java:545)
> 	at org.apache.curator.framework.recipes.cache.PathChildrenCache.processChildren(PathChildrenCache.java:668)
> 	at org.apache.curator.framework.recipes.cache.PathChildrenCache.access$200(PathChildrenCache.java:68)
> 	at org.apache.curator.framework.recipes.cache.PathChildrenCache$4.processResult(PathChildrenCache.java:490)
> 	at org.apache.curator.framework.imps.CuratorFrameworkImpl.sendToBackgroundCallback(CuratorFrameworkImpl.java:715)
> 	at org.apache.curator.framework.imps.CuratorFrameworkImpl.processBackgroundOperation(CuratorFrameworkImpl.java:502)
> 	at org.apache.curator.framework.imps.GetChildrenBuilderImpl$2.processResult(GetChildrenBuilderImpl.java:166)
> 	at org.apache.zookeeper.ClientCnxn$EventThread.processEvent(ClientCnxn.java:590)
> 	at org.apache.zookeeper.ClientCnxn$EventThread.run(ClientCnxn.java:498)
> {noformat}
> Therefore I have created a patch where PathChildrenCache.close() will wait until the
possibly running BackgroundCallback is finished.
> I will also attach a small class that illustrates the problem.



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

Mime
View raw message