From c0eb78e8d869826fcb9ec9009d543cfc63b56d8e Mon Sep 17 00:00:00 2001
From: Wei-Chiu Chuang <weichiu@apache.org>
Date: Tue, 3 Sep 2024 09:51:09 -0700
Subject: [PATCH 01/10] CDPD-73815. Quick fix for HDDS-9130.

Change-Id: I9c438e8a30e9b1e8b910d43a8303fcfd206debc0
(cherry picked from commit 9349185338cb94b201b14c70f8a5118cb1072c34)
---
 .../hadoop/ozone/container/keyvalue/KeyValueHandler.java     | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
index 402e1be4cd0f..eabaef0c1f46 100644
--- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
+++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
@@ -903,6 +903,11 @@ ContainerCommandResponseProto handleWriteChunk(
         // of order.
         blockData.setBlockCommitSequenceId(dispatcherContext.getLogIndex());
         boolean eob = writeChunk.getBlock().getEof();
+        if (eob) {
+          Preconditions.checkArgument(writeChunk.getBlock().getBlockData().getChunksList().isEmpty(),
+              "Client is not supposed to send empty EC blocks in piggybacking mode.");
+          chunkManager.finishWriteChunks(kvContainer, blockData);
+        }
         blockManager.putBlock(kvContainer, blockData, eob);
         blockDataProto = blockData.getProtoBufMessage();
         final long numBytes = blockDataProto.getSerializedSize();

From c575243484120f6382c7022b9301ff313903fec8 Mon Sep 17 00:00:00 2001
From: Wei-Chiu Chuang <weichiu@apache.org>
Date: Tue, 3 Sep 2024 11:18:50 -0700
Subject: [PATCH 02/10] Add one unit test.

Change-Id: I25a01523cc13cc0035a6bf98d962d89adf17dfd4
---
 .../keyvalue/impl/ChunkManagerDispatcher.java |  2 +-
 .../impl/AbstractTestChunkManager.java        | 42 +++++++++++++++++++
 .../impl/CommonChunkManagerTestCases.java     | 29 +++++++++++++
 3 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/ChunkManagerDispatcher.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/ChunkManagerDispatcher.java
index 6a1d5533cf2c..46be387f2cc8 100644
--- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/ChunkManagerDispatcher.java
+++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/ChunkManagerDispatcher.java
@@ -52,7 +52,7 @@
  */
 public class ChunkManagerDispatcher implements ChunkManager {
 
-  private static final Logger LOG =
+  static final Logger LOG =
       LoggerFactory.getLogger(ChunkManagerDispatcher.class);
 
   private final Map<ContainerLayoutVersion, ChunkManager> handlers
diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/AbstractTestChunkManager.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/AbstractTestChunkManager.java
index 0c373cb0dbf0..5b8b09665f21 100644
--- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/AbstractTestChunkManager.java
+++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/AbstractTestChunkManager.java
@@ -35,13 +35,18 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.io.TempDir;
 
+import java.io.BufferedReader;
 import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
 import java.nio.Buffer;
 import java.nio.ByteBuffer;
 import java.util.UUID;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.anyList;
@@ -128,6 +133,43 @@ protected void checkChunkFileCount(int expected) {
     assertEquals(expected, files.length);
   }
 
+  public static class FuserCheck {
+    public static boolean isFileNotInUse(String filePath) {
+      try {
+        Process process = new ProcessBuilder("fuser", filePath).start();
+        BufferedReader reader =
+            new BufferedReader(new InputStreamReader(process.getInputStream()));
+        return reader.readLine() ==
+            null;  // If fuser returns no output, the file is not in use
+      } catch (Exception e) {
+        e.printStackTrace();
+        return false;  // On failure, assume the file is in use
+      }
+    }
+  }
+
+  // check that all files under chunk path are closed.
+  protected boolean checkChunkFilesClosed() throws IOException {
+    //As in Setup, we try to create container, these paths should exist.
+    String path = keyValueContainerData.getChunksPath();
+    assertNotNull(path);
+
+    File dir = new File(path);
+    assertTrue(dir.exists());
+
+    File[] files = dir.listFiles();
+    assertNotNull(files);
+    for (File file : files) {
+      assertTrue(file.exists());
+      assertTrue(file.isFile());
+      // check that the file is closed.
+      if (!FuserCheck.isFileNotInUse(file.getAbsolutePath())) {
+        return false;
+      }
+    }
+    return true;
+  }
+
   protected void checkWriteIOStats(long length, long opCount) {
     VolumeIOStats volumeIOStats = hddsVolume.getVolumeIOStats();
     assertEquals(length, volumeIOStats.getWriteBytes());
diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/CommonChunkManagerTestCases.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/CommonChunkManagerTestCases.java
index 47d24874749e..cd8f27e577cc 100644
--- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/CommonChunkManagerTestCases.java
+++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/CommonChunkManagerTestCases.java
@@ -27,6 +27,7 @@
 import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainer;
 import org.apache.hadoop.ozone.container.keyvalue.interfaces.ChunkManager;
 import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
 
 import java.io.File;
 import java.io.IOException;
@@ -38,8 +39,11 @@
 import static org.apache.hadoop.ozone.container.common.ContainerTestUtils.WRITE_STAGE;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
+import static org.mockito.Mockito.when;
 
 /**
  * Common test cases for ChunkManager implementation tests.
@@ -222,4 +226,29 @@ public void testWriteAndReadChunkMultipleTimes() throws Exception {
     checkReadIOStats(len * count, count);
   }
 
+  @Test
+  public void testFinishWrite() throws Exception {
+    // GIVEN
+    ChunkManager chunkManager = createTestSubject();
+    checkChunkFileCount(0);
+    checkWriteIOStats(0, 0);
+
+    chunkManager.writeChunk(getKeyValueContainer(), getBlockID(),
+        getChunkInfo(), getData(),
+        WRITE_STAGE);
+
+    BlockData blockData = Mockito.mock(BlockData.class);
+    when(blockData.getBlockID()).thenReturn(getBlockID());
+
+    assertFalse(checkChunkFilesClosed());
+
+    chunkManager.finishWriteChunks(getKeyValueContainer(), blockData);
+
+    assertTrue(checkChunkFilesClosed());
+
+    // THEN
+    checkChunkFileCount(1);
+    checkWriteIOStats(getChunkInfo().getLen(), 1);
+  }
+
 }

From 3ea9475e41a0fec5be33a65c225df308f7d95489 Mon Sep 17 00:00:00 2001
From: Wei-Chiu Chuang <weichiu@apache.org>
Date: Tue, 3 Sep 2024 14:52:25 -0700
Subject: [PATCH 03/10] Add integration regression test.

Change-Id: I515cc2ce7cef83d7c69d739e814c84d2a8ebacc2
---
 .../impl/AbstractTestChunkManager.java        |  7 ++++--
 .../org/apache/hadoop/fs/ozone/TestHSync.java | 22 +++++++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/AbstractTestChunkManager.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/AbstractTestChunkManager.java
index 5b8b09665f21..8726f1904291 100644
--- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/AbstractTestChunkManager.java
+++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/AbstractTestChunkManager.java
@@ -148,10 +148,13 @@ public static boolean isFileNotInUse(String filePath) {
     }
   }
 
-  // check that all files under chunk path are closed.
   protected boolean checkChunkFilesClosed() throws IOException {
+    return checkChunkFilesClosed(keyValueContainerData.getChunksPath());
+  }
+
+  // check that all files under chunk path are closed.
+  public static boolean checkChunkFilesClosed(String path) {
     //As in Setup, we try to create container, these paths should exist.
-    String path = keyValueContainerData.getChunksPath();
     assertNotNull(path);
 
     File dir = new File(path);
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java
index 49b515d53c57..e2f45599d239 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java
@@ -19,6 +19,7 @@
 package org.apache.hadoop.fs.ozone;
 
 import java.io.Closeable;
+import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.nio.ByteBuffer;
@@ -70,6 +71,7 @@
 import org.apache.hadoop.fs.StreamCapabilities;
 
 import org.apache.hadoop.ozone.ClientConfigForTesting;
+import org.apache.hadoop.ozone.HddsDatanodeService;
 import org.apache.hadoop.ozone.MiniOzoneCluster;
 import org.apache.hadoop.ozone.OzoneConfigKeys;
 import org.apache.hadoop.ozone.OzoneConsts;
@@ -83,7 +85,9 @@
 import org.apache.hadoop.ozone.client.io.KeyOutputStream;
 import org.apache.hadoop.ozone.client.io.OzoneInputStream;
 import org.apache.hadoop.ozone.client.io.OzoneOutputStream;
+import org.apache.hadoop.ozone.container.TestHelper;
 import org.apache.hadoop.ozone.container.keyvalue.KeyValueHandler;
+import org.apache.hadoop.ozone.container.keyvalue.impl.AbstractTestChunkManager;
 import org.apache.hadoop.ozone.container.keyvalue.impl.BlockManagerImpl;
 import org.apache.hadoop.ozone.container.metadata.AbstractDatanodeStore;
 import org.apache.hadoop.ozone.om.OMMetadataManager;
@@ -93,6 +97,7 @@
 import org.apache.hadoop.ozone.om.helpers.BucketLayout;
 
 import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfo;
 import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo;
 import org.apache.hadoop.ozone.om.service.OpenKeyCleanupService;
 import org.apache.hadoop.security.UserGroupInformation;
@@ -358,12 +363,29 @@ public void testKeyHSyncThenClose() throws Exception {
     String data = "random data";
     final Path file = new Path(dir, "file-hsync-then-close");
     try (FileSystem fs = FileSystem.get(CONF)) {
+      String chunkPath;
       try (FSDataOutputStream outputStream = fs.create(file, true)) {
         outputStream.write(data.getBytes(UTF_8), 0, data.length());
         outputStream.hsync();
+        // locate the block file on the DataNode.
+        // first, find the block ID of the first block in the output stream.
+        KeyOutputStream groupOutputStream =
+            ((OzoneFSOutputStream)outputStream.getWrappedStream()).getWrappedOutputStream().getKeyOutputStream();
+        List<OmKeyLocationInfo> locationInfoList =
+            groupOutputStream.getLocationInfoList();
+        OmKeyLocationInfo omKeyLocationInfo = locationInfoList.get(0);
+        HddsDatanodeService dn = TestHelper.getDatanodeService(omKeyLocationInfo,
+            cluster);
+        chunkPath = dn.getDatanodeStateMachine()
+            .getContainer().getContainerSet()
+            .getContainer(omKeyLocationInfo.getContainerID()).
+            getContainerData().getChunksPath();
+        assertFalse(AbstractTestChunkManager.checkChunkFilesClosed(chunkPath));
       }
+      assertTrue(AbstractTestChunkManager.FuserCheck.isFileNotInUse(chunkPath));
     }
 
+
     OzoneManager ozoneManager = cluster.getOzoneManager();
     // Wait for double buffer to trigger all pending addToDBBatch(),
     // including OMKeyCommitResponse(WithFSO)'s that writes to deletedTable.

From 41c4ce3c6266fd4b984bed44f6312a33829eaa47 Mon Sep 17 00:00:00 2001
From: Wei-Chiu Chuang <weichiu@apache.org>
Date: Tue, 3 Sep 2024 15:03:34 -0700
Subject: [PATCH 04/10] Fix test & refactor

Change-Id: Idd8766189cd84a2f020750d5d1d85c8ffb4a1915
---
 .../impl/AbstractTestChunkManager.java        | 21 ++++++-----
 .../impl/CommonChunkManagerTestCases.java     |  2 --
 .../org/apache/hadoop/fs/ozone/TestHSync.java | 35 +++++++++++--------
 3 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/AbstractTestChunkManager.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/AbstractTestChunkManager.java
index 8726f1904291..8195f12741b8 100644
--- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/AbstractTestChunkManager.java
+++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/AbstractTestChunkManager.java
@@ -34,10 +34,11 @@
 import org.apache.hadoop.ozone.container.keyvalue.interfaces.ChunkManager;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.io.TempDir;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.io.BufferedReader;
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.nio.Buffer;
@@ -46,7 +47,6 @@
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.anyList;
@@ -58,6 +58,8 @@
  * Helpers for ChunkManager implementation tests.
  */
 public abstract class AbstractTestChunkManager {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(AbstractTestChunkManager.class);
 
   private HddsVolume hddsVolume;
   private KeyValueContainerData keyValueContainerData;
@@ -133,22 +135,23 @@ protected void checkChunkFileCount(int expected) {
     assertEquals(expected, files.length);
   }
 
+  /**
+   * Helper class to check if a file is in use.
+   */
   public static class FuserCheck {
     public static boolean isFileNotInUse(String filePath) {
       try {
         Process process = new ProcessBuilder("fuser", filePath).start();
-        BufferedReader reader =
-            new BufferedReader(new InputStreamReader(process.getInputStream()));
-        return reader.readLine() ==
-            null;  // If fuser returns no output, the file is not in use
-      } catch (Exception e) {
-        e.printStackTrace();
+        BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()));
+        return reader.readLine() == null;  // If fuser returns no output, the file is not in use
+      } catch (IOException e) {
+        LOG.warn("Failed to check if file is in use: {}", filePath, e);
         return false;  // On failure, assume the file is in use
       }
     }
   }
 
-  protected boolean checkChunkFilesClosed() throws IOException {
+  protected boolean checkChunkFilesClosed() {
     return checkChunkFilesClosed(keyValueContainerData.getChunksPath());
   }
 
diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/CommonChunkManagerTestCases.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/CommonChunkManagerTestCases.java
index cd8f27e577cc..5b74f80ee863 100644
--- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/CommonChunkManagerTestCases.java
+++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/CommonChunkManagerTestCases.java
@@ -239,11 +239,9 @@ public void testFinishWrite() throws Exception {
 
     BlockData blockData = Mockito.mock(BlockData.class);
     when(blockData.getBlockID()).thenReturn(getBlockID());
-
     assertFalse(checkChunkFilesClosed());
 
     chunkManager.finishWriteChunks(getKeyValueContainer(), blockData);
-
     assertTrue(checkChunkFilesClosed());
 
     // THEN
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java
index e2f45599d239..ca244c790291 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java
@@ -19,7 +19,6 @@
 package org.apache.hadoop.fs.ozone;
 
 import java.io.Closeable;
-import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.nio.ByteBuffer;
@@ -367,22 +366,12 @@ public void testKeyHSyncThenClose() throws Exception {
       try (FSDataOutputStream outputStream = fs.create(file, true)) {
         outputStream.write(data.getBytes(UTF_8), 0, data.length());
         outputStream.hsync();
-        // locate the block file on the DataNode.
-        // first, find the block ID of the first block in the output stream.
-        KeyOutputStream groupOutputStream =
-            ((OzoneFSOutputStream)outputStream.getWrappedStream()).getWrappedOutputStream().getKeyOutputStream();
-        List<OmKeyLocationInfo> locationInfoList =
-            groupOutputStream.getLocationInfoList();
-        OmKeyLocationInfo omKeyLocationInfo = locationInfoList.get(0);
-        HddsDatanodeService dn = TestHelper.getDatanodeService(omKeyLocationInfo,
-            cluster);
-        chunkPath = dn.getDatanodeStateMachine()
-            .getContainer().getContainerSet()
-            .getContainer(omKeyLocationInfo.getContainerID()).
-            getContainerData().getChunksPath();
+        // locate the container chunk path on the first DataNode.
+        chunkPath = getChunkPathOnDataNode(outputStream);
         assertFalse(AbstractTestChunkManager.checkChunkFilesClosed(chunkPath));
       }
-      assertTrue(AbstractTestChunkManager.FuserCheck.isFileNotInUse(chunkPath));
+      // After close, the chunk file should be closed.
+      assertTrue(AbstractTestChunkManager.checkChunkFilesClosed(chunkPath));
     }
 
 
@@ -409,6 +398,22 @@ public void testKeyHSyncThenClose() throws Exception {
     }
   }
 
+  private static String getChunkPathOnDataNode(FSDataOutputStream outputStream)
+      throws IOException {
+    String chunkPath;
+    KeyOutputStream groupOutputStream =
+        ((OzoneFSOutputStream) outputStream.getWrappedStream()).getWrappedOutputStream().getKeyOutputStream();
+    List<OmKeyLocationInfo> locationInfoList =
+        groupOutputStream.getLocationInfoList();
+    OmKeyLocationInfo omKeyLocationInfo = locationInfoList.get(0);
+    HddsDatanodeService dn = TestHelper.getDatanodeService(omKeyLocationInfo, cluster);
+    chunkPath = dn.getDatanodeStateMachine()
+        .getContainer().getContainerSet()
+        .getContainer(omKeyLocationInfo.getContainerID()).
+        getContainerData().getChunksPath();
+    return chunkPath;
+  }
+
   @ParameterizedTest
   @ValueSource(booleans = {false, true})
   public void testO3fsHSync(boolean incrementalChunkList) throws Exception {

From e2e48b6613bb19c6608c67970bc34e81045be917 Mon Sep 17 00:00:00 2001
From: Wei-Chiu Chuang <weichiu@apache.org>
Date: Tue, 3 Sep 2024 19:54:16 -0700
Subject: [PATCH 05/10] Remove buggy code.

Change-Id: I4490b614d0334db0aae2c8fe8accd2b792d3d198
---
 .../hadoop/ozone/container/keyvalue/KeyValueHandler.java       | 2 --
 .../ozone/container/keyvalue/impl/ChunkManagerDispatcher.java  | 2 +-
 .../src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java    | 3 +++
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
index eabaef0c1f46..a99a86d98bc1 100644
--- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
+++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
@@ -904,8 +904,6 @@ ContainerCommandResponseProto handleWriteChunk(
         blockData.setBlockCommitSequenceId(dispatcherContext.getLogIndex());
         boolean eob = writeChunk.getBlock().getEof();
         if (eob) {
-          Preconditions.checkArgument(writeChunk.getBlock().getBlockData().getChunksList().isEmpty(),
-              "Client is not supposed to send empty EC blocks in piggybacking mode.");
           chunkManager.finishWriteChunks(kvContainer, blockData);
         }
         blockManager.putBlock(kvContainer, blockData, eob);
diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/ChunkManagerDispatcher.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/ChunkManagerDispatcher.java
index 46be387f2cc8..6a1d5533cf2c 100644
--- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/ChunkManagerDispatcher.java
+++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/ChunkManagerDispatcher.java
@@ -52,7 +52,7 @@
  */
 public class ChunkManagerDispatcher implements ChunkManager {
 
-  static final Logger LOG =
+  private static final Logger LOG =
       LoggerFactory.getLogger(ChunkManagerDispatcher.class);
 
   private final Map<ContainerLayoutVersion, ChunkManager> handlers
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java
index ca244c790291..9942d086e403 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java
@@ -369,6 +369,9 @@ public void testKeyHSyncThenClose() throws Exception {
         // locate the container chunk path on the first DataNode.
         chunkPath = getChunkPathOnDataNode(outputStream);
         assertFalse(AbstractTestChunkManager.checkChunkFilesClosed(chunkPath));
+
+        // TODO: the next assertion will fail if the following line is commented out.
+        outputStream.write(data.getBytes(UTF_8), 0, data.length());
       }
       // After close, the chunk file should be closed.
       assertTrue(AbstractTestChunkManager.checkChunkFilesClosed(chunkPath));

From 6b5d50677c57b1c7d948d4317ae8429d0a1bea68 Mon Sep 17 00:00:00 2001
From: Wei-Chiu Chuang <weichiu@apache.org>
Date: Wed, 4 Sep 2024 00:55:18 -0700
Subject: [PATCH 06/10] Fix the corner case where Ratis output stream in
 incremental chunk list mode closes block and that the write buffer is empty.

Change-Id: I198fde6cb4e1cd2ee8ec792df7907345df9b1054
---
 .../ozone/container/keyvalue/KeyValueHandler.java     | 11 ++++++++---
 .../java/org/apache/hadoop/fs/ozone/TestHSync.java    |  4 ----
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
index a99a86d98bc1..1bcb64200b22 100644
--- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
+++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
@@ -122,6 +122,7 @@
 import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos
     .ContainerDataProto.State.RECOVERING;
 import static org.apache.hadoop.ozone.ClientVersion.EC_REPLICA_INDEX_REQUIRED_IN_BLOCK_REQUEST;
+import static org.apache.hadoop.ozone.OzoneConsts.INCREMENTAL_CHUNK_LIST;
 import static org.apache.hadoop.ozone.container.common.interfaces.Container.ScanResult;
 
 import org.apache.hadoop.util.Time;
@@ -547,9 +548,13 @@ ContainerCommandResponseProto handlePutBlock(
 
       boolean endOfBlock = false;
       if (!request.getPutBlock().hasEof() || request.getPutBlock().getEof()) {
-        // in EC, we will be doing empty put block.
-        // So, let's flush only when there are any chunks
-        if (!request.getPutBlock().getBlockData().getChunksList().isEmpty()) {
+        // There are two cases where client sends empty put block with eof.
+        // (1) An EC empty file. In this case, the block/chunk file does not exist,
+        //     so no need to flush/close the file.
+        // (2) Ratis output stream in incremental chunk list mode may send empty put block
+        //     to close the block, in which case we need to flush/close the file.
+        if (!request.getPutBlock().getBlockData().getChunksList().isEmpty() ||
+            blockData.getMetadata().containsKey(INCREMENTAL_CHUNK_LIST)) {
           chunkManager.finishWriteChunks(kvContainer, blockData);
         }
         endOfBlock = true;
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java
index 9942d086e403..faa7d8629d13 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java
@@ -369,15 +369,11 @@ public void testKeyHSyncThenClose() throws Exception {
         // locate the container chunk path on the first DataNode.
         chunkPath = getChunkPathOnDataNode(outputStream);
         assertFalse(AbstractTestChunkManager.checkChunkFilesClosed(chunkPath));
-
-        // TODO: the next assertion will fail if the following line is commented out.
-        outputStream.write(data.getBytes(UTF_8), 0, data.length());
       }
       // After close, the chunk file should be closed.
       assertTrue(AbstractTestChunkManager.checkChunkFilesClosed(chunkPath));
     }
 
-
     OzoneManager ozoneManager = cluster.getOzoneManager();
     // Wait for double buffer to trigger all pending addToDBBatch(),
     // including OMKeyCommitResponse(WithFSO)'s that writes to deletedTable.

From ba4d26d19c8ee4e87589929e8e9c22b0453b8ee5 Mon Sep 17 00:00:00 2001
From: Wei-Chiu Chuang <weichiu@apache.org>
Date: Wed, 4 Sep 2024 01:43:41 -0700
Subject: [PATCH 07/10] Checkstyle and findbugs

Change-Id: Ic042e0fa7b5038eaa7b37f574cc6c3c55effddb3
---
 .../impl/AbstractTestChunkManager.java        | 25 +++++++++++--------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/AbstractTestChunkManager.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/AbstractTestChunkManager.java
index 8195f12741b8..4e70874db7bc 100644
--- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/AbstractTestChunkManager.java
+++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/AbstractTestChunkManager.java
@@ -136,18 +136,19 @@ protected void checkChunkFileCount(int expected) {
   }
 
   /**
-   * Helper class to check if a file is in use.
+   * Helper method to check if a file is in use.
    */
-  public static class FuserCheck {
-    public static boolean isFileNotInUse(String filePath) {
-      try {
-        Process process = new ProcessBuilder("fuser", filePath).start();
-        BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()));
+  public static boolean isFileNotInUse(String filePath) {
+    try {
+      Process process = new ProcessBuilder("fuser", filePath).start();
+      try (BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream(), UTF_8))) {
         return reader.readLine() == null;  // If fuser returns no output, the file is not in use
-      } catch (IOException e) {
-        LOG.warn("Failed to check if file is in use: {}", filePath, e);
-        return false;  // On failure, assume the file is in use
+      } finally {
+        process.destroy();
       }
+    } catch (IOException e) {
+      LOG.warn("Failed to check if file is in use: {}", filePath, e);
+      return false;  // On failure, assume the file is in use
     }
   }
 
@@ -155,7 +156,9 @@ protected boolean checkChunkFilesClosed() {
     return checkChunkFilesClosed(keyValueContainerData.getChunksPath());
   }
 
-  // check that all files under chunk path are closed.
+  /**
+   * check that all files under chunk path are closed.
+  */
   public static boolean checkChunkFilesClosed(String path) {
     //As in Setup, we try to create container, these paths should exist.
     assertNotNull(path);
@@ -169,7 +172,7 @@ public static boolean checkChunkFilesClosed(String path) {
       assertTrue(file.exists());
       assertTrue(file.isFile());
       // check that the file is closed.
-      if (!FuserCheck.isFileNotInUse(file.getAbsolutePath())) {
+      if (!isFileNotInUse(file.getAbsolutePath())) {
         return false;
       }
     }

From 4c50bbf97217cdaa6e8f4c659998213ad55ecd54 Mon Sep 17 00:00:00 2001
From: Wei-Chiu Chuang <weichiu@apache.org>
Date: Wed, 4 Sep 2024 08:31:23 -0700
Subject: [PATCH 08/10] Fix findbugs and TestFilePerChunkStrategy.

Change-Id: I213cbf98ca978bfba04ddef1fc1aacfae6686716
---
 .../container/keyvalue/impl/AbstractTestChunkManager.java  | 7 ++++++-
 .../keyvalue/impl/CommonChunkManagerTestCases.java         | 1 -
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/AbstractTestChunkManager.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/AbstractTestChunkManager.java
index 4e70874db7bc..d9b95f199ddd 100644
--- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/AbstractTestChunkManager.java
+++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/AbstractTestChunkManager.java
@@ -142,7 +142,12 @@ public static boolean isFileNotInUse(String filePath) {
     try {
       Process process = new ProcessBuilder("fuser", filePath).start();
       try (BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream(), UTF_8))) {
-        return reader.readLine() == null;  // If fuser returns no output, the file is not in use
+        String output = reader.readLine();  // If fuser returns no output, the file is not in use
+        if (output == null) {
+          return true;
+        }
+        LOG.debug("File is in use: {}", filePath);
+        return false;
       } finally {
         process.destroy();
       }
diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/CommonChunkManagerTestCases.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/CommonChunkManagerTestCases.java
index 5b74f80ee863..4f29cb565ba1 100644
--- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/CommonChunkManagerTestCases.java
+++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/CommonChunkManagerTestCases.java
@@ -239,7 +239,6 @@ public void testFinishWrite() throws Exception {
 
     BlockData blockData = Mockito.mock(BlockData.class);
     when(blockData.getBlockID()).thenReturn(getBlockID());
-    assertFalse(checkChunkFilesClosed());
 
     chunkManager.finishWriteChunks(getKeyValueContainer(), blockData);
     assertTrue(checkChunkFilesClosed());

From 5b7007dbef2e06837209661d7e65e80adc3d0abe Mon Sep 17 00:00:00 2001
From: Wei-Chiu Chuang <weichiu@apache.org>
Date: Wed, 4 Sep 2024 09:05:09 -0700
Subject: [PATCH 09/10] Checkstyle

Change-Id: Idf579d0ce8c4c2eb974fb7b058bc00d8b605c770
---
 .../container/keyvalue/impl/CommonChunkManagerTestCases.java     | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/CommonChunkManagerTestCases.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/CommonChunkManagerTestCases.java
index 4f29cb565ba1..d4a12f577e98 100644
--- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/CommonChunkManagerTestCases.java
+++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/CommonChunkManagerTestCases.java
@@ -39,7 +39,6 @@
 import static org.apache.hadoop.ozone.container.common.ContainerTestUtils.WRITE_STAGE;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;

From 9ce2aaac394ccb3351a717df37aaa73cf1a828f4 Mon Sep 17 00:00:00 2001
From: Wei-Chiu Chuang <weichiu@apache.org>
Date: Wed, 4 Sep 2024 15:26:21 -0700
Subject: [PATCH 10/10] Fix test.

Change-Id: Id9839749b492735f1ce91b1e1fb0b74d786783a8
---
 .../src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java     | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java
index faa7d8629d13..72978f818109 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java
@@ -324,6 +324,8 @@ private void waitForEmptyDeletedTable()
   }
 
   @Test
+  // Making this the second test to be run to avoid lingering block files from previous tests
+  @Order(2)
   public void testEmptyHsync() throws Exception {
     // Check that deletedTable should not have keys with the same block as in
     // keyTable's when a key is hsync()'ed then close()'d.