[
https://issues.apache.org/jira/browse/CASSANDRA-12461?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15428470#comment-15428470
]
Anthony Cozzie commented on CASSANDRA-12461:
--------------------------------------------
Thanks for your review Alex! First, the nits:
I am -1 on any sort of synchronization optimization. It is a great way to
introduce stupid bugs and the performance benefits here would be nonexistent.
For example, I'm not sure your patch works there: suppose we have the following
interleaving:
* addHook reads the AtomicBoolean, it is false
* shutdownHook sets the AtomicBoolean to true
* shutdownHook runs the hooks
* addHook adds the hook
I don't even know if this analysis is correct; I just don't want to think about
it.
List add doesn't really seem to return anything useful according to the javadoc
"Lists that support this operation may place limitations on what elements may
be added to this list. In particular, some lists will refuse to add null
elements, and others will impose restrictions on the type of elements that may
be added. List classes should clearly specify in their documentation any
restrictions on what elements may be added.". So I am OK either way.
OK, the more serious stuff: I did not understand what drain() was doing. We
should definitely not run post shutdown hooks and turn off logging and such
until the very end. Probably the simplest thing would be to simply clear the
preshutdownhook list after calling them on drain() so they are not called twice.
I only smoke tested things on a shutdown, I'm wondering if that error has to do
with the shutdown hook for logback being called twice. I'll test that case
today.
> Add hooks to StorageService shutdown
> ------------------------------------
>
> Key: CASSANDRA-12461
> URL: https://issues.apache.org/jira/browse/CASSANDRA-12461
> Project: Cassandra
> Issue Type: Bug
> Reporter: Anthony Cozzie
> Assignee: Anthony Cozzie
> Attachments:
> 0001-CASSANDRA-12461-add-C-support-for-shutdown-runnables.patch
>
>
> The JVM will usually run shutdown hooks in parallel. This can lead to
> synchronization problems between Cassandra, services that depend on it, and
> services it depends on. This patch adds some simple support for shutdown
> hooks to StorageService.
> This should nearly solve CASSANDRA-12011
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
