zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From revans2 <...@git.apache.org>
Subject [GitHub] zookeeper pull request #180: ZOOKEEPER-2700 add JMX `takeSnapshot` method an...
Date Tue, 21 Feb 2017 16:05:59 GMT
Github user revans2 commented on a diff in the pull request:

    --- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
    @@ -303,15 +305,38 @@ public void loadData() throws IOException, InterruptedException
         public void takeSnapshot(){
    --- End diff --
    @eribeiro An AtomicBoolean is not enough as the SyncRequestProcessor ignores it, only
the JMX and admin commands use it.
    @flier there are several things going on here and I don't know the reasoning for all of
them but I can guess.
    1.  Only take one snapshot at a time.  I don't think this is super critical, because it
is not a correctness problem.  Having multiple snapshots happening at the same time should
work correctly even in the face of crashes, but it becomes a performance problem.  There is
a limited amount of CPU, Memory, and most importantly disk bandwidth and IOps.  The last successful
snapshot wins.  So having multiple snapshots all going at the same time means we are likely
to be doing a lot of wasted work if everything does happen successfully.  If we can schedule
the snapshots so there is only one going on at a time then the full bandwidth and IOps can
go to that snapshot.  Even better would be to space them out, so if we force a snapshot it
makes SyncRequestProcessor reset its counter so we don't take another one until X more transactions
have been processed.
    2. Taking a snapshot at the wrong time and then crashing can corrupt the DB on that node.
 This is potentially critical.  The probability of this happening is very very small, but
if we can fix it so it can never happen, I would be much happier.
    I think we can fix both issues at the same time, by making JMX and Admin use SyncRequestProcessor
instead of going to the ZookeeperServer directly.  If no SyncRequestProcessor is ready then
we can return such to the user so they know the node is not ready to take a snapshot. 
    I also really would like to understand the use case for this command, because I just don't
see much value to a user to force another snapshot if one is already in progress.  I also
would like to understand when you would want to take a snapshot and if these changes would
too severely limit that.

If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.

View raw message