You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@zookeeper.apache.org by Rakesh R <ra...@huawei.com> on 2014/05/23 13:21:31 UTC

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/
-----------------------------------------------------------

(Updated May 23, 2014, 11:21 a.m.)


Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.


Summary (updated)
-----------------

ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.


Bugs: ZOOKEEPER-1907
    https://issues.apache.org/jira/browse/ZOOKEEPER-1907


Repository: zookeeper


Description
-------

Improve the thread handling mechanism by detecting if any of the critical thread dies.
Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.


Diffs
-----

  ./src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/RequestProcessor.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/ServerConfig.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/SessionTracker.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/UnimplementedRequestProcessor.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/quorum/AckRequestProcessor.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/quorum/LeaderRequestProcessor.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1585370 
  ./src/java/main/org/apache/zookeeper/server/quorum/SendAckRequestProcessor.java 1585370 
  ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1585370 
  ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1585370 

Diff: https://reviews.apache.org/r/20071/diff/


Testing
-------

yet to be inlcuded


Thanks,

Rakesh R

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Raul Gutierrez Segales <rg...@itevenworks.net>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review48178
-----------------------------------------------------------

Ship it!


Ship It!

- Raul Gutierrez Segales


On May 28, 2014, 12:26 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated May 28, 2014, 12:26 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/RequestProcessor.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/ServerConfig.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/SessionTracker.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/UnimplementedRequestProcessor.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/quorum/AckRequestProcessor.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderRequestProcessor.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1597987 
>   ./src/java/main/org/apache/zookeeper/server/quorum/SendAckRequestProcessor.java 1597987 
>   ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1597987 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerTest.java 1597987 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1597987 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.


> On Feb. 3, 2015, 4:52 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java, line 26
> > <https://reviews.apache.org/r/20071/diff/4/?file=845242#file845242line26>
> >
> >     "This will notify the server that some critical thread has stopped. It usually takes place when fatal error occured."

ok


- Rakesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review70766
-----------------------------------------------------------


On Feb. 19, 2015, 3:23 a.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2015, 3:23 a.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1660779 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1660779 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1660779 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1660779 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1660779 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1660779 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.


> On Feb. 3, 2015, 4:52 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java, line 47
> > <https://reviews.apache.org/r/20071/diff/4/?file=845240#file845240line47>
> >
> >     Do you think handleException() and notifyStopping are duplicate? Can we keep just one and remove the other?

removed notifyStopping and reused handleException()


- Rakesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review70766
-----------------------------------------------------------


On Feb. 3, 2015, 8:35 a.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2015, 8:35 a.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1656643 
>   ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1656643 
>   ./src/java/test/org/apache/zookeeper/server/SessionTrackerTest.java 1656643 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1656643 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1656643 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1656643 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1656643 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1656643 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Hongchao Deng <hd...@cloudera.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review70766
-----------------------------------------------------------



./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java
<https://reviews.apache.org/r/20071/#comment116169>

    Do you think handleException() and notifyStopping are duplicate? Can we keep just one and remove the other?



./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java
<https://reviews.apache.org/r/20071/#comment116170>

    "This will notify the server that some critical thread has stopped. It usually takes place when fatal error occured."


- Hongchao Deng


On Feb. 3, 2015, 8:35 a.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2015, 8:35 a.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1656643 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1656643 
>   ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1656643 
>   ./src/java/test/org/apache/zookeeper/server/SessionTrackerTest.java 1656643 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1656643 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1656643 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1656643 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1656643 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1656643 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review71377
-----------------------------------------------------------



./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
<https://reviews.apache.org/r/20071/#comment117087>

    When we catch and swallow InterruptedException, one common pattern is -  call Thread.currentThread().interrupt() afterward, so that the interrupt status isn't lost.



./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java
<https://reviews.apache.org/r/20071/#comment117088>

    OK, I will change



./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java
<https://reviews.apache.org/r/20071/#comment117089>

    sure, will remove it



