[GitHub] [zookeeper] ztzg commented on a change in pull request #1202: ZOOKEEPER-3675: Clean Up committedLog Interaction in ZKDatabase

[GitBox <gi...@apache.org>]

ztzg commented on a change in pull request #1202: ZOOKEEPER-3675: Clean Up committedLog Interaction in ZKDatabase
URL: https://github.com/apache/zookeeper/pull/1202#discussion_r364101123
 
 

 ##########
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/ZKDatabase.java
 ##########
 @@ -314,26 +314,24 @@ private void addCommittedProposal(TxnHeader hdr, Record txn, TxnDigest digest) {
      * fast follower synchronization.
      * @param request committed request
      */
-    public void addCommittedProposal(Request request) {
+    public void addCommittedProposal(final Request request) {
         WriteLock wl = logLock.writeLock();
+        wl.lock();
         try {
-            wl.lock();
-            if (committedLog.size() > commitLogCount) {
-                committedLog.remove();
-                minCommittedLog = committedLog.peek().packet.getZxid();
-            }
-            if (committedLog.isEmpty()) {
-                minCommittedLog = request.zxid;
-                maxCommittedLog = request.zxid;
-            }
-
             byte[] data = SerializeUtils.serializeRequest(request);
             QuorumPacket pp = new QuorumPacket(Leader.PROPOSAL, request.zxid, data, null);
             Proposal p = new Proposal();
             p.packet = pp;
             p.request = request;
+
+            if (committedLog.size() == commitLogCount) {
 
 Review comment:
   Interesting.  I was initially puzzled by the change from `>`  to `==`. This also fixes an off-by-one error in the existing code (and, consequently, makes the log one entry shorter), doesn't it?  I'd probably add a bullet point saying so to the description.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services