----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25160/#review119151 -----------------------------------------------------------
./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java (line 164) <https://reviews.apache.org/r/25160/#comment180426> The red spaces here and elsewhere are probably tabs. Please try to remove ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java (line 210) <https://reviews.apache.org/r/25160/#comment180428> Log an error here if topPending.cxid is different than request.cxid ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java (line 215) <https://reviews.apache.org/r/25160/#comment180427> I'd move this to the end - you have this same code also on line 238, so group it all together in one place. ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java (line 233) <https://reviews.apache.org/r/25160/#comment180429> looks like if you remove !sessionQueue.isEmpty() from the if condition you'll be able to remove empty queues here also for the case that there aren't following reads in the session ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java (line 275) <https://reviews.apache.org/r/25160/#comment180430> is this if needed ? ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java (line 80) <https://reviews.apache.org/r/25160/#comment180432> why remove this ? ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java (line 203) <https://reviews.apache.org/r/25160/#comment180436> also check that all the following reads were executed ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java (line 207) <https://reviews.apache.org/r/25160/#comment180437> Please describe in the test plan what happens from this line onward. ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java (line 224) <https://reviews.apache.org/r/25160/#comment180438> check that the request in the queue is the write request ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java (line 270) <https://reviews.apache.org/r/25160/#comment180441> where is ready used ? ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java (line 281) <https://reviews.apache.org/r/25160/#comment180440> shouldn't this be 5 iterations ? - Alexander Shraer On Feb. 12, 2016, 7:22 p.m., Kfir Lev-Ari wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25160/ > ----------------------------------------------------------- > > (Updated Feb. 12, 2016, 7:22 p.m.) > > > Review request for zookeeper, Raul Gutierrez Segales and Alexander Shraer. > > > Repository: zookeeper > > > Description > ------- > > Please see https://issues.apache.org/jira/browse/ZOOKEEPER-2024 > > > Diffs > ----- > > ./src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java > 1726708 > > ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java > 1726708 > ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java > 1726708 > > Diff: https://reviews.apache.org/r/25160/diff/ > > > Testing > ------- > > The attached unit tests, as well as the system test found in > https://issues.apache.org/jira/browse/ZOOKEEPER-2023. > > > Thanks, > > Kfir Lev-Ari > >
