From dc872dd109e990dcc1825d29118b6c92e29af023 Mon Sep 17 00:00:00 2001 From: Sadanand Shenoy Date: Mon, 25 Sep 2023 12:18:12 +0530 Subject: [PATCH 1/3] rebase branch --- .../hadoop/fs/ozone/TestOzoneFileSystem.java | 14 +---- .../hadoop/ozone/om/TrashOzoneFileSystem.java | 63 +++++++++++-------- .../hadoop/ozone/om/TrashPolicyOzone.java | 28 +++++---- 3 files changed, 58 insertions(+), 47 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java index 118a4117b6be..16a7b856e9b8 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java @@ -61,7 +61,6 @@ import org.apache.hadoop.security.UserGroupInformation; import org.apache.ozone.test.GenericTestUtils; import org.apache.ozone.test.TestClock; -import org.apache.ozone.test.tag.Flaky; import org.junit.After; import org.junit.AfterClass; import org.junit.Assert; @@ -1635,7 +1634,6 @@ public void testDeleteRootWithTrash() throws IOException { * 2.Verify that the key gets deleted by the trash emptier. */ @Test - @Flaky("HDDS-6645") public void testTrash() throws Exception { String testKeyName = "testKey2"; Path path = new Path(OZONE_URI_DELIMITER, testKeyName); @@ -1655,12 +1653,10 @@ public void testTrash() throws Exception { // Call moveToTrash. We can't call protected fs.rename() directly trash.moveToTrash(path); - // Added this assertion here and will be tested as part of testTrash - // test case which needs to be tested with separate mini cluster having - // emptier thread started with close match of timings of relevant - // assertion statements and corresponding trash and checkpoint interval. + Assert.assertTrue(o3fs.exists(userTrash)); - Assert.assertTrue(o3fs.exists(userTrashCurrent)); + Assert.assertTrue(o3fs.exists(userTrashCurrent) || o3fs.listStatus( + o3fs.listStatus(userTrash)[0].getPath()).length > 0); // Wait until the TrashEmptier purges the key GenericTestUtils.waitFor(() -> { @@ -1673,10 +1669,6 @@ public void testTrash() throws Exception { } }, 100, 120000); - // userTrash path will contain the checkpoint folder - FileStatus[] statusList = fs.listStatus(userTrash); - Assert.assertNotEquals(Arrays.toString(statusList), 0, statusList.length); - // wait for deletion of checkpoint dir GenericTestUtils.waitFor(() -> { try { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java index 6779c3378bcd..766c89d7e6bd 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java @@ -28,8 +28,6 @@ import org.apache.hadoop.fs.FSDataOutputStream; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.PathIsNotEmptyDirectoryException; -import org.apache.hadoop.hdds.utils.db.Table; -import org.apache.hadoop.hdds.utils.db.TableIterator; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; import org.apache.hadoop.ozone.OFSPath; @@ -49,7 +47,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.Closeable; import java.io.IOException; import java.net.InetAddress; import java.net.URI; @@ -75,6 +72,8 @@ public class TrashOzoneFileSystem extends FileSystem { private static final int OZONE_FS_ITERATE_BATCH_SIZE = 100; + private static final int OZONE_MAX_LIST_KEYS_SIZE = 10000; + private final OzoneManager ozoneManager; private final String userName; @@ -167,9 +166,8 @@ public boolean rename(Path src, Path dst) throws IOException { equals(dstPath.getBucketName())); Preconditions.checkArgument(srcPath.getTrashRoot(). toString().equals(dstPath.getTrashRoot().toString())); - try (RenameIterator iterator = new RenameIterator(src, dst)) { - iterator.iterate(); - } + RenameIterator iterator = new RenameIterator(src, dst); + iterator.iterate(); return true; } @@ -198,9 +196,8 @@ public boolean delete(Path path, boolean b) throws IOException { if (bucket.getBucketLayout().isFileSystemOptimized()) { return deleteFSO(srcPath); } - try (DeleteIterator iterator = new DeleteIterator(path, true)) { - iterator.iterate(); - } + DeleteIterator iterator = new DeleteIterator(path, true); + iterator.iterate(); return true; } @@ -347,12 +344,11 @@ public boolean exists(Path f) throws IOException { } } - private abstract class OzoneListingIterator implements Closeable { + private abstract class OzoneListingIterator { private final Path path; private final FileStatus status; private String pathKey; - private TableIterator> - keyIterator; + private Iterator keyIterator; OzoneListingIterator(Path path) throws IOException { @@ -362,10 +358,35 @@ private abstract class OzoneListingIterator implements Closeable { if (status.isDirectory()) { this.pathKey = addTrailingSlashIfNeeded(pathKey); } - keyIterator = ozoneManager.getMetadataManager().getKeyIterator(); + OFSPath fsPath = new OFSPath(pathKey, + OzoneConfiguration.of(getConf())); + keyIterator = + getKeyIterator(fsPath.getVolumeName(), fsPath.getBucketName(), + fsPath.getKeyName()); } - /** + private Iterator getKeyIterator(String volumeName, + String bucketName, String keyName) throws IOException { + OMMetadataManager metadataManager = ozoneManager.getMetadataManager(); + List keys = new ArrayList<>( + metadataManager.listKeys(volumeName, bucketName, "", keyName, + OZONE_MAX_LIST_KEYS_SIZE)); + String lastKey = keys.get(keys.size() - 1).getKeyName(); + List nextBatchKeys = + metadataManager.listKeys(volumeName, bucketName, lastKey, keyName, + OZONE_MAX_LIST_KEYS_SIZE); + + while (!nextBatchKeys.isEmpty()) { + keys.addAll(nextBatchKeys); + lastKey = nextBatchKeys.get(nextBatchKeys.size() - 1).getKeyName(); + nextBatchKeys = + metadataManager.listKeys(volumeName, bucketName, lastKey, keyName, + OZONE_MAX_LIST_KEYS_SIZE); + } + return keys.iterator(); + } + + /** * The output of processKey determines if further iteration through the * keys should be done or not. * @@ -395,13 +416,10 @@ boolean iterate() throws IOException { String ofsPathprefix = ofsPath.getNonKeyPathNoPrefixDelim() + OZONE_URI_DELIMITER; while (keyIterator.hasNext()) { - Table.KeyValue< String, OmKeyInfo > kv = keyIterator.next(); - String keyPath = ofsPathprefix + kv.getValue().getKeyName(); + OmKeyInfo kv = keyIterator.next(); + String keyPath = ofsPathprefix + kv.getKeyName(); LOG.trace("iterating key path: {}", keyPath); - if (!kv.getValue().getKeyName().equals("") - && kv.getKey().startsWith("/" + pathKey)) { - keyPathList.add(keyPath); - } + keyPathList.add(keyPath); if (keyPathList.size() >= OZONE_FS_ITERATE_BATCH_SIZE) { if (!processKeyPath(keyPathList)) { return false; @@ -426,11 +444,6 @@ boolean iterate() throws IOException { FileStatus getStatus() { return status; } - - @Override - public void close() throws IOException { - keyIterator.close(); - } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java index aa44debd6f59..96d64c46c3f5 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java @@ -322,17 +322,8 @@ public void run() { continue; } TrashPolicyOzone trash = new TrashPolicyOzone(fs, conf, om); - Runnable task = () -> { - try { - om.getMetrics().incNumTrashRootsProcessed(); - trash.deleteCheckpoint(trashRoot.getPath(), false); - trash.createCheckpoint(trashRoot.getPath(), - new Date(Time.now())); - } catch (Exception e) { - om.getMetrics().incNumTrashFails(); - LOG.error("Unable to checkpoint:" + trashRoot.getPath(), e); - } - }; + Path trashRootPath = trashRoot.getPath(); + Runnable task = getEmptierTask(trashRootPath, trash, false); om.getMetrics().incNumTrashRootsEnqueued(); executor.submit(task); } @@ -357,6 +348,21 @@ public void run() { } } + public Runnable getEmptierTask(Path trashRootPath, TrashPolicyOzone trash, + boolean deleteImmediately) { + Runnable task = () -> { + try { + om.getMetrics().incNumTrashRootsProcessed(); + trash.deleteCheckpoint(trashRootPath, deleteImmediately); + trash.createCheckpoint(trashRootPath, new Date(Time.now())); + } catch (Exception e) { + om.getMetrics().incNumTrashFails(); + LOG.error("Unable to checkpoint:" + trashRootPath, e); + } + }; + return task; + } + private long ceiling(long time, long interval) { return floor(time, interval) + interval; } From 0d39e7b3f9c399f517c52f6ea24084fcd6f2d9bb Mon Sep 17 00:00:00 2001 From: Sadanand Shenoy Date: Thu, 28 Sep 2023 13:00:40 +0530 Subject: [PATCH 2/3] address comments --- .../hadoop/ozone/om/TrashOzoneFileSystem.java | 40 +++++++++++-------- .../hadoop/ozone/om/TrashPolicyOzone.java | 2 +- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java index 766c89d7e6bd..b3a166f9122d 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java @@ -56,6 +56,7 @@ import java.util.Map; import java.util.Iterator; import java.util.concurrent.atomic.AtomicLong; +import java.util.stream.Collectors; import static org.apache.hadoop.ozone.OzoneConsts.OZONE_O3TRASH_URI_SCHEME; import static org.apache.hadoop.ozone.OzoneConsts.OZONE_URI_DELIMITER; @@ -348,7 +349,7 @@ private abstract class OzoneListingIterator { private final Path path; private final FileStatus status; private String pathKey; - private Iterator keyIterator; + private Iterator keyIterator; OzoneListingIterator(Path path) throws IOException { @@ -365,23 +366,18 @@ private abstract class OzoneListingIterator { fsPath.getKeyName()); } - private Iterator getKeyIterator(String volumeName, + private Iterator getKeyIterator(String volumeName, String bucketName, String keyName) throws IOException { - OMMetadataManager metadataManager = ozoneManager.getMetadataManager(); - List keys = new ArrayList<>( - metadataManager.listKeys(volumeName, bucketName, "", keyName, - OZONE_MAX_LIST_KEYS_SIZE)); - String lastKey = keys.get(keys.size() - 1).getKeyName(); - List nextBatchKeys = - metadataManager.listKeys(volumeName, bucketName, lastKey, keyName, - OZONE_MAX_LIST_KEYS_SIZE); + List keys = new ArrayList<>( + listKeys(volumeName, bucketName, "", keyName)); + String lastKey = keys.get(keys.size() - 1); + List nextBatchKeys = + listKeys(volumeName, bucketName, lastKey, keyName); while (!nextBatchKeys.isEmpty()) { keys.addAll(nextBatchKeys); - lastKey = nextBatchKeys.get(nextBatchKeys.size() - 1).getKeyName(); - nextBatchKeys = - metadataManager.listKeys(volumeName, bucketName, lastKey, keyName, - OZONE_MAX_LIST_KEYS_SIZE); + lastKey = nextBatchKeys.get(nextBatchKeys.size() - 1); + nextBatchKeys = listKeys(volumeName, bucketName, lastKey, keyName); } return keys.iterator(); } @@ -416,8 +412,8 @@ boolean iterate() throws IOException { String ofsPathprefix = ofsPath.getNonKeyPathNoPrefixDelim() + OZONE_URI_DELIMITER; while (keyIterator.hasNext()) { - OmKeyInfo kv = keyIterator.next(); - String keyPath = ofsPathprefix + kv.getKeyName(); + String keyName = keyIterator.next(); + String keyPath = ofsPathprefix + keyName; LOG.trace("iterating key path: {}", keyPath); keyPathList.add(keyPath); if (keyPathList.size() >= OZONE_FS_ITERATE_BATCH_SIZE) { @@ -444,6 +440,18 @@ boolean iterate() throws IOException { FileStatus getStatus() { return status; } + + /** + * Return a listKeys output with only a list of keyNames. + */ + List listKeys(String volumeName, String bucketName, String startKey, + String keyPrefix) throws IOException { + OMMetadataManager metadataManager = ozoneManager.getMetadataManager(); + return metadataManager.listKeys(volumeName, bucketName, startKey, + keyPrefix, OZONE_MAX_LIST_KEYS_SIZE).stream() + .map(OmKeyInfo::getKeyName) + .collect(Collectors.toList()); + } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java index 96d64c46c3f5..cb5e36dd62ae 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java @@ -348,7 +348,7 @@ public void run() { } } - public Runnable getEmptierTask(Path trashRootPath, TrashPolicyOzone trash, + private Runnable getEmptierTask(Path trashRootPath, TrashPolicyOzone trash, boolean deleteImmediately) { Runnable task = () -> { try { From bf49eb906b7af3cffdef4238c5c85bf4f00cd8af Mon Sep 17 00:00:00 2001 From: Sadanand Shenoy Date: Thu, 28 Sep 2023 13:13:38 +0530 Subject: [PATCH 3/3] fix conflict after master merge --- .../java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java | 2 +- .../java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java index 16a7b856e9b8..452564a68582 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java @@ -1655,7 +1655,7 @@ public void testTrash() throws Exception { trash.moveToTrash(path); Assert.assertTrue(o3fs.exists(userTrash)); - Assert.assertTrue(o3fs.exists(userTrashCurrent) || o3fs.listStatus( + Assert.assertTrue(o3fs.exists(trashPath) || o3fs.listStatus( o3fs.listStatus(userTrash)[0].getPath()).length > 0); // Wait until the TrashEmptier purges the key diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java index b3a166f9122d..115a8c5f8e0a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java @@ -448,7 +448,7 @@ List listKeys(String volumeName, String bucketName, String startKey, String keyPrefix) throws IOException { OMMetadataManager metadataManager = ozoneManager.getMetadataManager(); return metadataManager.listKeys(volumeName, bucketName, startKey, - keyPrefix, OZONE_MAX_LIST_KEYS_SIZE).stream() + keyPrefix, OZONE_MAX_LIST_KEYS_SIZE).getKeys().stream() .map(OmKeyInfo::getKeyName) .collect(Collectors.toList()); }