./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java
<https://reviews.apache.org/r/20071/#comment117090>

    ok



./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java
<https://reviews.apache.org/r/20071/#comment117091>

    Yes, it synchronization should be improved further. Initially I thought of minimizing the scope of synchronized block, but I think it will not work out. Now, am planning to make both #startup() and #shutdown() synchronized method. Any comments?
    
    synchronized startup()
        //...
    }
    
    synchronized shutdown(){
        if(!isRunning()){ return; }
        super.shutdown();
        // ...  
    }


- Rakesh R


On Feb. 5, 2015, 6:32 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2015, 6:32 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/SessionTrackerTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1657623 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1657623 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.


> On Feb. 5, 2015, 6:50 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java, line 147
> > <https://reviews.apache.org/r/20071/diff/4-5/?file=845237#file845237line147>
> >
> >     Why interrupt() here?
> >     Add comments please.
> 
> Rakesh R wrote:
>     When we catch and swallow InterruptedException, one common pattern is -  call Thread.currentThread().interrupt() afterward, so that the interrupt status isn't lost.

After second thought your point is valid, since this is inside thread there is no effect on setting the status back to the thread. I will remove this.


> On Feb. 5, 2015, 6:50 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java, line 190
> > <https://reviews.apache.org/r/20071/diff/4-5/?file=845243#file845243line190>
> >
> >     Why interrupt?

OK. I will remove it


- Rakesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review71262
-----------------------------------------------------------


On Feb. 5, 2015, 6:32 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2015, 6:32 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/SessionTrackerTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1657623 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1657623 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.


> On Feb. 5, 2015, 6:50 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java, line 155
> > <https://reviews.apache.org/r/20071/diff/4-5/?file=845238#file845238line155>
> >
> >     Why interrupt()?

its not required, removed.


> On Feb. 5, 2015, 6:50 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java, line 147
> > <https://reviews.apache.org/r/20071/diff/4-5/?file=845237#file845237line147>
> >
> >     Why interrupt() here?
> >     Add comments please.
> 
> Rakesh R wrote:
>     When we catch and swallow InterruptedException, one common pattern is -  call Thread.currentThread().interrupt() afterward, so that the interrupt status isn't lost.
> 
> Rakesh R wrote:
>     After second thought your point is valid, since this is inside thread there is no effect on setting the status back to the thread. I will remove this.

its not required, removed.


> On Feb. 5, 2015, 6:50 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java, line 465
> > <https://reviews.apache.org/r/20071/diff/4-5/?file=845241#file845241line465>
> >
> >     Same. Why synchronized here? Some comments will help prevent people in the future move it.

Modified the synchronization to method level


> On Feb. 5, 2015, 6:50 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java, line 408
> > <https://reviews.apache.org/r/20071/diff/4-5/?file=845241#file845241line408>
> >
> >     Why synchronized here not startup() method? Add some comments?

Made synchronization at startup() method


- Rakesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review71262
-----------------------------------------------------------


On Feb. 19, 2015, 3:23 a.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2015, 3:23 a.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1660779 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1660779 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1660779 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1660779 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1660779 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1660779 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.


> On Feb. 5, 2015, 6:50 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java, line 147
> > <https://reviews.apache.org/r/20071/diff/4-5/?file=845237#file845237line147>
> >
> >     Why interrupt() here?
> >     Add comments please.

When we catch and swallow InterruptedException, one common pattern is -  call Thread.currentThread().interrupt() afterward, so that the interrupt status isn't lost.


> On Feb. 5, 2015, 6:50 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java, line 46
> > <https://reviews.apache.org/r/20071/diff/4-5/?file=845240#file845240line46>
> >
> >     thName -> threadName

Ok


> On Feb. 5, 2015, 6:50 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java, line 47
> > <https://reviews.apache.org/r/20071/diff/4-5/?file=845240#file845240line47>
> >
> >     The logging is duplicate to that in notifyStopping().
> >     
> >     I think we should remove the one in notifyStopping() because we might want to print exception stack.

