Github user hanm commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/157#discussion_r98734336
  
    --- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
    @@ -507,9 +507,12 @@ public synchronized void shutdown() {
             if (firstProcessor != null) {
                 firstProcessor.shutdown();
             }
    -        if (zkDb != null) {
    -            zkDb.clear();
    -        }
    +
    +        // There is no need to clear the database
    +        //  * When a new quorum is established we can still apply the diff
    +        //    on top of the same zkDb data
    +        //  * If we fetch a new snapshot from leader, the zkDb will be
    +        //    cleared anyway before loading the snapshot
     
    --- End diff --
    
    There is one case we may still want to clear db here - when one of the 
ZooKeeper critical threads (such as * processors, session trackers) fail, 
ZooKeeper server will shutdown (see runFromConfig) and consequently invoke 
ZooKeeper#shutdown. In this case, I don't see a particular reason not to clear 
the db, though not doing it might be fine (as one could argue the server will 
be dead anyway), but I tend to lean towards the safe side on cleaning the db. 
One way to conditionally do that is to add a Boolean parameter to 
ZooKeeper#shutdown so we can have fine grained control over when to clear db in 
what code path.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to