cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Petrov (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (CASSANDRA-12461) Add hooks to StorageService shutdown
Date Thu, 22 Sep 2016 12:38:20 GMT

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

Alex Petrov edited comment on CASSANDRA-12461 at 9/22/16 12:37 PM:
-------------------------------------------------------------------

I've discovered several more problems while working on this patch, in the last version (from
[here|https://github.com/acoz/cassandra/commits/12461]):

  * node drain code was duplicated (with minor differences, which I indicate below), as I
mentioned
  * it is possible to re-start services after drain which won't run regular shutdown path
on jvm exit
  * if the node was drained, under Windows the timer resolution (added in [CASSANDRA-9634])
was not reset, since the node was considered "already drained" (although this existed before
already)
  * same was happening with post-shutdown hooks in patch, since [here|https://github.com/acoz/cassandra/blob/f15cd6d2ea95540bfacd7285dc75d9d95999e5a2/src/java/org/apache/cassandra/service/StorageService.java#L575-L576]
we'll return from runnable, since those services were shut down during {{drain}} [here|https://github.com/acoz/cassandra/blob/f15cd6d2ea95540bfacd7285dc75d9d95999e5a2/src/java/org/apache/cassandra/service/StorageService.java#L586-L589].
So they wouldn't run at all if {{nodetool drain}} was called.
  * because logging system is shutdown in the post-shutdown hook, we depend on the order,
although we have to guarantee that logging is available for all hooks and avoid any races
or having to register hooks at the particular stage.

This is one of the reasons I was suggesting single drain process. 

I also suggest disallowing re-enabling auto-compaction, binary, gossip, handoff and thrift
to ensure that we do not need to re-stop them in the final shutdown hook. Operator can not
bring the node into "working" state after drain without restart anyways (one of the reasons
is the fact that commit log is shut down by that time), and it was most likely never intended
to do so.

I've made a comparison table to make it easier to see what {{drain()}} method was doing compared
to {{drainOnShutdown}} runnable:

|| nodetool drain || shutdown drain hook |
| disables autocompaction               |                                         |
| shuts down compaction manager | |
| recycles commitlog segment recycling |                                         |
| shuts down batchlog and hints earlier |                                         |
|                                       | flushes only tables with durable_writes |
|                                       | clears set timer resolution for windows |

I've combined the two processes, made clearer distinctions to allow running things in {{drainOnShutdown}}.
Since we can run all the items from the {{nodetool drain}} part of the list during the normal
node shutdown, the code got a bit simpler, too (the only difference is now logging). If this
granularity is not enough, we have two more options:
  * run post-shutdown hooks directly before the JVM shutdown
  * have 3 stages: pre-, post- drain and pre-jvm shutdown instead

Although I prefer the current way.

Preliminary version of the update (also, CI pending): 
|[12461-trunk-v2|https://github.com/ifesdjeen/cassandra/tree/12461-trunk-v2]|[dtest|http://cassci.datastax.com/job/ifesdjeen-12461-trunk-v2-dtest/]|[utest|http://cassci.datastax.com/job/ifesdjeen-12461-trunk-v2-testall/]|

(I've discussed the change "in theory" with [~slebresne], although it's still worth for someone
to take a deeper look at it, I'll ask around)


was (Author: ifesdjeen):
I've discovered several more problems while working on this patch:

  * node drain code was duplicated (with minor differences, which I indicate below)
  * if the node was drained, under Windows the timer resolution (added in [CASSANDRA-9634])
was not reset, since the node was considered "already drained".
  * same was happening with post-shutdown hooks, since [here|https://github.com/acoz/cassandra/blob/f15cd6d2ea95540bfacd7285dc75d9d95999e5a2/src/java/org/apache/cassandra/service/StorageService.java#L575-L576]
we'll return from runnable, since those services were shut down during {{drain}} [here|https://github.com/acoz/cassandra/blob/f15cd6d2ea95540bfacd7285dc75d9d95999e5a2/src/java/org/apache/cassandra/service/StorageService.java#L586-L589].


This is one of the reasons I was advocating for a single consistent drain process. 
I also suggest disallowing re-enabling auto-compaction, binary, gossip, handoff and thrift
to ensure that we do not need to re-stop them in the final shutdown hook. Operator can not
bring the node into "working" state after drain without restart anyways (one of the reasons
is the fact that commit log is shut down by that time), and it was most likely never intended
to do so.

I've made a comparison table to make it easier to see what {{drain()}} method was doing compared
to {{drainOnShutdown}} runnable:

|| nodetool drain || shutdown drain hook |
| disables autocompaction               |                                         |
| shuts down compaction manager | |
| recycles commitlog segment recycling |                                         |
| shuts down batchlog and hints earlier |                                         |
|                                       | flushes only tables with durable_writes |
|                                       | clears set timer resolution for windows |

I've combined the two processes, made clearer distinctions to allow running things in {{drainOnShutdown}}.
Since we can run all the items from the {{nodetool drain}} part of the list during the normal
node shutdown, the code got a bit simpler, too (the only difference is now logging). 

Preliminary version of the update (also, CI pending): 
|[12461-trunk-v2|https://github.com/ifesdjeen/cassandra/tree/12461-trunk-v2]|[dtest|http://cassci.datastax.com/job/ifesdjeen-12461-trunk-v2-dtest/]|[utest|http://cassci.datastax.com/job/ifesdjeen-12461-trunk-v2-testall/]|

(I've discussed the change "in theory" with [~slebresne], although it's still worth for someone
to take a deeper look at it, I'll ask around)

> 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
>             Fix For: 3.x
>
>         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)

Mime
View raw message