From 03e54156e4a920dcd3868a1db853b4050d325f6a Mon Sep 17 00:00:00 2001 From: Siyao Meng <50227127+smengcl@users.noreply.github.com> Date: Mon, 9 Feb 2026 18:13:56 -0800 Subject: [PATCH 1/3] HDDS-14600. [FSO] Non-recursive dir deletion fails with "Directory is not empty" despite all children deleted when double buffer is not flushed --- .../AbstractOzoneFileSystemTestWithFSO.java | 43 +++++++++++ .../ozone/om/request/file/OMFileRequest.java | 46 ++++++++++-- .../key/TestOMKeyDeleteRequestWithFSO.java | 74 +++++++++++++++++++ 3 files changed, 158 insertions(+), 5 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractOzoneFileSystemTestWithFSO.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractOzoneFileSystemTestWithFSO.java index 3d41012a98ac..c5243381f9fc 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractOzoneFileSystemTestWithFSO.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractOzoneFileSystemTestWithFSO.java @@ -42,6 +42,7 @@ import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmDirectoryInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.ratis.OzoneManagerDoubleBuffer; import org.apache.ozone.test.GenericTestUtils; import org.apache.ozone.test.GenericTestUtils.LogCapturer; import org.junit.jupiter.api.MethodOrderer; @@ -552,4 +553,46 @@ long verifyDirKey(long volumeId, long bucketId, long parentId, return dirInfo.getObjectID(); } + /** + * Test to reproduce "Directory Not Empty" bug using public FileSystem API. + * Tests both checkSubDirectoryExists() and checkSubFileExists() paths. + * Creates child directory and file, deletes them, then tries to delete parent. + * This test exposes a Ratis transaction visibility issue where deleted + * entries are in cache but not yet flushed to DB via double buffer. + */ + @Test + public void testDeleteParentAfterChildDeleted() throws Exception { + Path parent = new Path("/parent"); + Path childDir = new Path(parent, "childDir"); + Path childFile = new Path(parent, "childFile"); + + // Create parent directory + assertTrue(getFs().mkdirs(parent)); + // Create child directory (tests checkSubDirectoryExists path) + assertTrue(getFs().mkdirs(childDir)); + // Create child file (tests checkSubFileExists path) + ContractTestUtils.touch(getFs(), childFile); + + // Stop double buffer to prevent flushing deleted entries to DB + // This makes the bug reproduce more reliably + OzoneManagerDoubleBuffer doubleBuffer = getCluster().getOzoneManager() + .getOmRatisServer().getOmStateMachine().getOzoneManagerDoubleBuffer(); + doubleBuffer.stopDaemon(); + + try { + // Delete child directory + assertTrue(getFs().delete(childDir, false), "Child directory delete should succeed"); + // Delete child file + assertTrue(getFs().delete(childFile, false), "Child file delete should succeed"); + + // Try to delete parent directory (should succeed but may fail with the bug) + // Without the fix, this fails because deleted children are still in DB + boolean parentDeleted = getFs().delete(parent, false); + assertTrue(parentDeleted, "Parent delete should succeed after children deleted"); + } finally { + // Resume double buffer to avoid affecting other tests + doubleBuffer.resume(); + } + } + } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java index 8403828f395a..e30dc3c913b6 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java @@ -30,10 +30,12 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.hdds.client.DefaultReplicationConfig; import org.apache.hadoop.hdds.client.ReplicationConfig; @@ -866,11 +868,15 @@ private static boolean checkSubDirectoryExists(OmKeyInfo omKeyInfo, Iterator, CacheValue>> cacheIter = dirTable.cacheIterator(); + // Track deleted entries in cache (null values = pending Ratis delete) + Set deletedKeysInCache = new HashSet<>(); while (cacheIter.hasNext()) { Map.Entry, CacheValue> entry = cacheIter.next(); OmDirectoryInfo cacheOmDirInfo = entry.getValue().getCacheValue(); if (cacheOmDirInfo == null) { + // Entry marked for deletion in cache (Ratis transaction committed but not yet flushed to DB) + deletedKeysInCache.add(entry.getKey().getCacheKey()); continue; } if (isImmediateChild(cacheOmDirInfo.getParentObjectID(), @@ -889,11 +895,24 @@ private static boolean checkSubDirectoryExists(OmKeyInfo omKeyInfo, Table.KeyValue> iterator = dirTable.iterator(seekDirInDB)) { - if (iterator.hasNext()) { + while (iterator.hasNext()) { Table.KeyValue entry = iterator.next(); + String dbKey = entry.getKey(); OmDirectoryInfo dirInfo = entry.getValue(); - return isImmediateChild(dirInfo.getParentObjectID(), + boolean isChild = isImmediateChild(dirInfo.getParentObjectID(), omKeyInfo.getObjectID()); + + if (!isChild) { + return false; + } + + // If child found in DB, check if it's marked as deleted in cache + if (deletedKeysInCache.contains(dbKey)) { + // Entry is in DB but marked for deletion in cache, ignore it + continue; + } + + return true; } } @@ -911,11 +930,15 @@ private static boolean checkSubFileExists(OmKeyInfo omKeyInfo, Iterator, CacheValue>> cacheIter = fileTable.cacheIterator(); + // Track deleted entries in cache (null values = pending Ratis delete) + Set deletedKeysInCache = new HashSet<>(); while (cacheIter.hasNext()) { Map.Entry, CacheValue> entry = cacheIter.next(); OmKeyInfo cacheOmFileInfo = entry.getValue().getCacheValue(); if (cacheOmFileInfo == null) { + // Entry marked for deletion in cache (Ratis transaction committed but not yet flushed to DB) + deletedKeysInCache.add(entry.getKey().getCacheKey()); continue; } if (isImmediateChild(cacheOmFileInfo.getParentObjectID(), @@ -933,11 +956,24 @@ private static boolean checkSubFileExists(OmKeyInfo omKeyInfo, try (TableIterator> iterator = fileTable.iterator(seekFileInDB)) { - if (iterator.hasNext()) { + while (iterator.hasNext()) { Table.KeyValue entry = iterator.next(); + String dbKey = entry.getKey(); OmKeyInfo fileInfo = entry.getValue(); - return isImmediateChild(fileInfo.getParentObjectID(), - omKeyInfo.getObjectID()); // found a sub path file + boolean isChild = isImmediateChild(fileInfo.getParentObjectID(), + omKeyInfo.getObjectID()); + + if (!isChild) { + return false; + } + + // If child found in DB, check if it's marked as deleted in cache + if (deletedKeysInCache.contains(dbKey)) { + // Entry is in DB but marked for deletion in cache, ignore it + continue; + } + + return true; // found a sub path file } } return false; // no sub paths found diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequestWithFSO.java index eace03a8a1c0..2321504ad557 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequestWithFSO.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequestWithFSO.java @@ -29,6 +29,7 @@ import java.io.IOException; import java.util.Iterator; import java.util.NoSuchElementException; +import java.util.UUID; import org.apache.hadoop.hdds.client.RatisReplicationConfig; import org.apache.hadoop.ozone.om.OzonePrefixPathImpl; import org.apache.hadoop.ozone.om.exceptions.OMException; @@ -39,6 +40,8 @@ import org.apache.hadoop.ozone.om.request.OMRequestTestUtils; import org.apache.hadoop.ozone.om.response.OMClientResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteKeyRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.security.acl.OzonePrefixPath; import org.junit.jupiter.api.Test; @@ -259,4 +262,75 @@ public void testDeleteDirectoryWithColonInFSOBucket() throws Exception { assertEquals(OzoneManagerProtocolProtos.Status.OK, response.getOMResponse().getStatus()); assertNull(omMetadataManager.getDirectoryTable().get(dirName)); } + + private OMRequest createDeleteKeyRequest(String keyPath, boolean recursive) { + KeyArgs keyArgs = KeyArgs.newBuilder() + .setBucketName(bucketName) + .setVolumeName(volumeName) + .setKeyName(keyPath) + .setRecursive(recursive) + .build(); + + DeleteKeyRequest deleteKeyRequest = + DeleteKeyRequest.newBuilder().setKeyArgs(keyArgs).build(); + + return OMRequest.newBuilder() + .setDeleteKeyRequest(deleteKeyRequest) + .setCmdType(OzoneManagerProtocolProtos.Type.DeleteKey) + .setClientId(UUID.randomUUID().toString()) + .build(); + } + + /** + * Minimal test to reproduce "Directory Not Empty" bug with Ratis. + * Tests both checkSubDirectoryExists() and checkSubFileExists() paths. + * Creates child directory and file, deletes them, then tries to delete parent. + * This test exposes a Ratis transaction visibility issue where deleted + * entries are in cache but not yet flushed to DB via double buffer. + */ + @Test + public void testDeleteParentAfterChildDeleted() throws Exception { + OMRequestTestUtils.addVolumeAndBucketToDB(volumeName, bucketName, omMetadataManager, getBucketLayout()); + + String parentDir = "parent"; + long parentId = OMRequestTestUtils.addParentsToDirTable(volumeName, bucketName, parentDir, omMetadataManager); + + // Create a child directory (tests checkSubDirectoryExists path) + OMRequestTestUtils.addParentsToDirTable(volumeName, bucketName, parentDir + "/childDir", omMetadataManager); + + // Create a child file (tests checkSubFileExists path) + String fileName = "childFile"; + OmKeyInfo fileInfo = OMRequestTestUtils.createOmKeyInfo(volumeName, + bucketName, parentDir + "/" + fileName, RatisReplicationConfig.getInstance(ONE)) + .setObjectID(parentId + 2) + .setParentObjectID(parentId) + .setUpdateID(50L) + .build(); + fileInfo.setKeyName(fileName); + OMRequestTestUtils.addFileToKeyTable(false, false, fileName, fileInfo, -1, 50, omMetadataManager); + + // Delete the child directory + long txnId = 1000L; + OMRequest deleteChildDirRequest = doPreExecute(createDeleteKeyRequest(parentDir + "/childDir", true)); + OMKeyDeleteRequest deleteChildDirKeyRequest = getOmKeyDeleteRequest(deleteChildDirRequest); + OMClientResponse deleteChildDirResponse = deleteChildDirKeyRequest.validateAndUpdateCache(ozoneManager, txnId++); + assertEquals(OzoneManagerProtocolProtos.Status.OK, deleteChildDirResponse.getOMResponse().getStatus(), + "Child directory delete should succeed"); + + // Delete the child file + OMRequest deleteChildFileRequest = doPreExecute(createDeleteKeyRequest(parentDir + "/" + fileName, false)); + OMKeyDeleteRequest deleteChildFileKeyRequest = getOmKeyDeleteRequest(deleteChildFileRequest); + OMClientResponse deleteChildFileResponse = deleteChildFileKeyRequest.validateAndUpdateCache(ozoneManager, txnId++); + assertEquals(OzoneManagerProtocolProtos.Status.OK, deleteChildFileResponse.getOMResponse().getStatus(), + "Child file delete should succeed"); + + // Try to delete parent (should succeed but fails without fix) + OMRequest deleteParentRequest = doPreExecute(createDeleteKeyRequest(parentDir, false)); + OMKeyDeleteRequest deleteParentKeyRequest = getOmKeyDeleteRequest(deleteParentRequest); + OMClientResponse response = deleteParentKeyRequest.validateAndUpdateCache(ozoneManager, txnId); + + // This should succeed after the fix + assertEquals(OzoneManagerProtocolProtos.Status.OK, response.getOMResponse().getStatus(), + "Parent delete should succeed after children deleted"); + } } From 184bbc9bb205ec22cc373b7e810057eb9a1994c1 Mon Sep 17 00:00:00 2001 From: Siyao Meng <50227127+smengcl@users.noreply.github.com> Date: Mon, 9 Feb 2026 20:22:50 -0800 Subject: [PATCH 2/3] Address comments --- .../AbstractOzoneFileSystemTestWithFSO.java | 12 +++++------- .../ozone/om/request/file/OMFileRequest.java | 18 ++++++------------ .../key/TestOMKeyDeleteRequestWithFSO.java | 2 +- 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractOzoneFileSystemTestWithFSO.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractOzoneFileSystemTestWithFSO.java index c5243381f9fc..a1bb004746cc 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractOzoneFileSystemTestWithFSO.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractOzoneFileSystemTestWithFSO.java @@ -557,8 +557,6 @@ long verifyDirKey(long volumeId, long bucketId, long parentId, * Test to reproduce "Directory Not Empty" bug using public FileSystem API. * Tests both checkSubDirectoryExists() and checkSubFileExists() paths. * Creates child directory and file, deletes them, then tries to delete parent. - * This test exposes a Ratis transaction visibility issue where deleted - * entries are in cache but not yet flushed to DB via double buffer. */ @Test public void testDeleteParentAfterChildDeleted() throws Exception { @@ -573,11 +571,11 @@ public void testDeleteParentAfterChildDeleted() throws Exception { // Create child file (tests checkSubFileExists path) ContractTestUtils.touch(getFs(), childFile); - // Stop double buffer to prevent flushing deleted entries to DB - // This makes the bug reproduce more reliably + // Pause double buffer to prevent flushing deleted entries to DB + // This makes the bug reproduce deterministically OzoneManagerDoubleBuffer doubleBuffer = getCluster().getOzoneManager() .getOmRatisServer().getOmStateMachine().getOzoneManagerDoubleBuffer(); - doubleBuffer.stopDaemon(); + doubleBuffer.pause(); try { // Delete child directory @@ -590,8 +588,8 @@ public void testDeleteParentAfterChildDeleted() throws Exception { boolean parentDeleted = getFs().delete(parent, false); assertTrue(parentDeleted, "Parent delete should succeed after children deleted"); } finally { - // Resume double buffer to avoid affecting other tests - doubleBuffer.resume(); + // Unpause double buffer to avoid affecting other tests + doubleBuffer.unpause(); } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java index e30dc3c913b6..be8361be501f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java @@ -30,12 +30,10 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; -import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.Set; import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.hdds.client.DefaultReplicationConfig; import org.apache.hadoop.hdds.client.ReplicationConfig; @@ -868,15 +866,12 @@ private static boolean checkSubDirectoryExists(OmKeyInfo omKeyInfo, Iterator, CacheValue>> cacheIter = dirTable.cacheIterator(); - // Track deleted entries in cache (null values = pending Ratis delete) - Set deletedKeysInCache = new HashSet<>(); while (cacheIter.hasNext()) { Map.Entry, CacheValue> entry = cacheIter.next(); OmDirectoryInfo cacheOmDirInfo = entry.getValue().getCacheValue(); if (cacheOmDirInfo == null) { // Entry marked for deletion in cache (Ratis transaction committed but not yet flushed to DB) - deletedKeysInCache.add(entry.getKey().getCacheKey()); continue; } if (isImmediateChild(cacheOmDirInfo.getParentObjectID(), @@ -907,8 +902,9 @@ private static boolean checkSubDirectoryExists(OmKeyInfo omKeyInfo, } // If child found in DB, check if it's marked as deleted in cache - if (deletedKeysInCache.contains(dbKey)) { - // Entry is in DB but marked for deletion in cache, ignore it + CacheValue cacheValue = dirTable.getCacheValue(new CacheKey<>(dbKey)); + if (cacheValue != null && cacheValue.getCacheValue() == null) { + // Entry is in DB but marked for deletion in cache, ignore it and check next entry continue; } @@ -930,15 +926,12 @@ private static boolean checkSubFileExists(OmKeyInfo omKeyInfo, Iterator, CacheValue>> cacheIter = fileTable.cacheIterator(); - // Track deleted entries in cache (null values = pending Ratis delete) - Set deletedKeysInCache = new HashSet<>(); while (cacheIter.hasNext()) { Map.Entry, CacheValue> entry = cacheIter.next(); OmKeyInfo cacheOmFileInfo = entry.getValue().getCacheValue(); if (cacheOmFileInfo == null) { // Entry marked for deletion in cache (Ratis transaction committed but not yet flushed to DB) - deletedKeysInCache.add(entry.getKey().getCacheKey()); continue; } if (isImmediateChild(cacheOmFileInfo.getParentObjectID(), @@ -968,8 +961,9 @@ private static boolean checkSubFileExists(OmKeyInfo omKeyInfo, } // If child found in DB, check if it's marked as deleted in cache - if (deletedKeysInCache.contains(dbKey)) { - // Entry is in DB but marked for deletion in cache, ignore it + CacheValue cacheValue = fileTable.getCacheValue(new CacheKey<>(dbKey)); + if (cacheValue != null && cacheValue.getCacheValue() == null) { + // Entry is in DB but marked for deletion in cache, ignore it and check next entry continue; } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequestWithFSO.java index 2321504ad557..c537b09c85b8 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequestWithFSO.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequestWithFSO.java @@ -297,7 +297,7 @@ public void testDeleteParentAfterChildDeleted() throws Exception { // Create a child directory (tests checkSubDirectoryExists path) OMRequestTestUtils.addParentsToDirTable(volumeName, bucketName, parentDir + "/childDir", omMetadataManager); - + // Create a child file (tests checkSubFileExists path) String fileName = "childFile"; OmKeyInfo fileInfo = OMRequestTestUtils.createOmKeyInfo(volumeName, From 9ee0b26929941feae323efe09a77da42c3ca24ec Mon Sep 17 00:00:00 2001 From: Siyao Meng <50227127+smengcl@users.noreply.github.com> Date: Mon, 9 Feb 2026 22:36:26 -0800 Subject: [PATCH 3/3] Clean up --- .../org/apache/hadoop/ozone/om/request/file/OMFileRequest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java index be8361be501f..84722ede6294 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java @@ -871,7 +871,6 @@ private static boolean checkSubDirectoryExists(OmKeyInfo omKeyInfo, cacheIter.next(); OmDirectoryInfo cacheOmDirInfo = entry.getValue().getCacheValue(); if (cacheOmDirInfo == null) { - // Entry marked for deletion in cache (Ratis transaction committed but not yet flushed to DB) continue; } if (isImmediateChild(cacheOmDirInfo.getParentObjectID(), @@ -931,7 +930,6 @@ private static boolean checkSubFileExists(OmKeyInfo omKeyInfo, cacheIter.next(); OmKeyInfo cacheOmFileInfo = entry.getValue().getCacheValue(); if (cacheOmFileInfo == null) { - // Entry marked for deletion in cache (Ratis transaction committed but not yet flushed to DB) continue; } if (isImmediateChild(cacheOmFileInfo.getParentObjectID(),