You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by Thawan Kooburat <th...@fb.com> on 2013/02/20 02:00:57 UTC

Re: Review Request: ZOOKEEPER-1147: Add support for local sessions

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

(Updated Feb. 20, 2013, 1 a.m.)


Review request for zookeeper, Patrick Hunt and Mahadev Konar.


Changes
-------

- extract watcher test fix to a separate patch
- change ConcurentHashMap to ConcurrentMap in SessionTracker classes
- use ClientBase.CONNECTION_TIMEOUT for connection timeout whenever possible
- return false when ephemeral node creation failed


Description
-------

See ZOOKEEPER-1147 for high level description 

Implementation notes:

- Local sessions don’t get persisted on disk.  Existing SessionTrackerImpl is used by the leader to track global sessions.  Each participant (including the leader) also has a local session tracker (which is another instance of SessionTrackerImpl) to track its local session in memory. 

- Each participant intercepts a request before it enters the pipeline. In order to do 2 things
 o Update local session to global session when it saw create ephemeral node request. Update is done by issuing a create session rquest before issuing the create request
 o Add local session flag to a request. So that the pipeline know that this type of request don’t need to send to the leader and wait for commit

- PrepRequestProcessor (on the leader) now explicitly validate global session on create ephemeral node request.  For other type of request, there is no need for session validation because the leader doesn’t know about local sessions on other machine, so the request has to go through.  The correctness is preserve as long a no ephemeral node is created after session is already expired

- Observer/FollowerRequestProcessor has logic to validate session. However, I believe this logic is in correct since the request is already send downstream to the CommitProcessor. The observer and the follower have no other option but to send the request to leader. 


This addresses bug ZOOKEEPER-1147.
    https://issues.apache.org/jira/browse/ZOOKEEPER-1147


Diffs (updated)
-----

  /src/java/main/org/apache/zookeeper/KeeperException.java 1446831 
  /src/java/main/org/apache/zookeeper/cli/CreateCommand.java 1446831 
  /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1446831 
  /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1446831 
  /src/java/main/org/apache/zookeeper/server/Request.java 1446831 
  /src/java/main/org/apache/zookeeper/server/SessionTracker.java 1446831 
  /src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1446831 
  /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1446831 
  /src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1446831 
  /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java 1446831 
  /src/java/test/org/apache/zookeeper/test/LeaderSessionTrackerTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/LocalSessionRequestTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/LocalSessionsOnlyTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/QuorumBase.java 1446831 
  /src/java/test/org/apache/zookeeper/test/QuorumTest.java 1446831 
  /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1446831 
  /src/java/test/org/apache/zookeeper/test/ReadOnlyModeTest.java 1446831 
  /src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/SessionUpgradeTest.java PRE-CREATION 

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


Testing
-------

unit tests.  For our internal branch, we haven't run into bugs related to local session for quite a while so it is quite stable. We enabled local session by default in all our deployment.


Thanks,

Thawan Kooburat


Re: Review Request: ZOOKEEPER-1147: Add support for local sessions

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8935/#review18307
-----------------------------------------------------------



/src/java/test/org/apache/zookeeper/test/QuorumUtil.java
<https://reviews.apache.org/r/8935/#comment38514>

    Lines 273-279 can be rewritten as:
    
    List<QuorumPeer> peerList = new ArrayList<QuorumPeer>(ALL - 1);	
    
    for (Set<PeerStruct> ps: peers.values()) {
        if (ps.peer.leader == null) {
           peerList.add(ps.peer);      
        }
    }
    
    return Collections.unmodifiableList(peerList);


- Edward Ribeiro


