> On Jan. 27, 2013, 11:58 p.m., Edward Ribeiro wrote: > > /src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java, line 93 > > <https://reviews.apache.org/r/8935/diff/1/?file=247658#file247658line93> > > > > It'd be *very* cool if you could change this method signature to pass a > > j.u.c.ConcurrentMap interface instead of a concrete class > > (ConcurrentHashMap). I suppose it would require some more changes to the > > code, but giving preference to pass interfaces makes the code easier to > > change in the future, and it's best practice.
I agree, will do that - Thawan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8935/#review15739 ----------------------------------------------------------- On Jan. 14, 2013, 11:48 p.m., Thawan Kooburat wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8935/ > ----------------------------------------------------------- > > (Updated Jan. 14, 2013, 11:48 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 1427034 > /src/java/main/org/apache/zookeeper/cli/CreateCommand.java 1427034 > /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java > 1427034 > /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java > 1427034 > /src/java/main/org/apache/zookeeper/server/Request.java 1427034 > /src/java/main/org/apache/zookeeper/server/SessionTracker.java 1427034 > /src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 1427034 > /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1427034 > /src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 1427034 > /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java > 1427034 > > /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java > 1427034 > /src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java > PRE-CREATION > > /src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java > 1427034 > /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1427034 > /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java > 1427034 > > /src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java > 1427034 > > /src/java/main/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java > 1427034 > /src/java/main/org/apache/zookeeper/server/quorum/LocalSessionTracker.java > PRE-CREATION > > /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java > 1427034 > > /src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java > 1427034 > /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1427034 > /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java > 1427034 > /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java > 1427034 > > /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java > 1427034 > > /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java > 1427034 > > /src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java > PRE-CREATION > /src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java > 1427034 > /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java > 1427034 > /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 1427034 > /src/java/test/org/apache/zookeeper/test/QuorumTest.java 1427034 > /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1427034 > /src/java/test/org/apache/zookeeper/test/ReadOnlyModeTest.java 1427034 > /src/java/test/org/apache/zookeeper/test/SessionTrackerCheckTest.java > PRE-CREATION > /src/java/test/org/apache/zookeeper/test/SessionUpgradeTest.java > PRE-CREATION > /src/java/test/org/apache/zookeeper/test/WatcherTest.java 1427034 > > 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 > >
