You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/08/23 11:35:43 UTC
[GitHub] [ozone] navinko opened a new pull request, #3711: Hdds 7075 Implement FileSystem listStatusIterator to support file list paging
navinko opened a new pull request, #3711:
URL: https://github.com/apache/ozone/pull/3711
## What changes were proposed in this pull request?
OzoneFileSystem supposed to override default listStatusIterator.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7075
## How was this patch tested?
unit test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] navinko commented on a diff in pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
navinko commented on code in PR #3711:
URL: https://github.com/apache/ozone/pull/3711#discussion_r972381282
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java:
##########
@@ -387,6 +388,184 @@ public void testListStatus() throws Exception {
fs.delete(parent, true);
}
+ @Test
+ public void testListStatusIteratorWithDir() throws Exception {
+ Path parent = new Path(bucketPath, "testListStatus");
+ Path file1 = new Path(parent, "key1");
+ Path file2 = new Path(parent, "key2");
+
+ // Iterator should have no items when dir is empty
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ Assert.assertFalse(it.hasNext());
+ }
+ ContractTestUtils.touch(fs, file1);
+ ContractTestUtils.touch(fs, file2);
+ // Iterator should have an item when dir is not empty
+ it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertEquals("Parent path doesn't match",
+ fileStatus.getPath().toUri().getPath(), parent.toString());
+ }
+ // Iterator on a directory should return all subdirs along with
+ // files, even if there exists a file and sub-dir with the same name.
+ it = ofs.listStatusIterator(parent);
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals(
+ "Iterator did not return all the file status",
+ 2, iCount);
+ // Iterator should return file status for only the
+ // immediate children of a directory.
+ Path file3 = new Path(parent, "dir1/key3");
+ Path file4 = new Path(parent, "dir1/key4");
+ ContractTestUtils.touch(fs, file3);
+ ContractTestUtils.touch(fs, file4);
+ it = ofs.listStatusIterator(parent);
+ iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals("Iterator did not return file status " +
+ "of all the children of the directory", 3, iCount);
+
+ // Cleanup
+ fs.delete(parent, true);
+ }
+
+ /**
+ * Test listStatusIterator operation in a bucket.
+ */
+ @Test
+ public void testListStatusIteratorInBucket() throws Exception {
+ Path root = new Path("/" + volumeName + "/" + bucketName);
+ Path dir1 = new Path(root, "dir1");
+ Path dir12 = new Path(dir1, "dir12");
+ Path dir2 = new Path(root, "dir2");
+ try {
+ fs.mkdirs(dir12);
+ fs.mkdirs(dir2);
+
+ // ListStatus on root should return dir1 (even though /dir1 key does not
+ // exist) and dir2 only. dir12 is not an immediate child of root and
+ // hence should not be listed.
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(root);
+ // Verify that dir12 is not included in the result of the listStatus on
+ // root
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertNotEquals(fileStatus, dir12.toString());
+ }
+ Assert.assertEquals(
+ "FileStatus should return only the immediate children",
+ 2, iCount);
+
+ } finally {
+ // cleanup
+ fs.delete(dir1, true);
+ fs.delete(dir2, true);
+ }
+ }
+
+ /**
+ * Tests listStatusIterator operation on root directory.
+ */
+ @Test
+ public void testListStatusIteratorOnLargeDirectory() throws Exception {
+ Path root = new Path("/" + volumeName + "/" + bucketName);
+ Set<String> paths = new TreeSet<>();
+ int numDirs = LISTING_PAGE_SIZE + LISTING_PAGE_SIZE;
Review Comment:
@ChenSammi Added all the test cases for suggested numDirs.
testListStatusIteratorOnPageSize
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] navinko commented on pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
navinko commented on PR #3711:
URL: https://github.com/apache/ozone/pull/3711#issuecomment-1239648314
> FileStatusListing wrap class can also be removed. Refer to the example patch.
@ChenSammi Thanks for reviewing and the suggestions . Removed mentioned class.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] navinko commented on a diff in pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
navinko commented on code in PR #3711:
URL: https://github.com/apache/ozone/pull/3711#discussion_r965077547
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java:
##########
@@ -387,6 +388,184 @@ public void testListStatus() throws Exception {
fs.delete(parent, true);
}
+ @Test
+ public void testListStatusIteratorWithDir() throws Exception {
+ Path parent = new Path(bucketPath, "testListStatus");
+ Path file1 = new Path(parent, "key1");
+ Path file2 = new Path(parent, "key2");
+
+ // Iterator should have no items when dir is empty
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ Assert.assertFalse(it.hasNext());
Review Comment:
Resolved and added testListStatusIteratorWithPathNotFound
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java:
##########
@@ -387,6 +388,184 @@ public void testListStatus() throws Exception {
fs.delete(parent, true);
}
+ @Test
+ public void testListStatusIteratorWithDir() throws Exception {
+ Path parent = new Path(bucketPath, "testListStatus");
+ Path file1 = new Path(parent, "key1");
+ Path file2 = new Path(parent, "key2");
+
+ // Iterator should have no items when dir is empty
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ Assert.assertFalse(it.hasNext());
+ }
+ ContractTestUtils.touch(fs, file1);
+ ContractTestUtils.touch(fs, file2);
+ // Iterator should have an item when dir is not empty
+ it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertEquals("Parent path doesn't match",
+ fileStatus.getPath().toUri().getPath(), parent.toString());
+ }
+ // Iterator on a directory should return all subdirs along with
+ // files, even if there exists a file and sub-dir with the same name.
Review Comment:
removed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] navinko commented on a diff in pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
navinko commented on code in PR #3711:
URL: https://github.com/apache/ozone/pull/3711#discussion_r971852610
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -966,6 +966,135 @@ public RemoteIterator<LocatedFileStatus> listLocatedStatus(Path f)
return super.listLocatedStatus(f);
}
+ @Override
+ public RemoteIterator<FileStatus> listStatusIterator(Path f)
+ throws IOException {
+ return new OzoneFileStatusIterator<>(f);
+ }
+
+ /**
+ * A private class implementation for iterating list of file status.
+ *
+ * @param <T> the type of the file status.
+ */
+ private final class OzoneFileStatusIterator<T extends FileStatus>
+ implements RemoteIterator<T> {
+ private List<FileStatus> thisListing;
+ private int i;
+ private Path p;
+ private T curStat = null;
+ private String startPath = "";
+
+ /**
+ * Constructor to initialize OzoneFileStatusIterator.
+ * Get the first batch of entry for iteration.
+ *
+ * @param p path to file/directory.
+ * @throws IOException
+ */
+ private OzoneFileStatusIterator(Path p) throws IOException {
+ this.p = p;
+ // fetch the first batch of entries in the directory
+ thisListing = listFileStatus(p, startPath);
+ if (thisListing != null && !thisListing.isEmpty()) {
+ startPath = pathToKey(
+ thisListing.get(thisListing.size() - 1).getPath());
+ LOG.info("Got {} file status, next start path {}",
Review Comment:
Changed it to debug
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] navinko commented on a diff in pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
navinko commented on code in PR #3711:
URL: https://github.com/apache/ozone/pull/3711#discussion_r972381282
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java:
##########
@@ -387,6 +388,184 @@ public void testListStatus() throws Exception {
fs.delete(parent, true);
}
+ @Test
+ public void testListStatusIteratorWithDir() throws Exception {
+ Path parent = new Path(bucketPath, "testListStatus");
+ Path file1 = new Path(parent, "key1");
+ Path file2 = new Path(parent, "key2");
+
+ // Iterator should have no items when dir is empty
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ Assert.assertFalse(it.hasNext());
+ }
+ ContractTestUtils.touch(fs, file1);
+ ContractTestUtils.touch(fs, file2);
+ // Iterator should have an item when dir is not empty
+ it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertEquals("Parent path doesn't match",
+ fileStatus.getPath().toUri().getPath(), parent.toString());
+ }
+ // Iterator on a directory should return all subdirs along with
+ // files, even if there exists a file and sub-dir with the same name.
+ it = ofs.listStatusIterator(parent);
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals(
+ "Iterator did not return all the file status",
+ 2, iCount);
+ // Iterator should return file status for only the
+ // immediate children of a directory.
+ Path file3 = new Path(parent, "dir1/key3");
+ Path file4 = new Path(parent, "dir1/key4");
+ ContractTestUtils.touch(fs, file3);
+ ContractTestUtils.touch(fs, file4);
+ it = ofs.listStatusIterator(parent);
+ iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals("Iterator did not return file status " +
+ "of all the children of the directory", 3, iCount);
+
+ // Cleanup
+ fs.delete(parent, true);
+ }
+
+ /**
+ * Test listStatusIterator operation in a bucket.
+ */
+ @Test
+ public void testListStatusIteratorInBucket() throws Exception {
+ Path root = new Path("/" + volumeName + "/" + bucketName);
+ Path dir1 = new Path(root, "dir1");
+ Path dir12 = new Path(dir1, "dir12");
+ Path dir2 = new Path(root, "dir2");
+ try {
+ fs.mkdirs(dir12);
+ fs.mkdirs(dir2);
+
+ // ListStatus on root should return dir1 (even though /dir1 key does not
+ // exist) and dir2 only. dir12 is not an immediate child of root and
+ // hence should not be listed.
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(root);
+ // Verify that dir12 is not included in the result of the listStatus on
+ // root
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertNotEquals(fileStatus, dir12.toString());
+ }
+ Assert.assertEquals(
+ "FileStatus should return only the immediate children",
+ 2, iCount);
+
+ } finally {
+ // cleanup
+ fs.delete(dir1, true);
+ fs.delete(dir2, true);
+ }
+ }
+
+ /**
+ * Tests listStatusIterator operation on root directory.
+ */
+ @Test
+ public void testListStatusIteratorOnLargeDirectory() throws Exception {
+ Path root = new Path("/" + volumeName + "/" + bucketName);
+ Set<String> paths = new TreeSet<>();
+ int numDirs = LISTING_PAGE_SIZE + LISTING_PAGE_SIZE;
Review Comment:
@ChenSammi I had added 3 test cases for suggested numDir earlier as it was just one line change and duplicating the code . I solved the checkstyle issue in my setup which was not allowing me to push switch case.
Added all the test cases for suggested numDirs.
testListStatusIteratorOnPageSize
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ChenSammi commented on pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on PR #3711:
URL: https://github.com/apache/ozone/pull/3711#issuecomment-1239073804
FileStatusListing wrap class can also be removed. Refer to the example patch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] navinko commented on a diff in pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
navinko commented on code in PR #3711:
URL: https://github.com/apache/ozone/pull/3711#discussion_r965078654
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java:
##########
@@ -387,6 +388,184 @@ public void testListStatus() throws Exception {
fs.delete(parent, true);
}
+ @Test
+ public void testListStatusIteratorWithDir() throws Exception {
+ Path parent = new Path(bucketPath, "testListStatus");
+ Path file1 = new Path(parent, "key1");
+ Path file2 = new Path(parent, "key2");
+
+ // Iterator should have no items when dir is empty
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ Assert.assertFalse(it.hasNext());
+ }
+ ContractTestUtils.touch(fs, file1);
+ ContractTestUtils.touch(fs, file2);
+ // Iterator should have an item when dir is not empty
+ it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertEquals("Parent path doesn't match",
+ fileStatus.getPath().toUri().getPath(), parent.toString());
+ }
+ // Iterator on a directory should return all subdirs along with
+ // files, even if there exists a file and sub-dir with the same name.
+ it = ofs.listStatusIterator(parent);
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals(
+ "Iterator did not return all the file status",
+ 2, iCount);
+ // Iterator should return file status for only the
+ // immediate children of a directory.
+ Path file3 = new Path(parent, "dir1/key3");
+ Path file4 = new Path(parent, "dir1/key4");
+ ContractTestUtils.touch(fs, file3);
+ ContractTestUtils.touch(fs, file4);
+ it = ofs.listStatusIterator(parent);
+ iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals("Iterator did not return file status " +
+ "of all the children of the directory", 3, iCount);
+
+ // Cleanup
+ fs.delete(parent, true);
+ }
+
+ /**
+ * Test listStatusIterator operation in a bucket.
+ */
+ @Test
+ public void testListStatusIteratorInBucket() throws Exception {
+ Path root = new Path("/" + volumeName + "/" + bucketName);
+ Path dir1 = new Path(root, "dir1");
+ Path dir12 = new Path(dir1, "dir12");
+ Path dir2 = new Path(root, "dir2");
+ try {
+ fs.mkdirs(dir12);
+ fs.mkdirs(dir2);
+
+ // ListStatus on root should return dir1 (even though /dir1 key does not
+ // exist) and dir2 only. dir12 is not an immediate child of root and
+ // hence should not be listed.
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(root);
+ // Verify that dir12 is not included in the result of the listStatus on
+ // root
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertNotEquals(fileStatus, dir12.toString());
+ }
+ Assert.assertEquals(
+ "FileStatus should return only the immediate children",
+ 2, iCount);
+
+ } finally {
+ // cleanup
+ fs.delete(dir1, true);
+ fs.delete(dir2, true);
+ }
+ }
+
+ /**
+ * Tests listStatusIterator operation on root directory.
+ */
+ @Test
+ public void testListStatusIteratorOnLargeDirectory() throws Exception {
+ Path root = new Path("/" + volumeName + "/" + bucketName);
+ Set<String> paths = new TreeSet<>();
+ int numDirs = LISTING_PAGE_SIZE + LISTING_PAGE_SIZE;
Review Comment:
added test cases
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ChenSammi merged pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
ChenSammi merged PR #3711:
URL: https://github.com/apache/ozone/pull/3711
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] navinko commented on a diff in pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
navinko commented on code in PR #3711:
URL: https://github.com/apache/ozone/pull/3711#discussion_r972389476
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -966,6 +966,135 @@ public RemoteIterator<LocatedFileStatus> listLocatedStatus(Path f)
return super.listLocatedStatus(f);
}
+ @Override
+ public RemoteIterator<FileStatus> listStatusIterator(Path f)
+ throws IOException {
+ return new OzoneFileStatusIterator<>(f);
+ }
+
+ /**
+ * A private class implementation for iterating list of file status.
+ *
+ * @param <T> the type of the file status.
+ */
+ private final class OzoneFileStatusIterator<T extends FileStatus>
+ implements RemoteIterator<T> {
+ private List<FileStatus> thisListing;
+ private int i;
+ private Path p;
+ private T curStat = null;
+ private String startPath = "";
+
+ /**
+ * Constructor to initialize OzoneFileStatusIterator.
+ * Get the first batch of entry for iteration.
+ *
+ * @param p path to file/directory.
+ * @throws IOException
+ */
+ private OzoneFileStatusIterator(Path p) throws IOException {
+ this.p = p;
+ // fetch the first batch of entries in the directory
+ thisListing = listFileStatus(p, startPath);
+ if (thisListing != null && !thisListing.isEmpty()) {
+ startPath = pathToKey(
+ thisListing.get(thisListing.size() - 1).getPath());
+ LOG.info("Got {} file status, next start path {}",
+ thisListing.size(), startPath);
+ }
+ statistics.incrementReadOps(1);
Review Comment:
@ChenSammi Thank you for the review.
Updated the PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ChenSammi commented on pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on PR #3711:
URL: https://github.com/apache/ozone/pull/3711#issuecomment-1254528278
The last patch LGTM, +1.
@jojochuang , would you like to take another look of the patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] navinko commented on a diff in pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
navinko commented on code in PR #3711:
URL: https://github.com/apache/ozone/pull/3711#discussion_r965075526
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java:
##########
@@ -387,6 +388,184 @@ public void testListStatus() throws Exception {
fs.delete(parent, true);
}
+ @Test
+ public void testListStatusIteratorWithDir() throws Exception {
+ Path parent = new Path(bucketPath, "testListStatus");
+ Path file1 = new Path(parent, "key1");
+ Path file2 = new Path(parent, "key2");
+
+ // Iterator should have no items when dir is empty
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ Assert.assertFalse(it.hasNext());
+ }
+ ContractTestUtils.touch(fs, file1);
+ ContractTestUtils.touch(fs, file2);
+ // Iterator should have an item when dir is not empty
+ it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertEquals("Parent path doesn't match",
+ fileStatus.getPath().toUri().getPath(), parent.toString());
+ }
+ // Iterator on a directory should return all subdirs along with
+ // files, even if there exists a file and sub-dir with the same name.
+ it = ofs.listStatusIterator(parent);
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals(
+ "Iterator did not return all the file status",
+ 2, iCount);
+ // Iterator should return file status for only the
+ // immediate children of a directory.
+ Path file3 = new Path(parent, "dir1/key3");
+ Path file4 = new Path(parent, "dir1/key4");
+ ContractTestUtils.touch(fs, file3);
+ ContractTestUtils.touch(fs, file4);
+ it = ofs.listStatusIterator(parent);
+ iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals("Iterator did not return file status " +
+ "of all the children of the directory", 3, iCount);
+
+ // Cleanup
+ fs.delete(parent, true);
+ }
+
+ /**
+ * Test listStatusIterator operation in a bucket.
+ */
+ @Test
+ public void testListStatusIteratorInBucket() throws Exception {
+ Path root = new Path("/" + volumeName + "/" + bucketName);
+ Path dir1 = new Path(root, "dir1");
+ Path dir12 = new Path(dir1, "dir12");
+ Path dir2 = new Path(root, "dir2");
+ try {
+ fs.mkdirs(dir12);
+ fs.mkdirs(dir2);
+
+ // ListStatus on root should return dir1 (even though /dir1 key does not
+ // exist) and dir2 only. dir12 is not an immediate child of root and
+ // hence should not be listed.
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(root);
+ // Verify that dir12 is not included in the result of the listStatus on
+ // root
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertNotEquals(fileStatus, dir12.toString());
+ }
+ Assert.assertEquals(
+ "FileStatus should return only the immediate children",
+ 2, iCount);
+
+ } finally {
+ // cleanup
+ fs.delete(dir1, true);
+ fs.delete(dir2, true);
+ }
+ }
+
+ /**
+ * Tests listStatusIterator operation on root directory.
+ */
+ @Test
+ public void testListStatusIteratorOnLargeDirectory() throws Exception {
+ Path root = new Path("/" + volumeName + "/" + bucketName);
+ Set<String> paths = new TreeSet<>();
+ int numDirs = LISTING_PAGE_SIZE + LISTING_PAGE_SIZE;
+ try {
+ for (int i = 0; i < numDirs; i++) {
+ Path p = new Path(root, String.valueOf(i));
+ fs.mkdirs(p);
+ paths.add(p.getName());
+ }
+
+ RemoteIterator<FileStatus> iterator = ofs.listStatusIterator(root);
+ int iCount = 0;
+ if (iterator != null) {
+ while (iterator.hasNext()) {
+ FileStatus fileStatus = iterator.next();
+ iCount++;
+ LOG.info("path:{} in FileStatus " + fileStatus.getPath());
Review Comment:
removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] navinko commented on a diff in pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
navinko commented on code in PR #3711:
URL: https://github.com/apache/ozone/pull/3711#discussion_r971855293
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -966,6 +966,135 @@ public RemoteIterator<LocatedFileStatus> listLocatedStatus(Path f)
return super.listLocatedStatus(f);
}
+ @Override
+ public RemoteIterator<FileStatus> listStatusIterator(Path f)
+ throws IOException {
+ return new OzoneFileStatusIterator<>(f);
+ }
+
+ /**
+ * A private class implementation for iterating list of file status.
+ *
+ * @param <T> the type of the file status.
+ */
+ private final class OzoneFileStatusIterator<T extends FileStatus>
+ implements RemoteIterator<T> {
+ private List<FileStatus> thisListing;
+ private int i;
+ private Path p;
+ private T curStat = null;
+ private String startPath = "";
+
+ /**
+ * Constructor to initialize OzoneFileStatusIterator.
+ * Get the first batch of entry for iteration.
+ *
+ * @param p path to file/directory.
+ * @throws IOException
+ */
+ private OzoneFileStatusIterator(Path p) throws IOException {
+ this.p = p;
+ // fetch the first batch of entries in the directory
+ thisListing = listFileStatus(p, startPath);
+ if (thisListing != null && !thisListing.isEmpty()) {
+ startPath = pathToKey(
+ thisListing.get(thisListing.size() - 1).getPath());
+ LOG.info("Got {} file status, next start path {}",
+ thisListing.size(), startPath);
+ }
+ statistics.incrementReadOps(1);
Review Comment:
Thanks @ChenSammi for pointing out ,corrected it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] navinko commented on a diff in pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
navinko commented on code in PR #3711:
URL: https://github.com/apache/ozone/pull/3711#discussion_r965077845
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java:
##########
@@ -387,6 +388,184 @@ public void testListStatus() throws Exception {
fs.delete(parent, true);
}
+ @Test
+ public void testListStatusIteratorWithDir() throws Exception {
+ Path parent = new Path(bucketPath, "testListStatus");
+ Path file1 = new Path(parent, "key1");
+ Path file2 = new Path(parent, "key2");
+
+ // Iterator should have no items when dir is empty
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ Assert.assertFalse(it.hasNext());
+ }
+ ContractTestUtils.touch(fs, file1);
+ ContractTestUtils.touch(fs, file2);
+ // Iterator should have an item when dir is not empty
+ it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertEquals("Parent path doesn't match",
+ fileStatus.getPath().toUri().getPath(), parent.toString());
+ }
+ // Iterator on a directory should return all subdirs along with
+ // files, even if there exists a file and sub-dir with the same name.
+ it = ofs.listStatusIterator(parent);
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals(
+ "Iterator did not return all the file status",
+ 2, iCount);
+ // Iterator should return file status for only the
+ // immediate children of a directory.
+ Path file3 = new Path(parent, "dir1/key3");
+ Path file4 = new Path(parent, "dir1/key4");
+ ContractTestUtils.touch(fs, file3);
+ ContractTestUtils.touch(fs, file4);
+ it = ofs.listStatusIterator(parent);
+ iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals("Iterator did not return file status " +
+ "of all the children of the directory", 3, iCount);
+
+ // Cleanup
+ fs.delete(parent, true);
Review Comment:
corrected
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ChenSammi commented on a diff in pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3711:
URL: https://github.com/apache/ozone/pull/3711#discussion_r964406538
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -966,6 +1002,107 @@ public RemoteIterator<LocatedFileStatus> listLocatedStatus(Path f)
return super.listLocatedStatus(f);
}
+ @Override
+ public RemoteIterator<FileStatus> listStatusIterator(Path f)
+ throws IOException {
Review Comment:
Please change the continued line indent from 8 to 4.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java:
##########
@@ -387,6 +388,184 @@ public void testListStatus() throws Exception {
fs.delete(parent, true);
}
+ @Test
+ public void testListStatusIteratorWithDir() throws Exception {
+ Path parent = new Path(bucketPath, "testListStatus");
+ Path file1 = new Path(parent, "key1");
+ Path file2 = new Path(parent, "key2");
+
+ // Iterator should have no items when dir is empty
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ Assert.assertFalse(it.hasNext());
+ }
+ ContractTestUtils.touch(fs, file1);
+ ContractTestUtils.touch(fs, file2);
+ // Iterator should have an item when dir is not empty
+ it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertEquals("Parent path doesn't match",
+ fileStatus.getPath().toUri().getPath(), parent.toString());
+ }
+ // Iterator on a directory should return all subdirs along with
+ // files, even if there exists a file and sub-dir with the same name.
+ it = ofs.listStatusIterator(parent);
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals(
+ "Iterator did not return all the file status",
+ 2, iCount);
+ // Iterator should return file status for only the
+ // immediate children of a directory.
+ Path file3 = new Path(parent, "dir1/key3");
+ Path file4 = new Path(parent, "dir1/key4");
+ ContractTestUtils.touch(fs, file3);
+ ContractTestUtils.touch(fs, file4);
+ it = ofs.listStatusIterator(parent);
+ iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals("Iterator did not return file status " +
+ "of all the children of the directory", 3, iCount);
+
+ // Cleanup
+ fs.delete(parent, true);
+ }
+
+ /**
+ * Test listStatusIterator operation in a bucket.
+ */
+ @Test
+ public void testListStatusIteratorInBucket() throws Exception {
+ Path root = new Path("/" + volumeName + "/" + bucketName);
+ Path dir1 = new Path(root, "dir1");
+ Path dir12 = new Path(dir1, "dir12");
+ Path dir2 = new Path(root, "dir2");
+ try {
+ fs.mkdirs(dir12);
+ fs.mkdirs(dir2);
+
+ // ListStatus on root should return dir1 (even though /dir1 key does not
+ // exist) and dir2 only. dir12 is not an immediate child of root and
+ // hence should not be listed.
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(root);
+ // Verify that dir12 is not included in the result of the listStatus on
+ // root
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertNotEquals(fileStatus, dir12.toString());
+ }
+ Assert.assertEquals(
+ "FileStatus should return only the immediate children",
+ 2, iCount);
+
+ } finally {
+ // cleanup
+ fs.delete(dir1, true);
+ fs.delete(dir2, true);
+ }
+ }
+
+ /**
+ * Tests listStatusIterator operation on root directory.
+ */
+ @Test
+ public void testListStatusIteratorOnLargeDirectory() throws Exception {
+ Path root = new Path("/" + volumeName + "/" + bucketName);
+ Set<String> paths = new TreeSet<>();
+ int numDirs = LISTING_PAGE_SIZE + LISTING_PAGE_SIZE;
+ try {
+ for (int i = 0; i < numDirs; i++) {
+ Path p = new Path(root, String.valueOf(i));
+ fs.mkdirs(p);
+ paths.add(p.getName());
+ }
+
+ RemoteIterator<FileStatus> iterator = ofs.listStatusIterator(root);
+ int iCount = 0;
+ if (iterator != null) {
+ while (iterator.hasNext()) {
+ FileStatus fileStatus = iterator.next();
+ iCount++;
+ LOG.info("path:{} in FileStatus " + fileStatus.getPath());
+ Assert.assertTrue(paths.contains(fileStatus.getPath().getName()));
+ }
+ }
+ Assert.assertEquals(
+ "Total directories listed do not match the existing directories",
+ numDirs, iCount);
+
+ } finally {
+ // Cleanup
+ for (int i = 0; i < numDirs; i++) {
+ Path p = new Path(root, String.valueOf(i));
+ fs.delete(p, true);
+ }
+ }
+ }
+
+ /**
+ * Tests listStatusIterator on a path with subdirs.
+ */
+ @Test
+ public void testListStatusIteratorOnSubDirs() throws Exception {
+ // Create the following key structure
+ // /dir1/dir11/dir111
+ // /dir1/dir12
+ // /dir1/dir12/file121
+ // /dir2
+ // listStatusIterator on /dir1 should return all its immediated subdirs only
+ // which are /dir1/dir11 and /dir1/dir12. Super child files/dirs
+ // (/dir1/dir12/file121 and /dir1/dir11/dir111) should not be returned by
+ // listStatusIterator.
+ Path dir1 = new Path(bucketPath, "dir1");
+ Path dir11 = new Path(dir1, "dir11");
+ Path dir111 = new Path(dir11, "dir111");
+ Path dir12 = new Path(dir1, "dir12");
+ Path file121 = new Path(dir12, "file121");
+ Path dir2 = new Path(bucketPath, "dir2");
+ fs.mkdirs(dir111);
+ fs.mkdirs(dir12);
+ ContractTestUtils.touch(fs, file121);
+ fs.mkdirs(dir2);
+
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(dir1);
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertNotEquals(fileStatus, dir12.toString());
+ // Verify that the two children of /dir1
+ // returned by listStatusIterator operation
+ // are /dir1/dir11 and /dir1/dir12.
+ Assert.assertTrue(
+ fileStatus.getPath().toUri().getPath().
+ equals(dir11.toString()) ||
+ fileStatus.getPath().toUri().getPath().
+ equals(dir12.toString()));
+ }
+ Assert.assertEquals(
+ "Iterator should return only the immediate children",
+ 2, iCount);
+
+ // Cleanup
+ fs.delete(dir2, true);
Review Comment:
We should put the cleanup in the finally clause.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java:
##########
@@ -387,6 +388,184 @@ public void testListStatus() throws Exception {
fs.delete(parent, true);
}
+ @Test
+ public void testListStatusIteratorWithDir() throws Exception {
+ Path parent = new Path(bucketPath, "testListStatus");
+ Path file1 = new Path(parent, "key1");
+ Path file2 = new Path(parent, "key2");
+
+ // Iterator should have no items when dir is empty
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ Assert.assertFalse(it.hasNext());
+ }
+ ContractTestUtils.touch(fs, file1);
+ ContractTestUtils.touch(fs, file2);
+ // Iterator should have an item when dir is not empty
+ it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertEquals("Parent path doesn't match",
+ fileStatus.getPath().toUri().getPath(), parent.toString());
+ }
+ // Iterator on a directory should return all subdirs along with
+ // files, even if there exists a file and sub-dir with the same name.
+ it = ofs.listStatusIterator(parent);
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals(
+ "Iterator did not return all the file status",
+ 2, iCount);
+ // Iterator should return file status for only the
+ // immediate children of a directory.
+ Path file3 = new Path(parent, "dir1/key3");
+ Path file4 = new Path(parent, "dir1/key4");
+ ContractTestUtils.touch(fs, file3);
+ ContractTestUtils.touch(fs, file4);
+ it = ofs.listStatusIterator(parent);
+ iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals("Iterator did not return file status " +
+ "of all the children of the directory", 3, iCount);
+
+ // Cleanup
+ fs.delete(parent, true);
+ }
+
+ /**
+ * Test listStatusIterator operation in a bucket.
+ */
+ @Test
+ public void testListStatusIteratorInBucket() throws Exception {
+ Path root = new Path("/" + volumeName + "/" + bucketName);
+ Path dir1 = new Path(root, "dir1");
+ Path dir12 = new Path(dir1, "dir12");
+ Path dir2 = new Path(root, "dir2");
+ try {
+ fs.mkdirs(dir12);
+ fs.mkdirs(dir2);
+
+ // ListStatus on root should return dir1 (even though /dir1 key does not
+ // exist) and dir2 only. dir12 is not an immediate child of root and
+ // hence should not be listed.
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(root);
+ // Verify that dir12 is not included in the result of the listStatus on
+ // root
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertNotEquals(fileStatus, dir12.toString());
+ }
+ Assert.assertEquals(
+ "FileStatus should return only the immediate children",
+ 2, iCount);
+
+ } finally {
+ // cleanup
+ fs.delete(dir1, true);
+ fs.delete(dir2, true);
+ }
+ }
+
+ /**
+ * Tests listStatusIterator operation on root directory.
+ */
+ @Test
+ public void testListStatusIteratorOnLargeDirectory() throws Exception {
+ Path root = new Path("/" + volumeName + "/" + bucketName);
+ Set<String> paths = new TreeSet<>();
+ int numDirs = LISTING_PAGE_SIZE + LISTING_PAGE_SIZE;
+ try {
+ for (int i = 0; i < numDirs; i++) {
+ Path p = new Path(root, String.valueOf(i));
+ fs.mkdirs(p);
+ paths.add(p.getName());
+ }
+
+ RemoteIterator<FileStatus> iterator = ofs.listStatusIterator(root);
+ int iCount = 0;
+ if (iterator != null) {
+ while (iterator.hasNext()) {
+ FileStatus fileStatus = iterator.next();
+ iCount++;
+ LOG.info("path:{} in FileStatus " + fileStatus.getPath());
Review Comment:
Please remove this log.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java:
##########
@@ -387,6 +388,184 @@ public void testListStatus() throws Exception {
fs.delete(parent, true);
}
+ @Test
+ public void testListStatusIteratorWithDir() throws Exception {
+ Path parent = new Path(bucketPath, "testListStatus");
+ Path file1 = new Path(parent, "key1");
+ Path file2 = new Path(parent, "key2");
+
+ // Iterator should have no items when dir is empty
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ Assert.assertFalse(it.hasNext());
+ }
+ ContractTestUtils.touch(fs, file1);
+ ContractTestUtils.touch(fs, file2);
+ // Iterator should have an item when dir is not empty
+ it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertEquals("Parent path doesn't match",
+ fileStatus.getPath().toUri().getPath(), parent.toString());
+ }
+ // Iterator on a directory should return all subdirs along with
+ // files, even if there exists a file and sub-dir with the same name.
+ it = ofs.listStatusIterator(parent);
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals(
+ "Iterator did not return all the file status",
+ 2, iCount);
+ // Iterator should return file status for only the
+ // immediate children of a directory.
+ Path file3 = new Path(parent, "dir1/key3");
+ Path file4 = new Path(parent, "dir1/key4");
+ ContractTestUtils.touch(fs, file3);
+ ContractTestUtils.touch(fs, file4);
+ it = ofs.listStatusIterator(parent);
+ iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals("Iterator did not return file status " +
+ "of all the children of the directory", 3, iCount);
+
+ // Cleanup
+ fs.delete(parent, true);
+ }
+
+ /**
+ * Test listStatusIterator operation in a bucket.
+ */
+ @Test
+ public void testListStatusIteratorInBucket() throws Exception {
+ Path root = new Path("/" + volumeName + "/" + bucketName);
+ Path dir1 = new Path(root, "dir1");
+ Path dir12 = new Path(dir1, "dir12");
+ Path dir2 = new Path(root, "dir2");
+ try {
+ fs.mkdirs(dir12);
+ fs.mkdirs(dir2);
+
+ // ListStatus on root should return dir1 (even though /dir1 key does not
+ // exist) and dir2 only. dir12 is not an immediate child of root and
+ // hence should not be listed.
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(root);
+ // Verify that dir12 is not included in the result of the listStatus on
+ // root
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertNotEquals(fileStatus, dir12.toString());
+ }
+ Assert.assertEquals(
+ "FileStatus should return only the immediate children",
+ 2, iCount);
+
+ } finally {
+ // cleanup
+ fs.delete(dir1, true);
+ fs.delete(dir2, true);
+ }
+ }
+
+ /**
+ * Tests listStatusIterator operation on root directory.
+ */
+ @Test
+ public void testListStatusIteratorOnLargeDirectory() throws Exception {
+ Path root = new Path("/" + volumeName + "/" + bucketName);
+ Set<String> paths = new TreeSet<>();
+ int numDirs = LISTING_PAGE_SIZE + LISTING_PAGE_SIZE;
Review Comment:
Can you add more different numDirs value for this test, say 1, LISTING_PAGE_SIZE, LISTING_PAGE_SIZE + 1, LISTING_PAGE_SIZE -1, LISTING_PAGE_SIZE + LISTING_PAGE_SIZE/2, etc.
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java:
##########
@@ -864,6 +900,107 @@ public RemoteIterator<LocatedFileStatus> listLocatedStatus(Path f)
return super.listLocatedStatus(f);
}
+ @Override
+ public RemoteIterator<FileStatus> listStatusIterator(Path f)
+ throws IOException {
+ return new OzoneFileStatusIterator<>(f);
+ }
+
+ /**
+ * A private class implementation for iterating list of file status.
+ *
+ * @param <T> the type of the file status
+ */
+ private final class OzoneFileStatusIterator<T extends FileStatus>
Review Comment:
extra space between class and OzoneFileStatusIterator.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java:
##########
@@ -387,6 +388,184 @@ public void testListStatus() throws Exception {
fs.delete(parent, true);
}
+ @Test
+ public void testListStatusIteratorWithDir() throws Exception {
+ Path parent = new Path(bucketPath, "testListStatus");
+ Path file1 = new Path(parent, "key1");
+ Path file2 = new Path(parent, "key2");
+
+ // Iterator should have no items when dir is empty
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ Assert.assertFalse(it.hasNext());
+ }
+ ContractTestUtils.touch(fs, file1);
+ ContractTestUtils.touch(fs, file2);
+ // Iterator should have an item when dir is not empty
+ it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertEquals("Parent path doesn't match",
+ fileStatus.getPath().toUri().getPath(), parent.toString());
+ }
+ // Iterator on a directory should return all subdirs along with
+ // files, even if there exists a file and sub-dir with the same name.
+ it = ofs.listStatusIterator(parent);
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals(
+ "Iterator did not return all the file status",
+ 2, iCount);
+ // Iterator should return file status for only the
+ // immediate children of a directory.
+ Path file3 = new Path(parent, "dir1/key3");
+ Path file4 = new Path(parent, "dir1/key4");
+ ContractTestUtils.touch(fs, file3);
+ ContractTestUtils.touch(fs, file4);
+ it = ofs.listStatusIterator(parent);
+ iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals("Iterator did not return file status " +
+ "of all the children of the directory", 3, iCount);
+
+ // Cleanup
+ fs.delete(parent, true);
Review Comment:
Should put the cleanup in finally clause.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java:
##########
@@ -387,6 +388,184 @@ public void testListStatus() throws Exception {
fs.delete(parent, true);
}
+ @Test
+ public void testListStatusIteratorWithDir() throws Exception {
+ Path parent = new Path(bucketPath, "testListStatus");
+ Path file1 = new Path(parent, "key1");
+ Path file2 = new Path(parent, "key2");
+
+ // Iterator should have no items when dir is empty
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ Assert.assertFalse(it.hasNext());
Review Comment:
This line is unreachable if it.hasNext is false.
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java:
##########
@@ -635,6 +635,42 @@ workingDir, getUsername())
return statuses.toArray(new FileStatus[0]);
}
+ /**
+ * Get all the file status for input path and startPath.
+ *
+ * @param f
+ * @param startPath
+ * @return
+ * @throws IOException
+ */
+ public FileStatusListing listFileStatus(Path f, String startPath)
Review Comment:
Complete the java doc statement and make it a protected or private class since it's only called internally.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java:
##########
@@ -387,6 +388,184 @@ public void testListStatus() throws Exception {
fs.delete(parent, true);
}
+ @Test
+ public void testListStatusIteratorWithDir() throws Exception {
+ Path parent = new Path(bucketPath, "testListStatus");
+ Path file1 = new Path(parent, "key1");
+ Path file2 = new Path(parent, "key2");
+
+ // Iterator should have no items when dir is empty
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ Assert.assertFalse(it.hasNext());
Review Comment:
Please add one more test for query an path which doesn't exist.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java:
##########
@@ -387,6 +388,184 @@ public void testListStatus() throws Exception {
fs.delete(parent, true);
}
+ @Test
+ public void testListStatusIteratorWithDir() throws Exception {
+ Path parent = new Path(bucketPath, "testListStatus");
+ Path file1 = new Path(parent, "key1");
+ Path file2 = new Path(parent, "key2");
+
+ // Iterator should have no items when dir is empty
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ Assert.assertFalse(it.hasNext());
+ }
+ ContractTestUtils.touch(fs, file1);
+ ContractTestUtils.touch(fs, file2);
+ // Iterator should have an item when dir is not empty
+ it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertEquals("Parent path doesn't match",
+ fileStatus.getPath().toUri().getPath(), parent.toString());
+ }
+ // Iterator on a directory should return all subdirs along with
+ // files, even if there exists a file and sub-dir with the same name.
Review Comment:
Same name should not be allowed. please remove this statement.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ChenSammi commented on pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on PR #3711:
URL: https://github.com/apache/ozone/pull/3711#issuecomment-1257167136
Thanks @navinko for the contribution and @jojochuang @adoroszlai for code review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ChenSammi commented on a diff in pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3711:
URL: https://github.com/apache/ozone/pull/3711#discussion_r971732530
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -966,6 +966,135 @@ public RemoteIterator<LocatedFileStatus> listLocatedStatus(Path f)
return super.listLocatedStatus(f);
}
+ @Override
+ public RemoteIterator<FileStatus> listStatusIterator(Path f)
+ throws IOException {
+ return new OzoneFileStatusIterator<>(f);
+ }
+
+ /**
+ * A private class implementation for iterating list of file status.
+ *
+ * @param <T> the type of the file status.
+ */
+ private final class OzoneFileStatusIterator<T extends FileStatus>
+ implements RemoteIterator<T> {
+ private List<FileStatus> thisListing;
+ private int i;
+ private Path p;
+ private T curStat = null;
+ private String startPath = "";
+
+ /**
+ * Constructor to initialize OzoneFileStatusIterator.
+ * Get the first batch of entry for iteration.
+ *
+ * @param p path to file/directory.
+ * @throws IOException
+ */
+ private OzoneFileStatusIterator(Path p) throws IOException {
+ this.p = p;
+ // fetch the first batch of entries in the directory
+ thisListing = listFileStatus(p, startPath);
+ if (thisListing != null && !thisListing.isEmpty()) {
+ startPath = pathToKey(
+ thisListing.get(thisListing.size() - 1).getPath());
+ LOG.info("Got {} file status, next start path {}",
Review Comment:
Can you change log level to debug or trace?
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -966,6 +966,135 @@ public RemoteIterator<LocatedFileStatus> listLocatedStatus(Path f)
return super.listLocatedStatus(f);
}
+ @Override
+ public RemoteIterator<FileStatus> listStatusIterator(Path f)
+ throws IOException {
+ return new OzoneFileStatusIterator<>(f);
+ }
+
+ /**
+ * A private class implementation for iterating list of file status.
+ *
+ * @param <T> the type of the file status.
+ */
+ private final class OzoneFileStatusIterator<T extends FileStatus>
+ implements RemoteIterator<T> {
+ private List<FileStatus> thisListing;
+ private int i;
+ private Path p;
+ private T curStat = null;
+ private String startPath = "";
+
+ /**
+ * Constructor to initialize OzoneFileStatusIterator.
+ * Get the first batch of entry for iteration.
+ *
+ * @param p path to file/directory.
+ * @throws IOException
+ */
+ private OzoneFileStatusIterator(Path p) throws IOException {
+ this.p = p;
+ // fetch the first batch of entries in the directory
+ thisListing = listFileStatus(p, startPath);
+ if (thisListing != null && !thisListing.isEmpty()) {
+ startPath = pathToKey(
+ thisListing.get(thisListing.size() - 1).getPath());
+ LOG.info("Got {} file status, next start path {}",
+ thisListing.size(), startPath);
+ }
+ statistics.incrementReadOps(1);
Review Comment:
Don't need to update the statistics since it's already done in listFileStatus call. Please double check other places too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] navinko commented on pull request #3711: HDDS-7075 Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
navinko commented on PR #3711:
URL: https://github.com/apache/ozone/pull/3711#issuecomment-1224049002
Hi Team ,
This is my first PR for ozone , kindly review .
Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] jojochuang commented on pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
jojochuang commented on PR #3711:
URL: https://github.com/apache/ozone/pull/3711#issuecomment-1224400768
My suggestion is to take a look at the HDFS implementation: https://github.com/apache/hadoop/blob/4d935eaed722061070138490a3147b5f0e3b5ae2/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java#L1256
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] navinko commented on a diff in pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
navinko commented on code in PR #3711:
URL: https://github.com/apache/ozone/pull/3711#discussion_r965075211
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java:
##########
@@ -864,6 +900,107 @@ public RemoteIterator<LocatedFileStatus> listLocatedStatus(Path f)
return super.listLocatedStatus(f);
}
+ @Override
+ public RemoteIterator<FileStatus> listStatusIterator(Path f)
+ throws IOException {
+ return new OzoneFileStatusIterator<>(f);
+ }
+
+ /**
+ * A private class implementation for iterating list of file status.
+ *
+ * @param <T> the type of the file status
+ */
+ private final class OzoneFileStatusIterator<T extends FileStatus>
Review Comment:
addressed
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java:
##########
@@ -635,6 +635,42 @@ workingDir, getUsername())
return statuses.toArray(new FileStatus[0]);
}
+ /**
+ * Get all the file status for input path and startPath.
+ *
+ * @param f
+ * @param startPath
+ * @return
+ * @throws IOException
+ */
+ public FileStatusListing listFileStatus(Path f, String startPath)
Review Comment:
addressed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] navinko commented on pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
navinko commented on PR #3711:
URL: https://github.com/apache/ozone/pull/3711#issuecomment-1225223453
Thanks @adoroszlai , @jojochuang for reviewing . I will check the HDFS implementation .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ChenSammi commented on a diff in pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3711:
URL: https://github.com/apache/ozone/pull/3711#discussion_r971783397
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java:
##########
@@ -387,6 +388,184 @@ public void testListStatus() throws Exception {
fs.delete(parent, true);
}
+ @Test
+ public void testListStatusIteratorWithDir() throws Exception {
+ Path parent = new Path(bucketPath, "testListStatus");
+ Path file1 = new Path(parent, "key1");
+ Path file2 = new Path(parent, "key2");
+
+ // Iterator should have no items when dir is empty
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ Assert.assertFalse(it.hasNext());
+ }
+ ContractTestUtils.touch(fs, file1);
+ ContractTestUtils.touch(fs, file2);
+ // Iterator should have an item when dir is not empty
+ it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertEquals("Parent path doesn't match",
+ fileStatus.getPath().toUri().getPath(), parent.toString());
+ }
+ // Iterator on a directory should return all subdirs along with
+ // files, even if there exists a file and sub-dir with the same name.
+ it = ofs.listStatusIterator(parent);
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals(
+ "Iterator did not return all the file status",
+ 2, iCount);
+ // Iterator should return file status for only the
+ // immediate children of a directory.
+ Path file3 = new Path(parent, "dir1/key3");
+ Path file4 = new Path(parent, "dir1/key4");
+ ContractTestUtils.touch(fs, file3);
+ ContractTestUtils.touch(fs, file4);
+ it = ofs.listStatusIterator(parent);
+ iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals("Iterator did not return file status " +
+ "of all the children of the directory", 3, iCount);
+
+ // Cleanup
+ fs.delete(parent, true);
+ }
+
+ /**
+ * Test listStatusIterator operation in a bucket.
+ */
+ @Test
+ public void testListStatusIteratorInBucket() throws Exception {
+ Path root = new Path("/" + volumeName + "/" + bucketName);
+ Path dir1 = new Path(root, "dir1");
+ Path dir12 = new Path(dir1, "dir12");
+ Path dir2 = new Path(root, "dir2");
+ try {
+ fs.mkdirs(dir12);
+ fs.mkdirs(dir2);
+
+ // ListStatus on root should return dir1 (even though /dir1 key does not
+ // exist) and dir2 only. dir12 is not an immediate child of root and
+ // hence should not be listed.
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(root);
+ // Verify that dir12 is not included in the result of the listStatus on
+ // root
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertNotEquals(fileStatus, dir12.toString());
+ }
+ Assert.assertEquals(
+ "FileStatus should return only the immediate children",
+ 2, iCount);
+
+ } finally {
+ // cleanup
+ fs.delete(dir1, true);
+ fs.delete(dir2, true);
+ }
+ }
+
+ /**
+ * Tests listStatusIterator operation on root directory.
+ */
+ @Test
+ public void testListStatusIteratorOnLargeDirectory() throws Exception {
+ Path root = new Path("/" + volumeName + "/" + bucketName);
+ Set<String> paths = new TreeSet<>();
+ int numDirs = LISTING_PAGE_SIZE + LISTING_PAGE_SIZE;
Review Comment:
@navinko , please add all the suggested numDirs here, thanks.
1,
LISTING_PAGE_SIZE,
LISTING_PAGE_SIZE + 1,
LISTING_PAGE_SIZE -1,
LISTING_PAGE_SIZE + LISTING_PAGE_SIZE/2
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] navinko commented on a diff in pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
navinko commented on code in PR #3711:
URL: https://github.com/apache/ozone/pull/3711#discussion_r965074843
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -966,6 +1002,107 @@ public RemoteIterator<LocatedFileStatus> listLocatedStatus(Path f)
return super.listLocatedStatus(f);
}
+ @Override
+ public RemoteIterator<FileStatus> listStatusIterator(Path f)
+ throws IOException {
Review Comment:
addressed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] navinko commented on a diff in pull request #3711: HDDS-7075. Implement FileSystem listStatusIterator to support file list paging
Posted by GitBox <gi...@apache.org>.
navinko commented on code in PR #3711:
URL: https://github.com/apache/ozone/pull/3711#discussion_r965075779
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java:
##########
@@ -387,6 +388,184 @@ public void testListStatus() throws Exception {
fs.delete(parent, true);
}
+ @Test
+ public void testListStatusIteratorWithDir() throws Exception {
+ Path parent = new Path(bucketPath, "testListStatus");
+ Path file1 = new Path(parent, "key1");
+ Path file2 = new Path(parent, "key2");
+
+ // Iterator should have no items when dir is empty
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ Assert.assertFalse(it.hasNext());
+ }
+ ContractTestUtils.touch(fs, file1);
+ ContractTestUtils.touch(fs, file2);
+ // Iterator should have an item when dir is not empty
+ it = ofs.listStatusIterator(bucketPath);
+ while (it.hasNext()) {
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertEquals("Parent path doesn't match",
+ fileStatus.getPath().toUri().getPath(), parent.toString());
+ }
+ // Iterator on a directory should return all subdirs along with
+ // files, even if there exists a file and sub-dir with the same name.
+ it = ofs.listStatusIterator(parent);
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals(
+ "Iterator did not return all the file status",
+ 2, iCount);
+ // Iterator should return file status for only the
+ // immediate children of a directory.
+ Path file3 = new Path(parent, "dir1/key3");
+ Path file4 = new Path(parent, "dir1/key4");
+ ContractTestUtils.touch(fs, file3);
+ ContractTestUtils.touch(fs, file4);
+ it = ofs.listStatusIterator(parent);
+ iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ }
+ Assert.assertEquals("Iterator did not return file status " +
+ "of all the children of the directory", 3, iCount);
+
+ // Cleanup
+ fs.delete(parent, true);
+ }
+
+ /**
+ * Test listStatusIterator operation in a bucket.
+ */
+ @Test
+ public void testListStatusIteratorInBucket() throws Exception {
+ Path root = new Path("/" + volumeName + "/" + bucketName);
+ Path dir1 = new Path(root, "dir1");
+ Path dir12 = new Path(dir1, "dir12");
+ Path dir2 = new Path(root, "dir2");
+ try {
+ fs.mkdirs(dir12);
+ fs.mkdirs(dir2);
+
+ // ListStatus on root should return dir1 (even though /dir1 key does not
+ // exist) and dir2 only. dir12 is not an immediate child of root and
+ // hence should not be listed.
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(root);
+ // Verify that dir12 is not included in the result of the listStatus on
+ // root
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertNotEquals(fileStatus, dir12.toString());
+ }
+ Assert.assertEquals(
+ "FileStatus should return only the immediate children",
+ 2, iCount);
+
+ } finally {
+ // cleanup
+ fs.delete(dir1, true);
+ fs.delete(dir2, true);
+ }
+ }
+
+ /**
+ * Tests listStatusIterator operation on root directory.
+ */
+ @Test
+ public void testListStatusIteratorOnLargeDirectory() throws Exception {
+ Path root = new Path("/" + volumeName + "/" + bucketName);
+ Set<String> paths = new TreeSet<>();
+ int numDirs = LISTING_PAGE_SIZE + LISTING_PAGE_SIZE;
+ try {
+ for (int i = 0; i < numDirs; i++) {
+ Path p = new Path(root, String.valueOf(i));
+ fs.mkdirs(p);
+ paths.add(p.getName());
+ }
+
+ RemoteIterator<FileStatus> iterator = ofs.listStatusIterator(root);
+ int iCount = 0;
+ if (iterator != null) {
+ while (iterator.hasNext()) {
+ FileStatus fileStatus = iterator.next();
+ iCount++;
+ LOG.info("path:{} in FileStatus " + fileStatus.getPath());
+ Assert.assertTrue(paths.contains(fileStatus.getPath().getName()));
+ }
+ }
+ Assert.assertEquals(
+ "Total directories listed do not match the existing directories",
+ numDirs, iCount);
+
+ } finally {
+ // Cleanup
+ for (int i = 0; i < numDirs; i++) {
+ Path p = new Path(root, String.valueOf(i));
+ fs.delete(p, true);
+ }
+ }
+ }
+
+ /**
+ * Tests listStatusIterator on a path with subdirs.
+ */
+ @Test
+ public void testListStatusIteratorOnSubDirs() throws Exception {
+ // Create the following key structure
+ // /dir1/dir11/dir111
+ // /dir1/dir12
+ // /dir1/dir12/file121
+ // /dir2
+ // listStatusIterator on /dir1 should return all its immediated subdirs only
+ // which are /dir1/dir11 and /dir1/dir12. Super child files/dirs
+ // (/dir1/dir12/file121 and /dir1/dir11/dir111) should not be returned by
+ // listStatusIterator.
+ Path dir1 = new Path(bucketPath, "dir1");
+ Path dir11 = new Path(dir1, "dir11");
+ Path dir111 = new Path(dir11, "dir111");
+ Path dir12 = new Path(dir1, "dir12");
+ Path file121 = new Path(dir12, "file121");
+ Path dir2 = new Path(bucketPath, "dir2");
+ fs.mkdirs(dir111);
+ fs.mkdirs(dir12);
+ ContractTestUtils.touch(fs, file121);
+ fs.mkdirs(dir2);
+
+ RemoteIterator<FileStatus> it = ofs.listStatusIterator(dir1);
+ int iCount = 0;
+ while (it.hasNext()) {
+ iCount++;
+ FileStatus fileStatus = it.next();
+ Assert.assertNotNull(fileStatus);
+ Assert.assertNotEquals(fileStatus, dir12.toString());
+ // Verify that the two children of /dir1
+ // returned by listStatusIterator operation
+ // are /dir1/dir11 and /dir1/dir12.
+ Assert.assertTrue(
+ fileStatus.getPath().toUri().getPath().
+ equals(dir11.toString()) ||
+ fileStatus.getPath().toUri().getPath().
+ equals(dir12.toString()));
+ }
+ Assert.assertEquals(
+ "Iterator should return only the immediate children",
+ 2, iCount);
+
+ // Cleanup
+ fs.delete(dir2, true);
Review Comment:
corrected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org