curator-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Eric Tschetter (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CURATOR-17) PathChildrenCache closes it's executor always
Date Fri, 10 May 2013 19:55:16 GMT

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

Eric Tschetter commented on CURATOR-17:
---------------------------------------

The Branch generally looks fine to me.  There are two things of note, though:

There are two tests that I had created on TestPathChildrenCache for CURATOR-21 that I think
would be helpful in verifying these changes, but I don't see them on the branch.  One of the
tests was in the main patch and one of them was in the "fix races" patch.  I'm guessing the
branch was done off of master (or something that hasn't had CURATOR-21 applied to it yet)?

For the code itself, I'm not sure about the logic when closing the executor.  It currently
goes through all of the futures and tries to cancel them.  That's normally fine, but when
the cancel fails, it throws an exception out of the close.  I think failure to cancel a task
is actually a fairly common occurrence, so it'd probably be best to continue trying to close
things.  You could ignore the failures entirely (just log it) or throw an exception out at
the end.  I'd generally prefer to ignore and log, because I'm not sure that the person who
calls close() can do much about the exceptional case of some tasks running even though they
shouldn't be, but if you feel strongly about throwing an exception I won't hold it against
you.

                
> PathChildrenCache closes it's executor always
> ---------------------------------------------
>
>                 Key: CURATOR-17
>                 URL: https://issues.apache.org/jira/browse/CURATOR-17
>             Project: Apache Curator
>          Issue Type: Bug
>          Components: Recipes
>    Affects Versions: 2.0.0-incubating
>            Reporter: Eric Tschetter
>            Assignee: Jordan Zimmerman
>             Fix For: 2.0.1-incubating
>
>
> PathChildrenCache.close() calls shutdownNow() on its executor, always.
> I generally reuse executors and have been passing them in on the constructor.  I have
to wrap my executors in something that ignores the shutdownNow() call in order to work around
this.  It's not the end of the world, but it is a little annoying.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message