ofcourse, will remove duplicates


> On Feb. 5, 2015, 6:50 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java, line 48
> > <https://reviews.apache.org/r/20071/diff/4-5/?file=845240#file845240line48>
> >
> >     this.getName() -> threadName

ok


> On Feb. 5, 2015, 6:50 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java, line 168
> > <https://reviews.apache.org/r/20071/diff/4-5/?file=845249#file845249line168>
> >
> >     Isn't "running" set to false in super.shutdown()?

Yes, it synchronization should be improved further. Initially I thought of minimizing the scope of synchronized block, but I think it will not work out. Now, am planning to make both #startup() and #shutdown() synchronized methods. Any comments?

synchronized startup()
    //...
}

synchronized shutdown(){
    if(!isRunning()){ return; }
    super.shutdown();
    // ...
}


> On Feb. 5, 2015, 6:50 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java, line 135
> > <https://reviews.apache.org/r/20071/diff/4-5/?file=845252#file845252line135>
> >
> >     Same.. it's always false.

sure, will change this logic.


- Rakesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review71262
-----------------------------------------------------------


On Feb. 5, 2015, 6:32 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2015, 6:32 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/SessionTrackerTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1657623 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1657623 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Hongchao Deng <hd...@cloudera.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review71262
-----------------------------------------------------------



./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
<https://reviews.apache.org/r/20071/#comment116928>

    Why interrupt() here?
    Add comments please.



./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java
<https://reviews.apache.org/r/20071/#comment116929>

    Why interrupt()?



./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java
<https://reviews.apache.org/r/20071/#comment116933>

    thName -> threadName



./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java
<https://reviews.apache.org/r/20071/#comment116931>

    The logging is duplicate to that in notifyStopping().
    
    I think we should remove the one in notifyStopping() because we might want to print exception stack.



./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java
<https://reviews.apache.org/r/20071/#comment116932>

    this.getName() -> threadName



./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
<https://reviews.apache.org/r/20071/#comment116934>

    Why synchronized here not startup() method? Add some comments?



./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
<https://reviews.apache.org/r/20071/#comment116935>

    Same. Why synchronized here? Some comments will help prevent people in the future move it.



./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java
<https://reviews.apache.org/r/20071/#comment116936>

    Why interrupt?



./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java
<https://reviews.apache.org/r/20071/#comment116938>

    Isn't "running" set to false in super.shutdown()?



./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java
<https://reviews.apache.org/r/20071/#comment116939>

    Same.. it's always false.


- Hongchao Deng


On Feb. 5, 2015, 6:32 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2015, 6:32 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/SessionTrackerTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1657623 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1657623 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.


> On Feb. 5, 2015, 6:58 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java, line 115
> > <https://reviews.apache.org/r/20071/diff/5/?file=850832#file850832line115>
> >
> >     And we don't need listener now... Seems like we need to refactor a lot..

ZooKeeperServerListener instance is still maintained at ZooKeeperServer and not exactly singleton. The reason is, ZooKeeperServerListener needs ZooKeeperServer reference to call #shutdown. Do you have anything different approach in your mind. If not, will continue this fashion. ok?


- Rakesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review71269
-----------------------------------------------------------


On Feb. 5, 2015, 6:32 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2015, 6:32 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/SessionTrackerTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1657623 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1657623 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Hongchao Deng <hd...@cloudera.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review71269
-----------------------------------------------------------



./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
<https://reviews.apache.org/r/20071/#comment116943>

    And we don't need listener now... Seems like we need to refactor a lot..


- Hongchao Deng


On Feb. 5, 2015, 6:32 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2015, 6:32 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/SessionTrackerTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1657623 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1657623 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.


> On March 2, 2015, 5:35 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java, line 688
> > <https://reviews.apache.org/r/20071/diff/7/?file=882155#file882155line688>
> >
> >     Adding some comments here would be helpful too.

