[bookkeeper] branch branch-4.16 updated: Fix write entry failed in without writing journal case (#3884)

[ch...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

chenhang pushed a commit to branch branch-4.16
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git


The following commit(s) were added to refs/heads/branch-4.16 by this push:
     new 332647feaf Fix write entry failed in without writing journal case (#3884)
332647feaf is described below

commit 332647feaff26a1865285e56e437deb7045eed7b
Author: Hang Chen <ch...@apache.org>
AuthorDate: Sun Mar 26 11:11:20 2023 +0800

    Fix write entry failed in without writing journal case (#3884)
    
    ### Motivation
    https://github.com/apache/bookkeeper/pull/3837 introduced group flush add responses triggered by journal sync. However, if we skip writing journals, the add responses won't be flushed to the netty channel and the client will receive write entries timeout.
    
    ### Changes
    - Flush the add responses when skipping writing journals
    - Add tests to cover V2 protocol and skip writing journal cases.
    
    (cherry picked from commit 5deadcc2411d225e5bdf5dfbf25469fef6adb3b0)
---
 .../java/org/apache/bookkeeper/bookie/BookieImpl.java |  4 ++++
 .../bookkeeper/client/BookieWriteLedgerTest.java      | 19 +++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieImpl.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieImpl.java
index 0db230d9d3..0628ec28af 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieImpl.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieImpl.java
@@ -69,6 +69,7 @@ import org.apache.bookkeeper.discover.RegistrationManager;
 import org.apache.bookkeeper.net.BookieId;
 import org.apache.bookkeeper.net.BookieSocketAddress;
 import org.apache.bookkeeper.net.DNS;
+import org.apache.bookkeeper.proto.BookieRequestHandler;
 import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.WriteCallback;
 import org.apache.bookkeeper.stats.NullStatsLogger;
 import org.apache.bookkeeper.stats.StatsLogger;
@@ -948,6 +949,9 @@ public class BookieImpl extends BookieCriticalThread implements Bookie {
 
         if (!writeDataToJournal) {
             cb.writeComplete(0, ledgerId, entryId, null, ctx);
+            if (ctx instanceof BookieRequestHandler) {
+                ((BookieRequestHandler) ctx).flushPendingResponse();
+            }
             return;
         }
 
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieWriteLedgerTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieWriteLedgerTest.java
index 8ae0b72019..2a57f9952f 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieWriteLedgerTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieWriteLedgerTest.java
@@ -39,6 +39,7 @@ import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.List;
@@ -68,6 +69,8 @@ import org.apache.commons.lang3.tuple.Pair;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
 import org.powermock.reflect.Whitebox;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -75,12 +78,26 @@ import org.slf4j.LoggerFactory;
 /**
  * Testing ledger write entry cases.
  */
+@RunWith(Parameterized.class)
 public class BookieWriteLedgerTest extends
     BookKeeperClusterTestCase implements AddCallback {
 
     private static final Logger LOG = LoggerFactory
             .getLogger(BookieWriteLedgerTest.class);
 
+    @Parameterized.Parameters
+    public static Collection<Object[]> data() {
+        return Arrays.asList(new Object[][] {
+            { true, true }, { true, false }, { false, true }, { false, false }
+        });
+    }
+
+    @Parameterized.Parameter(0)
+    public boolean useV2;
+
+    @Parameterized.Parameter(1)
+    public boolean writeJournal;
+
     byte[] ledgerPassword = "aaa".getBytes();
     LedgerHandle lh, lh2;
     Enumeration<LedgerEntry> ls;
@@ -119,12 +136,14 @@ public class BookieWriteLedgerTest extends
         String ledgerManagerFactory = "org.apache.bookkeeper.meta.HierarchicalLedgerManagerFactory";
         // set ledger manager
         baseConf.setLedgerManagerFactoryClassName(ledgerManagerFactory);
+        baseConf.setJournalWriteData(writeJournal);
         /*
          * 'testLedgerCreateAdvWithLedgerIdInLoop2' testcase relies on skipListSizeLimit,
          * so setting it to some small value for making that testcase lite.
          */
         baseConf.setSkipListSizeLimit(4 * 1024 * 1024);
         baseClientConf.setLedgerManagerFactoryClassName(ledgerManagerFactory);
+        baseClientConf.setUseV2WireProtocol(useV2);
     }
 
     /**