ZOOKEEPER-4362: ZKDatabase.txnCount should not log non transactional requests

ZKDatabase.txnCount logged non transactional requests

    public boolean append(Request si) throws IOException {
        txnCount.incrementAndGet();
        return this.snapLog.append(si);
    }
snaplog.append may return false, but txnCount increased

maybe it would be better

    public boolean append(Request si) throws IOException {
        if (this.snapLog.append(si)) {
            txnCount.incrementAndGet();
            return true;
        }
        return false;
    }

Author: lanicc <lanettiesso@gmail.com>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, maoling <maoling@apache.org>

Closes #1744 from lanicc/ZOOKEEPER-4362 and squashes the following commits:

4b08e05ff [lanicc] ZOOKEEPER-4366: fix typo
4e1ad8d2c [lanicc] ZOOKEEPER-4366: Add test case for ZkDatabase txnCount
40b86bcad [lanicc] ZOOKEEPER-4362: ZKDatabase.txnCount should not log non transactional requests

(cherry picked from commit eb63dd9757e7234515e81799d82cc0af4a0ae7f9)
Signed-off-by: maoling <maoling@apache.org>

2 files changed, 79 insertions, 2 deletions

diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZKDatabase.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZKDatabase.java
index f758f5de5..893eb0af6 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZKDatabase.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZKDatabase.java
@@ -637,8 +637,11 @@ public class ZKDatabase {
      * @return true if the append was succesfull and false if not
      */
     public boolean append(Request si) throws IOException {
-        txnCount.incrementAndGet();
-        return this.snapLog.append(si);
+        if (this.snapLog.append(si)) {
+            txnCount.incrementAndGet();
+            return true;
+        }
+        return false;
     }
 
     /**
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/TxnLogCountTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/TxnLogCountTest.java
new file mode 100644
index 000000000..afcb09bdb
--- /dev/null
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/TxnLogCountTest.java
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.server;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import java.io.File;
+import java.io.IOException;
+import org.apache.jute.OutputArchive;
+import org.apache.zookeeper.server.persistence.FileTxnSnapLog;
+import org.apache.zookeeper.test.ClientBase;
+import org.apache.zookeeper.txn.TxnHeader;
+import org.junit.Test;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+
+public class TxnLogCountTest {
+
+    /**
+     * Test ZkDatabase's txnCount
+     */
+    @Test
+    public void testTxnLogCount() throws IOException {
+        File tmpDir = ClientBase.createTmpDir();
+        FileTxnSnapLog snapLog = new FileTxnSnapLog(tmpDir, tmpDir);
+        ZKDatabase zkDatabase = new ZKDatabase(snapLog);
+        int txnRequestCnt = 10;
+        int nonTxnRequestCnt = 10;
+        for (int i = 0; i < txnRequestCnt && zkDatabase.append(mockTxnRequest()); i++) {}
+        assertEquals(txnRequestCnt, zkDatabase.getTxnCount());
+
+        for (int i = 0; i < nonTxnRequestCnt && !zkDatabase.append(mockNonTxnRequest()); i++) {}
+        assertEquals(txnRequestCnt, zkDatabase.getTxnCount());
+    }
+
+    private Request mockTxnRequest() throws IOException {
+        TxnHeader header = mock(TxnHeader.class);
+        doAnswer(new Answer() {
+            @Override
+            public Object answer(InvocationOnMock invocation) throws Throwable {
+                Object[] args = invocation.getArguments();
+                OutputArchive oa = (OutputArchive) args[0];
+                oa.writeString("header", "test");
+                return null;
+            }
+        }).when(header).serialize(any(OutputArchive.class), anyString());
+        Request request = new Request(1, 2, 3, header, null, 4);
+        return request;
+    }
+
+    private Request mockNonTxnRequest() {
+        Request request = new Request(0, 0, 0, null, null, 0);
+        return request;
+    }
+}