added comments


- Rakesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review74761
-----------------------------------------------------------


On March 5, 2015, 6:55 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated March 5, 2015, 6:55 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1664445 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1664445 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1664445 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1664445 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1664445 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1664445 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.


> On March 2, 2015, 5:35 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java, line 688
> > <https://reviews.apache.org/r/20071/diff/7/?file=882155#file882155line688>
> >
> >     Adding some comments here would be helpful too.
> 
> Rakesh R wrote:
>     added comments

done


- Rakesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review74761
-----------------------------------------------------------


On March 5, 2015, 6:55 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated March 5, 2015, 6:55 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1664445 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1664445 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1664445 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1664445 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1664445 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1664445 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Hongchao Deng <hd...@cloudera.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review74761
-----------------------------------------------------------



./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
<https://reviews.apache.org/r/20071/#comment121478>

    Adding some comments here would be helpful too.


- Hongchao Deng


On March 2, 2015, 5:26 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated March 2, 2015, 5:26 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1661461 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1661461 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1661461 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1661461 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1661461 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1661461 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/
-----------------------------------------------------------

(Updated March 5, 2015, 6:55 p.m.)


Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.


Bugs: ZOOKEEPER-1907
    https://issues.apache.org/jira/browse/ZOOKEEPER-1907


Repository: zookeeper


Description
-------

Improve the thread handling mechanism by detecting if any of the critical thread dies.
Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.


Diffs (updated)
-----

  ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
  ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1664445 
  ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1664445 
  ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1664445 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1664445 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1664445 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
  ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1664445 
  ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1664445 
  ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1664445 
  ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1664445 
  ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1664445 
  ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1664445 
  ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1664445 
  ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1664445 
  ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1664445 
  ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1664445 
  ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1664445 
  ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1664445 
  ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1664445 
  ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1664445 
  ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1664445 
  ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1664445 
  ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1664445 
  ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1664445 

Diff: https://reviews.apache.org/r/20071/diff/


Testing
-------

yet to be inlcuded


Thanks,

Rakesh R

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.


> On March 2, 2015, 5:30 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java, line 149
> > <https://reviews.apache.org/r/20071/diff/6-7/?file=868720#file868720line149>
> >
> >     not needed

done


> On March 2, 2015, 5:30 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java, line 137
> > <https://reviews.apache.org/r/20071/diff/6-7/?file=868720#file868720line137>
> >
> >     not needed.

done


> On March 2, 2015, 5:30 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java, line 690
> > <https://reviews.apache.org/r/20071/diff/6-7/?file=868720#file868720line690>
> >
> >     state != State.Running.
> 
> Hongchao Deng wrote:
>     Or we can just leave "if (firstProcessor == null) {" unchanged. This is the original behavior.

Modified as state!=State.RUNNING


- Rakesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review74757
-----------------------------------------------------------


On March 5, 2015, 6:55 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated March 5, 2015, 6:55 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1664445 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1664445 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1664445 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1664445 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1664445 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1664445 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Hongchao Deng <hd...@cloudera.com>.


> On March 2, 2015, 5:30 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java, line 690
> > <https://reviews.apache.org/r/20071/diff/6-7/?file=868720#file868720line690>
> >
> >     state != State.Running.

Or we can just leave "if (firstProcessor == null) {" unchanged. This is the original behavior.


- Hongchao


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review74757
-----------------------------------------------------------


On March 2, 2015, 5:26 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated March 2, 2015, 5:26 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1661461 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1661461 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1661461 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1661461 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1661461 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1661461 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.


- Rakesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review74757
-----------------------------------------------------------


On March 5, 2015, 6:55 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated March 5, 2015, 6:55 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1664445 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1664445 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1664445 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1664445 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1664445 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1664445 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1664445 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Hongchao Deng <hd...@cloudera.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review74757
-----------------------------------------------------------



./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
<https://reviews.apache.org/r/20071/#comment121464>

    not needed.



