[bookkeeper] branch master 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 master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git


The following commit(s) were added to refs/heads/master by this push:
     new 5deadcc241 Fix write entry failed in without writing journal case (#3884)
5deadcc241 is described below

commit 5deadcc2411d225e5bdf5dfbf25469fef6adb3b0
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.
---
 .../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);
     }
 
     /**