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..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 @@ -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,44 @@ 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. + */ + @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); + + // Pause double buffer to prevent flushing deleted entries to DB + // This makes the bug reproduce deterministically + OzoneManagerDoubleBuffer doubleBuffer = getCluster().getOzoneManager() + .getOmRatisServer().getOmStateMachine().getOzoneManagerDoubleBuffer(); + doubleBuffer.pause(); + + 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 { + // 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 8403828f395a..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 @@ -889,11 +889,25 @@ 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 + 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; + } + + return true; } } @@ -933,11 +947,25 @@ 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 + 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; + } + + 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..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 @@ -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"); + } }