./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
<https://reviews.apache.org/r/20071/#comment121465>

    not needed



./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
<https://reviews.apache.org/r/20071/#comment121466>

    state != State.Running.


- Hongchao Deng


On March 2, 2015, 5:26 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated March 2, 2015, 5:26 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1661461 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1661461 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1661461 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1661461 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1661461 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1661461 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1661461 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/
-----------------------------------------------------------

(Updated March 2, 2015, 5:26 p.m.)


Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.


Bugs: ZOOKEEPER-1907
    https://issues.apache.org/jira/browse/ZOOKEEPER-1907


Repository: zookeeper


Description
-------

Improve the thread handling mechanism by detecting if any of the critical thread dies.
Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.


Diffs (updated)
-----

  ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
  ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1661461 
  ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1661461 
  ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1661461 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1661461 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1661461 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
  ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1661461 
  ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1661461 
  ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1661461 
  ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1661461 
  ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1661461 
  ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1661461 
  ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1661461 
  ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1661461 
  ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1661461 
  ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1661461 
  ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1661461 
  ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1661461 
  ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1661461 
  ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1661461 
  ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1661461 
  ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1661461 
  ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1661461 
  ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1661461 

Diff: https://reviews.apache.org/r/20071/diff/


Testing
-------

yet to be inlcuded


Thanks,

Rakesh R

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/
-----------------------------------------------------------

(Updated Feb. 19, 2015, 3:23 a.m.)


Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.


Bugs: ZOOKEEPER-1907
    https://issues.apache.org/jira/browse/ZOOKEEPER-1907


Repository: zookeeper


Description
-------

Improve the thread handling mechanism by detecting if any of the critical thread dies.
Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.


Diffs (updated)
-----

  ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
  ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1660779 
  ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1660779 
  ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1660779 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1660779 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1660779 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
  ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1660779 
  ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1660779 
  ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1660779 
  ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1660779 
  ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1660779 
  ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1660779 
  ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1660779 
  ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1660779 
  ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1660779 
  ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1660779 
  ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1660779 
  ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1660779 
  ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1660779 
  ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1660779 
  ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1660779 
  ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1660779 
  ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1660779 
  ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1660779 

Diff: https://reviews.apache.org/r/20071/diff/


Testing
-------

yet to be inlcuded


Thanks,

Rakesh R

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.


> On Feb. 18, 2015, 9:54 p.m., Hongchao Deng wrote:
> > ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java, line 66
> > <https://reviews.apache.org/r/20071/diff/5/?file=850859#file850859line66>
> >
> >     Can we change the name to
> >     "testZKSListener" since it's a test local variable. It's really disturbing when grepping "getZKServerListener" :)

done


- Rakesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review73016
-----------------------------------------------------------


On Feb. 19, 2015, 3:23 a.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2015, 3:23 a.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1660779 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1660779 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1660779 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1660779 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1660779 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1660779 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1660779 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Hongchao Deng <hd...@cloudera.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review73016
-----------------------------------------------------------



./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
<https://reviews.apache.org/r/20071/#comment119156>

    Comments here:
    
    "ZooKeeperServerListener instance is maintained at ZooKeeperServer and shared among request processors"



./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java
<https://reviews.apache.org/r/20071/#comment119163>

    Can we change the name to
    "testZKSListener" since it's a test local variable. It's really disturbing when grepping "getZKServerListener" :)


- Hongchao Deng


On Feb. 5, 2015, 6:32 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2015, 6:32 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1657623 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/SessionTrackerTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1657623 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1657623 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1657623 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/
-----------------------------------------------------------

(Updated Feb. 5, 2015, 6:32 p.m.)


Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.


Bugs: ZOOKEEPER-1907
    https://issues.apache.org/jira/browse/ZOOKEEPER-1907


Repository: zookeeper


Description
-------

Improve the thread handling mechanism by detecting if any of the critical thread dies.
Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.