On March 6, 2013, 11:42 p.m., Thawan Kooburat wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8935/
> -----------------------------------------------------------
> 
> (Updated March 6, 2013, 11:42 p.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt and Mahadev Konar.
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1147 for high level description 
> 
> Implementation notes:
> 
> - Local sessions don’t get persisted on disk.  Existing SessionTrackerImpl is used by the leader to track global sessions.  Each participant (including the leader) also has a local session tracker (which is another instance of SessionTrackerImpl) to track its local session in memory. 
> 
> - Each participant intercepts a request before it enters the pipeline. In order to do 2 things
>  o Update local session to global session when it saw create ephemeral node request. Update is done by issuing a create session rquest before issuing the create request
>  o Add local session flag to a request. So that the pipeline know that this type of request don’t need to send to the leader and wait for commit
> 
> - PrepRequestProcessor (on the leader) now explicitly validate global session on create ephemeral node request.  For other type of request, there is no need for session validation because the leader doesn’t know about local sessions on other machine, so the request has to go through.  The correctness is preserve as long a no ephemeral node is created after session is already expired
> 
> - Observer/FollowerRequestProcessor has logic to validate session. However, I believe this logic is in correct since the request is already send downstream to the CommitProcessor. The observer and the follower have no other option but to send the request to leader. 
> 
> 
> This addresses bug ZOOKEEPER-1147.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1147
> 
> 
> Diffs
> -----
> 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1448007 
>   /src/java/main/org/apache/zookeeper/cli/CreateCommand.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/SessionTracker.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderRequestProcessor.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/DuplicateLocalSessionUpgradeTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LeaderSessionTrackerTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LocalSessionRequestTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LocalSessionsOnlyTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumBase.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/QuorumTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/ReadOnlyModeTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/SessionUpgradeTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8935/diff/
> 
> 
> Testing
> -------
> 
> unit tests.  For our internal branch, we haven't run into bugs related to local session for quite a while so it is quite stable. We enabled local session by default in all our deployment.
> 
> 
> Thanks,
> 
> Thawan Kooburat
> 
>


Re: Review Request: ZOOKEEPER-1147: Add support for local sessions

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8935/#review18306
-----------------------------------------------------------



/src/java/test/org/apache/zookeeper/test/QuorumUtil.java
<https://reviews.apache.org/r/8935/#comment38513>

    It's not a good practice to return null. Maybe throw an exception if this situation should never happen or return a dummy QuorumPeer is a better approach, imo.


- Edward Ribeiro


On March 6, 2013, 11:42 p.m., Thawan Kooburat wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8935/
> -----------------------------------------------------------
> 
> (Updated March 6, 2013, 11:42 p.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt and Mahadev Konar.
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1147 for high level description 
> 
> Implementation notes:
> 
> - Local sessions don’t get persisted on disk.  Existing SessionTrackerImpl is used by the leader to track global sessions.  Each participant (including the leader) also has a local session tracker (which is another instance of SessionTrackerImpl) to track its local session in memory. 
> 
> - Each participant intercepts a request before it enters the pipeline. In order to do 2 things
>  o Update local session to global session when it saw create ephemeral node request. Update is done by issuing a create session rquest before issuing the create request
>  o Add local session flag to a request. So that the pipeline know that this type of request don’t need to send to the leader and wait for commit
> 
> - PrepRequestProcessor (on the leader) now explicitly validate global session on create ephemeral node request.  For other type of request, there is no need for session validation because the leader doesn’t know about local sessions on other machine, so the request has to go through.  The correctness is preserve as long a no ephemeral node is created after session is already expired
> 
> - Observer/FollowerRequestProcessor has logic to validate session. However, I believe this logic is in correct since the request is already send downstream to the CommitProcessor. The observer and the follower have no other option but to send the request to leader. 
> 
> 
> This addresses bug ZOOKEEPER-1147.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1147
> 
> 
> Diffs
> -----
> 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1448007 
>   /src/java/main/org/apache/zookeeper/cli/CreateCommand.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/SessionTracker.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderRequestProcessor.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/DuplicateLocalSessionUpgradeTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LeaderSessionTrackerTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LocalSessionRequestTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LocalSessionsOnlyTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumBase.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/QuorumTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/ReadOnlyModeTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/SessionUpgradeTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8935/diff/
> 
> 
> Testing
> -------
> 
> unit tests.  For our internal branch, we haven't run into bugs related to local session for quite a while so it is quite stable. We enabled local session by default in all our deployment.
> 
> 
> Thanks,
> 
> Thawan Kooburat
> 
>


Re: Review Request: ZOOKEEPER-1147: Add support for local sessions

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8935/#review18299
-----------------------------------------------------------



/src/java/main/org/apache/zookeeper/server/quorum/LeaderRequestProcessor.java
<https://reviews.apache.org/r/8935/#comment38500>

    Add final keyword.


- Edward Ribeiro


On March 6, 2013, 11:42 p.m., Thawan Kooburat wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8935/
> -----------------------------------------------------------
> 
> (Updated March 6, 2013, 11:42 p.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt and Mahadev Konar.
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1147 for high level description 
> 
> Implementation notes:
> 
> - Local sessions don’t get persisted on disk.  Existing SessionTrackerImpl is used by the leader to track global sessions.  Each participant (including the leader) also has a local session tracker (which is another instance of SessionTrackerImpl) to track its local session in memory. 
> 
> - Each participant intercepts a request before it enters the pipeline. In order to do 2 things
>  o Update local session to global session when it saw create ephemeral node request. Update is done by issuing a create session rquest before issuing the create request
>  o Add local session flag to a request. So that the pipeline know that this type of request don’t need to send to the leader and wait for commit
> 
> - PrepRequestProcessor (on the leader) now explicitly validate global session on create ephemeral node request.  For other type of request, there is no need for session validation because the leader doesn’t know about local sessions on other machine, so the request has to go through.  The correctness is preserve as long a no ephemeral node is created after session is already expired
> 
> - Observer/FollowerRequestProcessor has logic to validate session. However, I believe this logic is in correct since the request is already send downstream to the CommitProcessor. The observer and the follower have no other option but to send the request to leader. 
> 
> 
> This addresses bug ZOOKEEPER-1147.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1147
> 
> 
> Diffs
> -----
> 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1448007 
>   /src/java/main/org/apache/zookeeper/cli/CreateCommand.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/SessionTracker.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderRequestProcessor.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/DuplicateLocalSessionUpgradeTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LeaderSessionTrackerTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LocalSessionRequestTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LocalSessionsOnlyTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumBase.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/QuorumTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/ReadOnlyModeTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/SessionUpgradeTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8935/diff/
> 
> 
> Testing
> -------
> 
> unit tests.  For our internal branch, we haven't run into bugs related to local session for quite a while so it is quite stable. We enabled local session by default in all our deployment.
> 
> 
> Thanks,
> 
> Thawan Kooburat
> 
>


Re: Review Request: ZOOKEEPER-1147: Add support for local sessions

Posted by Thawan Kooburat <th...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8935/
-----------------------------------------------------------

(Updated April 24, 2013, 1:31 a.m.)


Review request for zookeeper, Patrick Hunt and Mahadev Konar.


Changes
-------

-Rebase with current trunk (merge with dynamic config patch)
-Address comments provided by Edward
-Change error code due to conflict with trunk


Description
-------

See ZOOKEEPER-1147 for high level description 

Implementation notes:

- Local sessions don’t get persisted on disk.  Existing SessionTrackerImpl is used by the leader to track global sessions.  Each participant (including the leader) also has a local session tracker (which is another instance of SessionTrackerImpl) to track its local session in memory. 

- Each participant intercepts a request before it enters the pipeline. In order to do 2 things
 o Update local session to global session when it saw create ephemeral node request. Update is done by issuing a create session rquest before issuing the create request
 o Add local session flag to a request. So that the pipeline know that this type of request don’t need to send to the leader and wait for commit

- PrepRequestProcessor (on the leader) now explicitly validate global session on create ephemeral node request.  For other type of request, there is no need for session validation because the leader doesn’t know about local sessions on other machine, so the request has to go through.  The correctness is preserve as long a no ephemeral node is created after session is already expired

- Observer/FollowerRequestProcessor has logic to validate session. However, I believe this logic is in correct since the request is already send downstream to the CommitProcessor. The observer and the follower have no other option but to send the request to leader. 


This addresses bug ZOOKEEPER-1147.
    https://issues.apache.org/jira/browse/ZOOKEEPER-1147


Diffs (updated)
-----

  /src/c/include/zookeeper.h 1463329 
  /src/java/main/org/apache/zookeeper/KeeperException.java 1463329 
  /src/java/main/org/apache/zookeeper/cli/CreateCommand.java 1463329 
  /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1463329 
  /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1463329 
  /src/java/main/org/apache/zookeeper/server/Request.java 1463329 
  /src/java/main/org/apache/zookeeper/server/SessionTracker.java 1463329 
  /src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1463329 
  /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1463329 
  /src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1463329 
  /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1463329 
  /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1463329 
  /src/java/main/org/apache/zookeeper/server/quorum/LeaderRequestProcessor.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1463329 
  /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1463329 
  /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1463329 
  /src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1463329 
  /src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1463329 
  /src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1463329 
  /src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1463329 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1463329 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1463329 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1463329 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1463329 
  /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1463329 
  /src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1463329 
  /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java 1463329 
  /src/java/test/org/apache/zookeeper/test/DuplicateLocalSessionUpgradeTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/LeaderSessionTrackerTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/LocalSessionRequestTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/LocalSessionsOnlyTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/QuorumBase.java 1463329 
  /src/java/test/org/apache/zookeeper/test/QuorumTest.java 1463329 
  /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1463329 
  /src/java/test/org/apache/zookeeper/test/ReadOnlyModeTest.java 1463329 
  /src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/SessionUpgradeTest.java PRE-CREATION 

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


Testing
-------

unit tests.  For our internal branch, we haven't run into bugs related to local session for quite a while so it is quite stable. We enabled local session by default in all our deployment.


Thanks,

Thawan Kooburat


Re: Review Request: ZOOKEEPER-1147: Add support for local sessions

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8935/#review18300
-----------------------------------------------------------



/src/java/main/org/apache/zookeeper/server/quorum/LeaderRequestProcessor.java
<https://reviews.apache.org/r/8935/#comment38501>

    Add final keyword.


- Edward Ribeiro


On March 6, 2013, 11:42 p.m., Thawan Kooburat wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8935/
> -----------------------------------------------------------
> 
> (Updated March 6, 2013, 11:42 p.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt and Mahadev Konar.
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1147 for high level description 
> 
> Implementation notes:
> 
> - Local sessions don’t get persisted on disk.  Existing SessionTrackerImpl is used by the leader to track global sessions.  Each participant (including the leader) also has a local session tracker (which is another instance of SessionTrackerImpl) to track its local session in memory. 
> 
> - Each participant intercepts a request before it enters the pipeline. In order to do 2 things
>  o Update local session to global session when it saw create ephemeral node request. Update is done by issuing a create session rquest before issuing the create request
>  o Add local session flag to a request. So that the pipeline know that this type of request don’t need to send to the leader and wait for commit
> 
> - PrepRequestProcessor (on the leader) now explicitly validate global session on create ephemeral node request.  For other type of request, there is no need for session validation because the leader doesn’t know about local sessions on other machine, so the request has to go through.  The correctness is preserve as long a no ephemeral node is created after session is already expired
> 
> - Observer/FollowerRequestProcessor has logic to validate session. However, I believe this logic is in correct since the request is already send downstream to the CommitProcessor. The observer and the follower have no other option but to send the request to leader. 
> 
> 
> This addresses bug ZOOKEEPER-1147.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1147
> 
> 
> Diffs
> -----
> 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1448007 
>   /src/java/main/org/apache/zookeeper/cli/CreateCommand.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/SessionTracker.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderRequestProcessor.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/DuplicateLocalSessionUpgradeTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LeaderSessionTrackerTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LocalSessionRequestTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LocalSessionsOnlyTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumBase.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/QuorumTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/ReadOnlyModeTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/SessionUpgradeTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8935/diff/
> 
> 
> Testing
> -------
> 
> unit tests.  For our internal branch, we haven't run into bugs related to local session for quite a while so it is quite stable. We enabled local session by default in all our deployment.
> 
> 
> Thanks,
> 
> Thawan Kooburat
> 
>


Re: Review Request: ZOOKEEPER-1147: Add support for local sessions

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8935/#review18305
-----------------------------------------------------------



/src/java/test/org/apache/zookeeper/test/QuorumUtil.java
<https://reviews.apache.org/r/8935/#comment38512>

    You can replace lines 264-268 by:
    
    for (Set<PeerStruct> ps: peers.values()) {
        if (ps.peer.leader != null) {
           return ps.peer;
        }
    }
    
    Advantages are: you don't need multiples .get(i) and simplifies code a little bit.


- Edward Ribeiro


On March 6, 2013, 11:42 p.m., Thawan Kooburat wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8935/
> -----------------------------------------------------------
> 
> (Updated March 6, 2013, 11:42 p.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt and Mahadev Konar.
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1147 for high level description 
> 
> Implementation notes:
> 
> - Local sessions don’t get persisted on disk.  Existing SessionTrackerImpl is used by the leader to track global sessions.  Each participant (including the leader) also has a local session tracker (which is another instance of SessionTrackerImpl) to track its local session in memory. 
> 
> - Each participant intercepts a request before it enters the pipeline. In order to do 2 things
>  o Update local session to global session when it saw create ephemeral node request. Update is done by issuing a create session rquest before issuing the create request
>  o Add local session flag to a request. So that the pipeline know that this type of request don’t need to send to the leader and wait for commit
> 
> - PrepRequestProcessor (on the leader) now explicitly validate global session on create ephemeral node request.  For other type of request, there is no need for session validation because the leader doesn’t know about local sessions on other machine, so the request has to go through.  The correctness is preserve as long a no ephemeral node is created after session is already expired
> 
> - Observer/FollowerRequestProcessor has logic to validate session. However, I believe this logic is in correct since the request is already send downstream to the CommitProcessor. The observer and the follower have no other option but to send the request to leader. 
> 
> 
> This addresses bug ZOOKEEPER-1147.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1147
> 
> 
> Diffs
> -----
> 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1448007 
>   /src/java/main/org/apache/zookeeper/cli/CreateCommand.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/SessionTracker.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderRequestProcessor.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/DuplicateLocalSessionUpgradeTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LeaderSessionTrackerTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LocalSessionRequestTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LocalSessionsOnlyTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumBase.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/QuorumTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/ReadOnlyModeTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/SessionUpgradeTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8935/diff/
> 
> 
> Testing
> -------
> 
> unit tests.  For our internal branch, we haven't run into bugs related to local session for quite a while so it is quite stable. We enabled local session by default in all our deployment.
> 
> 
> Thanks,
> 
> Thawan Kooburat
> 
>


Re: Review Request: ZOOKEEPER-1147: Add support for local sessions

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8935/#review18297
-----------------------------------------------------------



/src/java/main/org/apache/zookeeper/KeeperException.java
<https://reviews.apache.org/r/8935/#comment38497>

    It would be nice to remove this line.


- Edward Ribeiro


On March 6, 2013, 11:42 p.m., Thawan Kooburat wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8935/
> -----------------------------------------------------------
> 
> (Updated March 6, 2013, 11:42 p.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt and Mahadev Konar.
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1147 for high level description 
> 
> Implementation notes:
> 
> - Local sessions don’t get persisted on disk.  Existing SessionTrackerImpl is used by the leader to track global sessions.  Each participant (including the leader) also has a local session tracker (which is another instance of SessionTrackerImpl) to track its local session in memory. 
> 
> - Each participant intercepts a request before it enters the pipeline. In order to do 2 things
>  o Update local session to global session when it saw create ephemeral node request. Update is done by issuing a create session rquest before issuing the create request
>  o Add local session flag to a request. So that the pipeline know that this type of request don’t need to send to the leader and wait for commit
> 
> - PrepRequestProcessor (on the leader) now explicitly validate global session on create ephemeral node request.  For other type of request, there is no need for session validation because the leader doesn’t know about local sessions on other machine, so the request has to go through.  The correctness is preserve as long a no ephemeral node is created after session is already expired
> 
> - Observer/FollowerRequestProcessor has logic to validate session. However, I believe this logic is in correct since the request is already send downstream to the CommitProcessor. The observer and the follower have no other option but to send the request to leader. 
> 
> 
> This addresses bug ZOOKEEPER-1147.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1147
> 
> 
> Diffs
> -----
> 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1448007 
>   /src/java/main/org/apache/zookeeper/cli/CreateCommand.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/SessionTracker.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderRequestProcessor.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/DuplicateLocalSessionUpgradeTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LeaderSessionTrackerTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LocalSessionRequestTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LocalSessionsOnlyTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumBase.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/QuorumTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/ReadOnlyModeTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/SessionUpgradeTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8935/diff/
> 
> 
> Testing
> -------
> 
> unit tests.  For our internal branch, we haven't run into bugs related to local session for quite a while so it is quite stable. We enabled local session by default in all our deployment.
> 
> 
> Thanks,
> 
> Thawan Kooburat
> 
>


Re: Review Request: ZOOKEEPER-1147: Add support for local sessions

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8935/#review18301
-----------------------------------------------------------



/src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java
<https://reviews.apache.org/r/8935/#comment38502>

    You may replace lines 198-200 by:
    
    SortedSet<Long> sessionIds = new TreeSet<Long>(globalSessionsWithTimeouts.keySet());
    
    I will 'automatically' order the longs (and avoid duplicates, even though this is not possible in this context) so that you don't even need to call Collections.sort(). 


- Edward Ribeiro


On March 6, 2013, 11:42 p.m., Thawan Kooburat wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8935/
> -----------------------------------------------------------
> 
> (Updated March 6, 2013, 11:42 p.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt and Mahadev Konar.
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1147 for high level description 
> 
> Implementation notes:
> 
> - Local sessions don’t get persisted on disk.  Existing SessionTrackerImpl is used by the leader to track global sessions.  Each participant (including the leader) also has a local session tracker (which is another instance of SessionTrackerImpl) to track its local session in memory. 
> 
> - Each participant intercepts a request before it enters the pipeline. In order to do 2 things
>  o Update local session to global session when it saw create ephemeral node request. Update is done by issuing a create session rquest before issuing the create request
>  o Add local session flag to a request. So that the pipeline know that this type of request don’t need to send to the leader and wait for commit
> 
> - PrepRequestProcessor (on the leader) now explicitly validate global session on create ephemeral node request.  For other type of request, there is no need for session validation because the leader doesn’t know about local sessions on other machine, so the request has to go through.  The correctness is preserve as long a no ephemeral node is created after session is already expired
> 
> - Observer/FollowerRequestProcessor has logic to validate session. However, I believe this logic is in correct since the request is already send downstream to the CommitProcessor. The observer and the follower have no other option but to send the request to leader. 
> 
> 
> This addresses bug ZOOKEEPER-1147.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1147
> 
> 
> Diffs
> -----
> 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1448007 
>   /src/java/main/org/apache/zookeeper/cli/CreateCommand.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/SessionTracker.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderRequestProcessor.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/DuplicateLocalSessionUpgradeTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LeaderSessionTrackerTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LocalSessionRequestTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LocalSessionsOnlyTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumBase.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/QuorumTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/ReadOnlyModeTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/SessionUpgradeTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8935/diff/
> 
> 
> Testing
> -------
> 
> unit tests.  For our internal branch, we haven't run into bugs related to local session for quite a while so it is quite stable. We enabled local session by default in all our deployment.
> 
> 
> Thanks,
> 
> Thawan Kooburat
> 
>


Re: Review Request: ZOOKEEPER-1147: Add support for local sessions

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8935/#review18302
-----------------------------------------------------------



/src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java
<https://reviews.apache.org/r/8935/#comment38503>

    It'd be nice to replace ConcurrentHashMap by ConcurrentMap interface. 


- Edward Ribeiro


On March 6, 2013, 11:42 p.m., Thawan Kooburat wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8935/
> -----------------------------------------------------------
> 
> (Updated March 6, 2013, 11:42 p.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt and Mahadev Konar.
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1147 for high level description 
> 
> Implementation notes:
> 
> - Local sessions don’t get persisted on disk.  Existing SessionTrackerImpl is used by the leader to track global sessions.  Each participant (including the leader) also has a local session tracker (which is another instance of SessionTrackerImpl) to track its local session in memory. 
> 
> - Each participant intercepts a request before it enters the pipeline. In order to do 2 things
>  o Update local session to global session when it saw create ephemeral node request. Update is done by issuing a create session rquest before issuing the create request
>  o Add local session flag to a request. So that the pipeline know that this type of request don’t need to send to the leader and wait for commit
> 
> - PrepRequestProcessor (on the leader) now explicitly validate global session on create ephemeral node request.  For other type of request, there is no need for session validation because the leader doesn’t know about local sessions on other machine, so the request has to go through.  The correctness is preserve as long a no ephemeral node is created after session is already expired
> 
> - Observer/FollowerRequestProcessor has logic to validate session. However, I believe this logic is in correct since the request is already send downstream to the CommitProcessor. The observer and the follower have no other option but to send the request to leader. 
> 
> 
> This addresses bug ZOOKEEPER-1147.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1147
> 
> 
> Diffs
> -----
> 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1448007 
>   /src/java/main/org/apache/zookeeper/cli/CreateCommand.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/SessionTracker.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderRequestProcessor.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1448007 
>   /src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/DuplicateLocalSessionUpgradeTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LeaderSessionTrackerTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LocalSessionRequestTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/LocalSessionsOnlyTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumBase.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/QuorumTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/ReadOnlyModeTest.java 1448007 
>   /src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/SessionUpgradeTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8935/diff/
> 
> 
> Testing
> -------
> 
> unit tests.  For our internal branch, we haven't run into bugs related to local session for quite a while so it is quite stable. We enabled local session by default in all our deployment.
> 
> 
> Thanks,
> 
> Thawan Kooburat
> 
>


Re: Review Request: ZOOKEEPER-1147: Add support for local sessions

Posted by Thawan Kooburat <th...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8935/
-----------------------------------------------------------

(Updated March 6, 2013, 11:42 p.m.)


Review request for zookeeper, Patrick Hunt and Mahadev Konar.


Changes
-------

Fix duplicate CreateSession request when local session upgrade is made by the learner. See comments in JIRA for more details 


Description
-------

See ZOOKEEPER-1147 for high level description 

Implementation notes:

- Local sessions don’t get persisted on disk.  Existing SessionTrackerImpl is used by the leader to track global sessions.  Each participant (including the leader) also has a local session tracker (which is another instance of SessionTrackerImpl) to track its local session in memory. 

- Each participant intercepts a request before it enters the pipeline. In order to do 2 things
 o Update local session to global session when it saw create ephemeral node request. Update is done by issuing a create session rquest before issuing the create request
 o Add local session flag to a request. So that the pipeline know that this type of request don’t need to send to the leader and wait for commit

- PrepRequestProcessor (on the leader) now explicitly validate global session on create ephemeral node request.  For other type of request, there is no need for session validation because the leader doesn’t know about local sessions on other machine, so the request has to go through.  The correctness is preserve as long a no ephemeral node is created after session is already expired

- Observer/FollowerRequestProcessor has logic to validate session. However, I believe this logic is in correct since the request is already send downstream to the CommitProcessor. The observer and the follower have no other option but to send the request to leader. 


This addresses bug ZOOKEEPER-1147.
    https://issues.apache.org/jira/browse/ZOOKEEPER-1147


Diffs (updated)
-----

  /src/java/main/org/apache/zookeeper/KeeperException.java 1448007 
  /src/java/main/org/apache/zookeeper/cli/CreateCommand.java 1448007 
  /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1448007 
  /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1448007 
  /src/java/main/org/apache/zookeeper/server/Request.java 1448007 
  /src/java/main/org/apache/zookeeper/server/SessionTracker.java 1448007 
  /src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1448007 
  /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1448007 
  /src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1448007 
  /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1448007 
  /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1448007 
  /src/java/main/org/apache/zookeeper/server/quorum/LeaderRequestProcessor.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1448007 
  /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1448007 
  /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1448007 
  /src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1448007 
  /src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1448007 
  /src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1448007 
  /src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1448007 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1448007 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1448007 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1448007 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1448007 
  /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1448007 
  /src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1448007 
  /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java 1448007 
  /src/java/test/org/apache/zookeeper/test/DuplicateLocalSessionUpgradeTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/LeaderSessionTrackerTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/LocalSessionRequestTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/LocalSessionsOnlyTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/QuorumBase.java 1448007 
  /src/java/test/org/apache/zookeeper/test/QuorumTest.java 1448007 
  /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1448007 
  /src/java/test/org/apache/zookeeper/test/ReadOnlyModeTest.java 1448007 
  /src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/SessionUpgradeTest.java PRE-CREATION 

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


Testing
-------

unit tests.  For our internal branch, we haven't run into bugs related to local session for quite a while so it is quite stable. We enabled local session by default in all our deployment.


Thanks,

Thawan Kooburat


Re: Review Request: ZOOKEEPER-1147: Add support for local sessions

Posted by Thawan Kooburat <th...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8935/
-----------------------------------------------------------

(Updated March 2, 2013, 8:03 p.m.)


Review request for zookeeper, Patrick Hunt and Mahadev Konar.


Changes
-------

- Extract check session logic for create request into a separate method
- Remove checkSession() call in Follower/ObserverRequestProcessor


Description
-------

See ZOOKEEPER-1147 for high level description 

Implementation notes:

- Local sessions don’t get persisted on disk.  Existing SessionTrackerImpl is used by the leader to track global sessions.  Each participant (including the leader) also has a local session tracker (which is another instance of SessionTrackerImpl) to track its local session in memory. 

- Each participant intercepts a request before it enters the pipeline. In order to do 2 things
 o Update local session to global session when it saw create ephemeral node request. Update is done by issuing a create session rquest before issuing the create request
 o Add local session flag to a request. So that the pipeline know that this type of request don’t need to send to the leader and wait for commit

- PrepRequestProcessor (on the leader) now explicitly validate global session on create ephemeral node request.  For other type of request, there is no need for session validation because the leader doesn’t know about local sessions on other machine, so the request has to go through.  The correctness is preserve as long a no ephemeral node is created after session is already expired

- Observer/FollowerRequestProcessor has logic to validate session. However, I believe this logic is in correct since the request is already send downstream to the CommitProcessor. The observer and the follower have no other option but to send the request to leader. 


This addresses bug ZOOKEEPER-1147.
    https://issues.apache.org/jira/browse/ZOOKEEPER-1147


Diffs (updated)
-----

  /src/java/main/org/apache/zookeeper/KeeperException.java 1446831 
  /src/java/main/org/apache/zookeeper/cli/CreateCommand.java 1446831 
  /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1446831 
  /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1446831 
  /src/java/main/org/apache/zookeeper/server/Request.java 1446831 
  /src/java/main/org/apache/zookeeper/server/SessionTracker.java 1446831 
  /src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1446831 
  /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1446831 
  /src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1446831 
  /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java 1446831 
  /src/java/test/org/apache/zookeeper/test/LeaderSessionTrackerTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/LocalSessionRequestTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/LocalSessionsOnlyTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/QuorumBase.java 1446831 
  /src/java/test/org/apache/zookeeper/test/QuorumTest.java 1446831 
  /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1446831 
  /src/java/test/org/apache/zookeeper/test/ReadOnlyModeTest.java 1446831 
  /src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/SessionUpgradeTest.java PRE-CREATION 

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


Testing
-------

unit tests.  For our internal branch, we haven't run into bugs related to local session for quite a while so it is quite stable. We enabled local session by default in all our deployment.


Thanks,

Thawan Kooburat


Re: Review Request: ZOOKEEPER-1147: Add support for local sessions

Posted by Thawan Kooburat <th...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8935/
-----------------------------------------------------------

(Updated Feb. 20, 2013, 2:20 a.m.)


Review request for zookeeper, Patrick Hunt and Mahadev Konar.


Changes
-------

Upload the right file


Description
-------

See ZOOKEEPER-1147 for high level description 

Implementation notes:

- Local sessions don’t get persisted on disk.  Existing SessionTrackerImpl is used by the leader to track global sessions.  Each participant (including the leader) also has a local session tracker (which is another instance of SessionTrackerImpl) to track its local session in memory. 

- Each participant intercepts a request before it enters the pipeline. In order to do 2 things
 o Update local session to global session when it saw create ephemeral node request. Update is done by issuing a create session rquest before issuing the create request
 o Add local session flag to a request. So that the pipeline know that this type of request don’t need to send to the leader and wait for commit

- PrepRequestProcessor (on the leader) now explicitly validate global session on create ephemeral node request.  For other type of request, there is no need for session validation because the leader doesn’t know about local sessions on other machine, so the request has to go through.  The correctness is preserve as long a no ephemeral node is created after session is already expired

- Observer/FollowerRequestProcessor has logic to validate session. However, I believe this logic is in correct since the request is already send downstream to the CommitProcessor. The observer and the follower have no other option but to send the request to leader. 


This addresses bug ZOOKEEPER-1147.
    https://issues.apache.org/jira/browse/ZOOKEEPER-1147


Diffs (updated)
-----

  /src/java/main/org/apache/zookeeper/KeeperException.java 1446831 
  /src/java/main/org/apache/zookeeper/cli/CreateCommand.java 1446831 
  /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1446831 
  /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1446831 
  /src/java/main/org/apache/zookeeper/server/Request.java 1446831 
  /src/java/main/org/apache/zookeeper/server/SessionTracker.java 1446831 
  /src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1446831 
  /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1446831 
  /src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 1446831 
  /src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 1446831 
  /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java 1446831 
  /src/java/test/org/apache/zookeeper/test/LeaderSessionTrackerTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/LocalSessionRequestTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/LocalSessionsOnlyTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/QuorumBase.java 1446831 
  /src/java/test/org/apache/zookeeper/test/QuorumTest.java 1446831 
  /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1446831 
  /src/java/test/org/apache/zookeeper/test/ReadOnlyModeTest.java 1446831 
  /src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/SessionUpgradeTest.java PRE-CREATION 

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


Testing
-------

unit tests.  For our internal branch, we haven't run into bugs related to local session for quite a while so it is quite stable. We enabled local session by default in all our deployment.


Thanks,

Thawan Kooburat