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 2020/06/17 21:28:11 UTC
[GitHub] [hadoop-ozone] smengcl opened a new pull request #1089: [WIP] HDDS-3705. [OFS] Implement getTrashRoots for trash cleanup
smengcl opened a new pull request #1089:
URL: https://github.com/apache/hadoop-ozone/pull/1089
## What changes were proposed in this pull request?
Implement getTrashRoots for trash cleanup
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-3705
## How was this patch tested?
Might write a new unit test or two later.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1089: HDDS-3705. [OFS] Implement getTrashRoots for trash cleanup
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1089:
URL: https://github.com/apache/hadoop-ozone/pull/1089#discussion_r451112711
##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -516,6 +520,62 @@ public FileStatusAdapter getFileStatus(String path, URI uri,
}
}
+ /**
+ * Get trash roots for current user or all users.
+ *
+ * Note:
+ * 1. When allUsers flag is false, this only returns the trash roots for
+ * those that the current user has access to.
+ * 2. Also it is not particularly efficient to use this API when there are
+ * a lot of volumes and buckets as the client has to iterate through all
+ * buckets in all volumes.
+ *
+ * @param allUsers return trashRoots of all users if true, used by emptier
+ * @param fs Pointer to the current OFS FileSystem
+ * @return
+ */
+ public Collection<FileStatus> getTrashRoots(boolean allUsers,
+ BasicRootedOzoneFileSystem fs) {
+ List<FileStatus> ret = new ArrayList<>();
+ try {
+ Iterator<? extends OzoneVolume> iterVol;
+ String username = UserGroupInformation.getCurrentUser().getUserName();
+ if (allUsers) {
+ iterVol = objectStore.listVolumes("");
+ } else {
+ iterVol = objectStore.listVolumesByUser(username, "", "");
+ }
+ while (iterVol.hasNext()) {
+ OzoneVolume volume = iterVol.next();
+ Path volumePath = new Path(OZONE_URI_DELIMITER, volume.getName());
+ Iterator<? extends OzoneBucket> bucketIter = volume.listBuckets("");
+ while (bucketIter.hasNext()) {
+ OzoneBucket bucket = bucketIter.next();
+ Path bucketPath = new Path(volumePath, bucket.getName());
+ Path trashRoot = new Path(bucketPath, FileSystem.TRASH_PREFIX);
+ if (allUsers) {
+ if (fs.exists(trashRoot)) {
+ for (FileStatus candidate : fs.listStatus(trashRoot)) {
+ if (fs.exists(candidate.getPath()) && candidate.isDirectory()) {
+ ret.add(candidate);
+ }
+ }
+ }
+ } else {
+ Path userTrash = new Path(trashRoot, username);
+ if (fs.exists(userTrash) &&
+ fs.getFileStatus(userTrash).isDirectory()) {
+ ret.add(fs.getFileStatus(userTrash));
+ }
+ }
+ }
+ }
+ } catch (IOException ex) {
+ throw new RuntimeException(ex);
Review comment:
I don't think we should throw RuntimeException here. We can log a warning and return an empty collection instead.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on pull request #1089: [WIP] HDDS-3705. [OFS] Implement getTrashRoots for trash cleanup
Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #1089:
URL: https://github.com/apache/hadoop-ozone/pull/1089#issuecomment-651985170
I'm rebasing the branch since the last update is a while ago. Also there were some flaky ozone-recon robot test in the [first run](https://github.com/apache/hadoop-ozone/runs/782209210).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #1089: HDDS-3705. [OFS] Implement getTrashRoots for trash cleanup
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #1089:
URL: https://github.com/apache/hadoop-ozone/pull/1089#discussion_r451113467
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
##########
@@ -871,4 +876,103 @@ public void testFailToDeleteRoot() throws IOException {
Assert.assertFalse(fs.delete(new Path("/"), true));
}
+ /**
+ * Test getTrashRoots() in OFS. Different from the existing test for o3fs.
+ */
+ @Test
+ public void testGetTrashRoots() throws IOException {
+ String username = UserGroupInformation.getCurrentUser().getShortUserName();
+ OzoneVolume volume1 = objectStore.getVolume(volumeName);
+ String prevOwner = volume1.getOwner();
+ // Set owner of the volume to current user, so it will show up in vol list
+ Assert.assertTrue(volume1.setOwner(username));
+
+ Path trashRoot1 = new Path(bucketPath, TRASH_PREFIX);
+ Path user1Trash1 = new Path(trashRoot1, username);
+ // When user trash dir isn't been created
+ Assert.assertEquals(0, fs.getTrashRoots(false).size());
+ Assert.assertEquals(0, fs.getTrashRoots(true).size());
+ // Let's create our first user1 (current user) trash dir.
+ fs.mkdirs(user1Trash1);
+ // Results should be getTrashRoots(false)=1, gTR(true)=1
+ Collection<FileStatus> res = fs.getTrashRoots(false);
+ Assert.assertEquals(1, res.size());
+ res.forEach(e -> Assert.assertEquals(
Review comment:
done d997ac0fb09b0145824ae8196e6b9bb47956c086
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] codecov-commenter commented on pull request #1089: HDDS-3705. [OFS] Implement getTrashRoots for trash cleanup
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1089:
URL: https://github.com/apache/hadoop-ozone/pull/1089#issuecomment-655086823
# [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1089?src=pr&el=h1) Report
> Merging [#1089](https://codecov.io/gh/apache/hadoop-ozone/pull/1089?src=pr&el=desc) into [master](https://codecov.io/gh/apache/hadoop-ozone/commit/1d13b4fb18d5ceb830380f09b62fa1740c96b5f5&el=desc) will **increase** coverage by `0.11%`.
> The diff coverage is `89.65%`.
[](https://codecov.io/gh/apache/hadoop-ozone/pull/1089?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1089 +/- ##
============================================
+ Coverage 73.34% 73.46% +0.11%
- Complexity 9962 10035 +73
============================================
Files 969 974 +5
Lines 49470 49714 +244
Branches 4859 4892 +33
============================================
+ Hits 36285 36520 +235
+ Misses 10869 10863 -6
- Partials 2316 2331 +15
```
| [Impacted Files](https://codecov.io/gh/apache/hadoop-ozone/pull/1089?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...va/org/apache/hadoop/ozone/client/ObjectStore.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1089/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL2NsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL296b25lL2NsaWVudC9PYmplY3RTdG9yZS5qYXZh) | `89.85% <ø> (ø)` | `22.00 <0.00> (ø)` | |
| [...op/fs/ozone/BasicRootedOzoneClientAdapterImpl.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1089/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL296b25lZnMtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvZnMvb3pvbmUvQmFzaWNSb290ZWRPem9uZUNsaWVudEFkYXB0ZXJJbXBsLmphdmE=) | `70.12% <89.28%> (+1.50%)` | `58.00 <10.00> (+10.00)` | |
| [...he/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1089/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL296b25lZnMtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvZnMvb3pvbmUvQmFzaWNSb290ZWRPem9uZUZpbGVTeXN0ZW0uamF2YQ==) | `74.48% <100.00%> (+0.07%)` | `51.00 <1.00> (+1.00)` | |
| [...iner/ozoneimpl/ContainerScrubberConfiguration.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1089/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvb3pvbmVpbXBsL0NvbnRhaW5lclNjcnViYmVyQ29uZmlndXJhdGlvbi5qYXZh) | `81.81% <0.00%> (-18.19%)` | `7.00% <0.00%> (-1.00%)` | |
| [...rg/apache/hadoop/hdds/scm/pipeline/PipelineID.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1089/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy9zY20vcGlwZWxpbmUvUGlwZWxpbmVJRC5qYXZh) | `88.88% <0.00%> (-5.23%)` | `12.00% <0.00%> (+1.00%)` | :arrow_down: |
| [...ent/algorithms/SCMContainerPlacementRackAware.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1089/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvc2VydmVyLXNjbS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL2hkZHMvc2NtL2NvbnRhaW5lci9wbGFjZW1lbnQvYWxnb3JpdGhtcy9TQ01Db250YWluZXJQbGFjZW1lbnRSYWNrQXdhcmUuamF2YQ==) | `76.69% <0.00%> (-3.01%)` | `31.00% <0.00%> (-2.00%)` | |
| [...rg/apache/hadoop/hdds/conf/OzoneConfiguration.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1089/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy9jb25mL096b25lQ29uZmlndXJhdGlvbi5qYXZh) | `69.14% <0.00%> (-1.64%)` | `17.00% <0.00%> (+1.00%)` | :arrow_down: |
| [...p/ozone/container/keyvalue/helpers/ChunkUtils.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1089/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIva2V5dmFsdWUvaGVscGVycy9DaHVua1V0aWxzLmphdmE=) | `85.45% <0.00%> (-0.91%)` | `30.00% <0.00%> (-1.00%)` | |
| [.../org/apache/hadoop/hdds/scm/pipeline/Pipeline.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1089/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy9zY20vcGlwZWxpbmUvUGlwZWxpbmUuamF2YQ==) | `86.30% <0.00%> (-0.85%)` | `48.00% <0.00%> (+2.00%)` | :arrow_down: |
| [...hadoop/ozone/om/ratis/OzoneManagerRatisServer.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1089/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL296b25lLW1hbmFnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9vbS9yYXRpcy9Pem9uZU1hbmFnZXJSYXRpc1NlcnZlci5qYXZh) | `79.29% <0.00%> (-0.79%)` | `35.00% <0.00%> (-1.00%)` | |
| ... and [48 more](https://codecov.io/gh/apache/hadoop-ozone/pull/1089/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1089?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1089?src=pr&el=footer). Last update [1d13b4f...e163f2c](https://codecov.io/gh/apache/hadoop-ozone/pull/1089?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #1089: HDDS-3705. [OFS] Implement getTrashRoots for trash cleanup
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #1089:
URL: https://github.com/apache/hadoop-ozone/pull/1089#discussion_r451129142
##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -516,6 +520,62 @@ public FileStatusAdapter getFileStatus(String path, URI uri,
}
}
+ /**
+ * Get trash roots for current user or all users.
+ *
+ * Note:
+ * 1. When allUsers flag is false, this only returns the trash roots for
+ * those that the current user has access to.
+ * 2. Also it is not particularly efficient to use this API when there are
+ * a lot of volumes and buckets as the client has to iterate through all
+ * buckets in all volumes.
+ *
+ * @param allUsers return trashRoots of all users if true, used by emptier
+ * @param fs Pointer to the current OFS FileSystem
+ * @return
+ */
+ public Collection<FileStatus> getTrashRoots(boolean allUsers,
+ BasicRootedOzoneFileSystem fs) {
+ List<FileStatus> ret = new ArrayList<>();
+ try {
+ Iterator<? extends OzoneVolume> iterVol;
+ String username = UserGroupInformation.getCurrentUser().getUserName();
+ if (allUsers) {
+ iterVol = objectStore.listVolumes("");
+ } else {
+ iterVol = objectStore.listVolumesByUser(username, "", "");
+ }
+ while (iterVol.hasNext()) {
+ OzoneVolume volume = iterVol.next();
+ Path volumePath = new Path(OZONE_URI_DELIMITER, volume.getName());
+ Iterator<? extends OzoneBucket> bucketIter = volume.listBuckets("");
+ while (bucketIter.hasNext()) {
+ OzoneBucket bucket = bucketIter.next();
+ Path bucketPath = new Path(volumePath, bucket.getName());
+ Path trashRoot = new Path(bucketPath, FileSystem.TRASH_PREFIX);
+ if (allUsers) {
+ if (fs.exists(trashRoot)) {
+ for (FileStatus candidate : fs.listStatus(trashRoot)) {
+ if (fs.exists(candidate.getPath()) && candidate.isDirectory()) {
+ ret.add(candidate);
+ }
+ }
+ }
+ } else {
+ Path userTrash = new Path(trashRoot, username);
+ if (fs.exists(userTrash) &&
+ fs.getFileStatus(userTrash).isDirectory()) {
+ ret.add(fs.getFileStatus(userTrash));
+ }
+ }
+ }
+ }
+ } catch (IOException ex) {
+ throw new RuntimeException(ex);
Review comment:
done 1940f99f524ef589b2c5a1613031f3cb7b1066b4
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1089: HDDS-3705. [OFS] Implement getTrashRoots for trash cleanup
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1089:
URL: https://github.com/apache/hadoop-ozone/pull/1089#discussion_r451102321
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
##########
@@ -871,4 +876,103 @@ public void testFailToDeleteRoot() throws IOException {
Assert.assertFalse(fs.delete(new Path("/"), true));
}
+ /**
+ * Test getTrashRoots() in OFS. Different from the existing test for o3fs.
+ */
+ @Test
+ public void testGetTrashRoots() throws IOException {
+ String username = UserGroupInformation.getCurrentUser().getShortUserName();
+ OzoneVolume volume1 = objectStore.getVolume(volumeName);
+ String prevOwner = volume1.getOwner();
+ // Set owner of the volume to current user, so it will show up in vol list
+ Assert.assertTrue(volume1.setOwner(username));
+
+ Path trashRoot1 = new Path(bucketPath, TRASH_PREFIX);
+ Path user1Trash1 = new Path(trashRoot1, username);
+ // When user trash dir isn't been created
+ Assert.assertEquals(0, fs.getTrashRoots(false).size());
+ Assert.assertEquals(0, fs.getTrashRoots(true).size());
+ // Let's create our first user1 (current user) trash dir.
+ fs.mkdirs(user1Trash1);
+ // Results should be getTrashRoots(false)=1, gTR(true)=1
+ Collection<FileStatus> res = fs.getTrashRoots(false);
+ Assert.assertEquals(1, res.size());
+ res.forEach(e -> Assert.assertEquals(
Review comment:
NIT: forEach is unnecessary as we have assert the size is 1.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl merged pull request #1089: HDDS-3705. [OFS] Implement getTrashRoots for trash cleanup
Posted by GitBox <gi...@apache.org>.
smengcl merged pull request #1089:
URL: https://github.com/apache/hadoop-ozone/pull/1089
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on pull request #1089: HDDS-3705. [OFS] Implement getTrashRoots for trash cleanup
Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #1089:
URL: https://github.com/apache/hadoop-ozone/pull/1089#issuecomment-654981393
The new test contaminated the results of other tests in the same test class. Fixing this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #1089: HDDS-3705. [OFS] Implement getTrashRoots for trash cleanup
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #1089:
URL: https://github.com/apache/hadoop-ozone/pull/1089#discussion_r451126852
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
##########
@@ -871,4 +876,103 @@ public void testFailToDeleteRoot() throws IOException {
Assert.assertFalse(fs.delete(new Path("/"), true));
}
+ /**
+ * Test getTrashRoots() in OFS. Different from the existing test for o3fs.
+ */
+ @Test
+ public void testGetTrashRoots() throws IOException {
+ String username = UserGroupInformation.getCurrentUser().getShortUserName();
+ OzoneVolume volume1 = objectStore.getVolume(volumeName);
+ String prevOwner = volume1.getOwner();
+ // Set owner of the volume to current user, so it will show up in vol list
+ Assert.assertTrue(volume1.setOwner(username));
+
+ Path trashRoot1 = new Path(bucketPath, TRASH_PREFIX);
+ Path user1Trash1 = new Path(trashRoot1, username);
+ // When user trash dir isn't been created
+ Assert.assertEquals(0, fs.getTrashRoots(false).size());
+ Assert.assertEquals(0, fs.getTrashRoots(true).size());
+ // Let's create our first user1 (current user) trash dir.
+ fs.mkdirs(user1Trash1);
+ // Results should be getTrashRoots(false)=1, gTR(true)=1
+ Collection<FileStatus> res = fs.getTrashRoots(false);
+ Assert.assertEquals(1, res.size());
+ res.forEach(e -> Assert.assertEquals(
+ user1Trash1.toString(), e.getPath().toUri().getPath()));
+ res = fs.getTrashRoots(true);
+ Assert.assertEquals(1, res.size());
+ res.forEach(e -> Assert.assertEquals(
+ user1Trash1.toString(), e.getPath().toUri().getPath()));
+
+ // Create one more trash for user2 in the same bucket
+ Path user2Trash1 = new Path(trashRoot1, "testuser2");
+ fs.mkdirs(user2Trash1);
+ // Results should be getTrashRoots(false)=1, gTR(true)=2
+ Assert.assertEquals(1, fs.getTrashRoots(false).size());
Review comment:
sure thing. done in ebf104b3d6b0c6531ce2bec33fe00c996fcb16cf
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on pull request #1089: HDDS-3705. [OFS] Implement getTrashRoots for trash cleanup
Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #1089:
URL: https://github.com/apache/hadoop-ozone/pull/1089#issuecomment-655205691
Thanks @xiaoyuyao for the review. Will merge shortly.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1089: HDDS-3705. [OFS] Implement getTrashRoots for trash cleanup
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1089:
URL: https://github.com/apache/hadoop-ozone/pull/1089#discussion_r451103682
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
##########
@@ -871,4 +876,103 @@ public void testFailToDeleteRoot() throws IOException {
Assert.assertFalse(fs.delete(new Path("/"), true));
}
+ /**
+ * Test getTrashRoots() in OFS. Different from the existing test for o3fs.
+ */
+ @Test
+ public void testGetTrashRoots() throws IOException {
+ String username = UserGroupInformation.getCurrentUser().getShortUserName();
+ OzoneVolume volume1 = objectStore.getVolume(volumeName);
+ String prevOwner = volume1.getOwner();
+ // Set owner of the volume to current user, so it will show up in vol list
+ Assert.assertTrue(volume1.setOwner(username));
+
+ Path trashRoot1 = new Path(bucketPath, TRASH_PREFIX);
+ Path user1Trash1 = new Path(trashRoot1, username);
+ // When user trash dir isn't been created
+ Assert.assertEquals(0, fs.getTrashRoots(false).size());
+ Assert.assertEquals(0, fs.getTrashRoots(true).size());
+ // Let's create our first user1 (current user) trash dir.
+ fs.mkdirs(user1Trash1);
+ // Results should be getTrashRoots(false)=1, gTR(true)=1
+ Collection<FileStatus> res = fs.getTrashRoots(false);
+ Assert.assertEquals(1, res.size());
+ res.forEach(e -> Assert.assertEquals(
+ user1Trash1.toString(), e.getPath().toUri().getPath()));
+ res = fs.getTrashRoots(true);
+ Assert.assertEquals(1, res.size());
+ res.forEach(e -> Assert.assertEquals(
+ user1Trash1.toString(), e.getPath().toUri().getPath()));
+
+ // Create one more trash for user2 in the same bucket
+ Path user2Trash1 = new Path(trashRoot1, "testuser2");
+ fs.mkdirs(user2Trash1);
+ // Results should be getTrashRoots(false)=1, gTR(true)=2
+ Assert.assertEquals(1, fs.getTrashRoots(false).size());
+ Assert.assertEquals(2, fs.getTrashRoots(true).size());
+
+ // Create a new bucket in the same volume
+ final String bucketName2 = "trashroottest2";
+ volume1.createBucket(bucketName2);
+ Path bucketPath2 = new Path(volumePath, bucketName2);
+ Path trashRoot2 = new Path(bucketPath2, TRASH_PREFIX);
+ Path user1Trash2 = new Path(trashRoot2, username);
+ // Create a file at the trash location, it shouldn't be recognized as trash
+ try (FSDataOutputStream out1 = fs.create(user1Trash2)) {
+ out1.write(123);
+ }
+ // Results should still be getTrashRoots(false)=1, gTR(true)=2
+ Assert.assertEquals(1, fs.getTrashRoots(false).size());
+ res.forEach(e -> Assert.assertEquals(
Review comment:
I see you have the assertion here for user1Trash1. So you can ignore the previous comment.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1089: HDDS-3705. [OFS] Implement getTrashRoots for trash cleanup
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1089:
URL: https://github.com/apache/hadoop-ozone/pull/1089#discussion_r451103207
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
##########
@@ -871,4 +876,103 @@ public void testFailToDeleteRoot() throws IOException {
Assert.assertFalse(fs.delete(new Path("/"), true));
}
+ /**
+ * Test getTrashRoots() in OFS. Different from the existing test for o3fs.
+ */
+ @Test
+ public void testGetTrashRoots() throws IOException {
+ String username = UserGroupInformation.getCurrentUser().getShortUserName();
+ OzoneVolume volume1 = objectStore.getVolume(volumeName);
+ String prevOwner = volume1.getOwner();
+ // Set owner of the volume to current user, so it will show up in vol list
+ Assert.assertTrue(volume1.setOwner(username));
+
+ Path trashRoot1 = new Path(bucketPath, TRASH_PREFIX);
+ Path user1Trash1 = new Path(trashRoot1, username);
+ // When user trash dir isn't been created
+ Assert.assertEquals(0, fs.getTrashRoots(false).size());
+ Assert.assertEquals(0, fs.getTrashRoots(true).size());
+ // Let's create our first user1 (current user) trash dir.
+ fs.mkdirs(user1Trash1);
+ // Results should be getTrashRoots(false)=1, gTR(true)=1
+ Collection<FileStatus> res = fs.getTrashRoots(false);
+ Assert.assertEquals(1, res.size());
+ res.forEach(e -> Assert.assertEquals(
+ user1Trash1.toString(), e.getPath().toUri().getPath()));
+ res = fs.getTrashRoots(true);
+ Assert.assertEquals(1, res.size());
+ res.forEach(e -> Assert.assertEquals(
+ user1Trash1.toString(), e.getPath().toUri().getPath()));
+
+ // Create one more trash for user2 in the same bucket
+ Path user2Trash1 = new Path(trashRoot1, "testuser2");
+ fs.mkdirs(user2Trash1);
+ // Results should be getTrashRoots(false)=1, gTR(true)=2
+ Assert.assertEquals(1, fs.getTrashRoots(false).size());
Review comment:
Can we assert the trash roots is for user one with invoke getTrashRoots(false)?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on pull request #1089: HDDS-3705. [OFS] Implement getTrashRoots for trash cleanup
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on pull request #1089:
URL: https://github.com/apache/hadoop-ozone/pull/1089#issuecomment-655139139
LGTM, +1 pending CI.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on pull request #1089: HDDS-3705. [OFS] Implement getTrashRoots for trash cleanup
Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #1089:
URL: https://github.com/apache/hadoop-ozone/pull/1089#issuecomment-652241525
This PR is ready for 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org