Diffs (updated)
-----

  ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
  ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1657623 
  ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1657623 
  ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1657623 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1657623 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1657623 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
  ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1657623 
  ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1657623 
  ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1657623 
  ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1657623 
  ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1657623 
  ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1657623 
  ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1657623 
  ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1657623 
  ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1657623 
  ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1657623 
  ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1657623 
  ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1657623 
  ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1657623 
  ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1657623 
  ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1657623 
  ./src/java/test/org/apache/zookeeper/server/SessionTrackerTest.java 1657623 
  ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1657623 
  ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1657623 
  ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1657623 
  ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1657623 
  ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1657623 

Diff: https://reviews.apache.org/r/20071/diff/


Testing
-------

yet to be inlcuded


Thanks,

Rakesh R

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/
-----------------------------------------------------------

(Updated Feb. 3, 2015, 8:35 a.m.)


Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.


Bugs: ZOOKEEPER-1907
    https://issues.apache.org/jira/browse/ZOOKEEPER-1907


Repository: zookeeper


Description
-------

Improve the thread handling mechanism by detecting if any of the critical thread dies.
Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.


Diffs (updated)
-----

  ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
  ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1656643 
  ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1656643 
  ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1656643 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1656643 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1656643 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
  ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1656643 
  ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1656643 
  ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1656643 
  ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1656643 
  ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1656643 
  ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1656643 
  ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1656643 
  ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1656643 
  ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1656643 
  ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1656643 
  ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1656643 
  ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1656643 
  ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1656643 
  ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1656643 
  ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1656643 
  ./src/java/test/org/apache/zookeeper/server/SessionTrackerTest.java 1656643 
  ./src/java/test/org/apache/zookeeper/server/ZooKeeperThreadTest.java 1656643 
  ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1656643 
  ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1656643 
  ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1656643 
  ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1656643 

Diff: https://reviews.apache.org/r/20071/diff/


Testing
-------

yet to be inlcuded


Thanks,

Rakesh R

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Hongchao Deng <hd...@cloudera.com>.


