[
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