> On Jan. 26, 2015, 5:32 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java, line 159
> > <https://reviews.apache.org/r/20071/diff/3/?file=834241#file834241line159>
> >
> >     Is there any race in listener between run() and shutdown()? Maybe some comments?
> 
> Rakesh R wrote:
>     yes. I missed that part. How about making listener as 'volatile' ?
> 
> Hongchao Deng wrote:
>     It's a problem called time-to-check-time-to-use.
> 
> Rakesh R wrote:
>     Then will move the notification inside exception-handling block of each thread. I'm just trying to avoid extra synchronized block. agree?
> 
> Rakesh R wrote:
>     Adding one more point to the above. My intention to make listener=null is, the listener should not be invoked when the thread stops execution gracefully(through #shutdown call).

Adding a synchronized block seems fine to me actually. We don't bother to care about efficiency in shutting down. But making the code have good concurrency practice has more long term benefit.

synchronizing over the exit code, or the singleton listener class, or anything you can think of in any processor class is good here.


- Hongchao


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review69622
-----------------------------------------------------------


On Jan. 26, 2015, 2:12 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2015, 2:12 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/SessionTrackerTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1654804 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1654804 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Hongchao Deng <hd...@cloudera.com>.


> On Jan. 26, 2015, 5:32 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java, line 159
> > <https://reviews.apache.org/r/20071/diff/3/?file=834241#file834241line159>
> >
> >     Is there any race in listener between run() and shutdown()? Maybe some comments?
> 
> Rakesh R wrote:
>     yes. I missed that part. How about making listener as 'volatile' ?

It's a problem called time-to-check-time-to-use.


- Hongchao


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review69622
-----------------------------------------------------------


On Jan. 26, 2015, 2:12 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2015, 2:12 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/SessionTrackerTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1654804 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1654804 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.


> On Jan. 26, 2015, 5:32 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java, line 159
> > <https://reviews.apache.org/r/20071/diff/3/?file=834241#file834241line159>
> >
> >     Is there any race in listener between run() and shutdown()? Maybe some comments?
> 
> Rakesh R wrote:
>     yes. I missed that part. How about making listener as 'volatile' ?
> 
> Hongchao Deng wrote:
>     It's a problem called time-to-check-time-to-use.

Then will move the notification inside exception-handling block of each thread. I'm just trying to avoid extra synchronized block. agree?


- Rakesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review69622
-----------------------------------------------------------


On Jan. 26, 2015, 2:12 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2015, 2:12 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/SessionTrackerTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1654804 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1654804 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.


> On Jan. 26, 2015, 5:32 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java, line 159
> > <https://reviews.apache.org/r/20071/diff/3/?file=834241#file834241line159>
> >
> >     Is there any race in listener between run() and shutdown()? Maybe some comments?
> 
> Rakesh R wrote:
>     yes. I missed that part. How about making listener as 'volatile' ?
> 
> Hongchao Deng wrote:
>     It's a problem called time-to-check-time-to-use.
> 
> Rakesh R wrote:
>     Then will move the notification inside exception-handling block of each thread. I'm just trying to avoid extra synchronized block. agree?
> 
> Rakesh R wrote:
>     Adding one more point to the above. My intention to make listener=null is, the listener should not be invoked when the thread stops execution gracefully(through #shutdown call).
> 
> Hongchao Deng wrote:
>     Adding a synchronized block seems fine to me actually. We don't bother to care about efficiency in shutting down. But making the code have good concurrency practice has more long term benefit.
>     
>     synchronizing over the exit code, or the singleton listener class, or anything you can think of in any processor class is good here.

I've moved the listerner object to the ZKCriticalThread and notified in the exception block. Does this sound good to you?


> On Jan. 26, 2015, 5:32 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java, line 430
> > <https://reviews.apache.org/r/20071/diff/3/?file=834244#file834244line430>
> >
> >     This should be a singleton, right?
> 
> Rakesh R wrote:
>     yes, I will make it singleton.

private final ZooKeeperServerListener listener = new ZooKeeperServerListenerImpl();

Set the listener at ZooKeeperServer level. ok?


- Rakesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review69622
-----------------------------------------------------------


On Jan. 26, 2015, 2:12 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2015, 2:12 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/SessionTrackerTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1654804 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1654804 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.


> On Jan. 26, 2015, 5:32 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java, line 159
> > <https://reviews.apache.org/r/20071/diff/3/?file=834241#file834241line159>
> >
> >     Is there any race in listener between run() and shutdown()? Maybe some comments?

yes. I missed that part. How about making listener as 'volatile' ?


> On Jan. 26, 2015, 5:32 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java, line 430
> > <https://reviews.apache.org/r/20071/diff/3/?file=834244#file834244line430>
> >
> >     This should be a singleton, right?

yes, I will make it singleton.


> On Jan. 26, 2015, 5:32 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java, line 33
> > <https://reviews.apache.org/r/20071/diff/3/?file=834245#file834245line33>
> >
> >     unexpectedError -> errorCode

ok


- Rakesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review69622
-----------------------------------------------------------


On Jan. 26, 2015, 2:12 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2015, 2:12 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/SessionTrackerTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1654804 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1654804 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.


> On Jan. 26, 2015, 5:32 p.m., Hongchao Deng wrote:
> > ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java, line 159
> > <https://reviews.apache.org/r/20071/diff/3/?file=834241#file834241line159>
> >
> >     Is there any race in listener between run() and shutdown()? Maybe some comments?
> 
> Rakesh R wrote:
>     yes. I missed that part. How about making listener as 'volatile' ?
> 
> Hongchao Deng wrote:
>     It's a problem called time-to-check-time-to-use.
> 
> Rakesh R wrote:
>     Then will move the notification inside exception-handling block of each thread. I'm just trying to avoid extra synchronized block. agree?

Adding one more point to the above. My intention to make listener=null is, the listener should not be invoked when the thread stops execution gracefully(through #shutdown call).


- Rakesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review69622
-----------------------------------------------------------


On Jan. 26, 2015, 2:12 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2015, 2:12 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/SessionTrackerTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1654804 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1654804 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Hongchao Deng <hd...@cloudera.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/#review69622
-----------------------------------------------------------



./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
<https://reviews.apache.org/r/20071/#comment114345>

    Is there any race in listener between run() and shutdown()? Maybe some comments?



./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
<https://reviews.apache.org/r/20071/#comment114350>

    This should be a singleton, right?



./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java
<https://reviews.apache.org/r/20071/#comment114352>

    unexpectedError -> errorCode


- Hongchao Deng


On Jan. 26, 2015, 2:12 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20071/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2015, 2:12 p.m.)
> 
> 
> Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.
> 
> 
> Bugs: ZOOKEEPER-1907
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1907
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> Improve the thread handling mechanism by detecting if any of the critical thread dies.
> Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
>   ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1654804 
>   ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/SessionTrackerTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1654804 
>   ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1654804 
>   ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1654804 
> 
> Diff: https://reviews.apache.org/r/20071/diff/
> 
> 
> Testing
> -------
> 
> yet to be inlcuded
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/
-----------------------------------------------------------

(Updated Jan. 26, 2015, 2:12 p.m.)


Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.


Changes
-------

Attached new patch where I've tried another better approach. Introduced ZooKeeperServerListener - will notify the critical resource(thread) failures and act upon. Appreciate any comments. Thanks!


Bugs: ZOOKEEPER-1907
    https://issues.apache.org/jira/browse/ZOOKEEPER-1907


Repository: zookeeper


Description
-------

Improve the thread handling mechanism by detecting if any of the critical thread dies.
Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.


Diffs (updated)
-----

  ./src/java/main/org/apache/zookeeper/server/ExitCode.java PRE-CREATION 
  ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1654804 
  ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1654804 
  ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1654804 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1654804 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerListener.java PRE-CREATION 
  ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1654804 
  ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1654804 
  ./src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java 1654804 
  ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1654804 
  ./src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1654804 
  ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1654804 
  ./src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1654804 
  ./src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java 1654804 
  ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1654804 
  ./src/java/main/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java 1654804 
  ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1654804 
  ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1654804 
  ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1654804 
  ./src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java 1654804 
  ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1654804 
  ./src/java/test/org/apache/zookeeper/server/SessionTrackerTest.java 1654804 
  ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java 1654804 
  ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1654804 
  ./src/java/test/org/apache/zookeeper/test/ClientBase.java 1654804 
  ./src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java 1654804 

Diff: https://reviews.apache.org/r/20071/diff/


Testing
-------

yet to be inlcuded


Thanks,

Rakesh R

Re: Review Request 20071: ZOOKEEPER-1907]-mprove the thread handling by detecting if any of the critical thread dies.

Posted by Rakesh R <ra...@huawei.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20071/
-----------------------------------------------------------

(Updated May 28, 2014, 12:26 p.m.)


Review request for zookeeper, michim, Raul Gutierrez Segales, and Camille Fournier.


Changes
-------

Attached new patch addressing Raul's comments


Bugs: ZOOKEEPER-1907
    https://issues.apache.org/jira/browse/ZOOKEEPER-1907


Repository: zookeeper


Description
-------

Improve the thread handling mechanism by detecting if any of the critical thread dies.
Here the idea is to periodically checking the status of all the critical threads in ZK server using DeathWatcherThread.


Diffs (updated)
-----

  ./src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/RequestProcessor.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/ServerConfig.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/SessionTracker.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/UnimplementedRequestProcessor.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperCriticalThread.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/quorum/AckRequestProcessor.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/quorum/LeaderRequestProcessor.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1597987 
  ./src/java/main/org/apache/zookeeper/server/quorum/SendAckRequestProcessor.java 1597987 
  ./src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1597987 
  ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerTest.java 1597987 
  ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java 1597987 

Diff: https://reviews.apache.org/r/20071/diff/


Testing
-------

yet to be inlcuded


Thanks,

Rakesh R