You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "sumitagrawl (via GitHub)" <gi...@apache.org> on 2023/03/15 16:27:39 UTC
[GitHub] [ozone] sumitagrawl opened a new pull request, #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
sumitagrawl opened a new pull request, #4405:
URL: https://github.com/apache/ozone/pull/4405
## What changes were proposed in this pull request?
For link bucket as orphan, fix the failure of volume not found or bucket not found.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7884
## How was this patch tested?
Integration test added for the orphan link bucket deletion
--
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] neils-dev commented on pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on PR #4405:
URL: https://github.com/apache/ozone/pull/4405#issuecomment-1510712405
Thanks @sumitagrawl for reaching out. Let me take a look and also let me coordinate with @smengcl having this update and [PR #4246](https://github.com/apache/ozone/pull/4246) added as they will overlap a bit and conflict when we merge.
--
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] sumitagrawl commented on a diff in pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #4405:
URL: https://github.com/apache/ozone/pull/4405#discussion_r1155453485
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java:
##########
@@ -647,6 +647,68 @@ public void testDeleteToTrashOrSkipTrash() throws Exception {
}
}
+ @Test
Review Comment:
@neils-dev Cluster initialisation needs to be done with specific OM ID, and need some shell execute method to added again TestRootedOzoneFileSystem, and since this class is not doing shell related testing, so added in TestOzoneShellHA,
I think this is not required as way of testing the link bucket is different in both case.
--
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] sumitagrawl commented on a diff in pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #4405:
URL: https://github.com/apache/ozone/pull/4405#discussion_r1155463111
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/OzoneClientUtils.java:
##########
@@ -69,9 +71,24 @@ public static BucketLayout resolveLinkBucketLayout(OzoneBucket bucket,
DETECTED_LOOP_IN_BUCKET_LINKS);
}
- OzoneBucket sourceBucket =
- objectStore.getVolume(bucket.getSourceVolume())
- .getBucket(bucket.getSourceBucket());
+ OzoneBucket sourceBucket;
+ try {
+ sourceBucket =
+ objectStore.getVolume(bucket.getSourceVolume())
+ .getBucket(bucket.getSourceBucket());
+ } catch (OMException ex) {
+ if (ex.getResult().equals(VOLUME_NOT_FOUND)
+ || ex.getResult().equals(BUCKET_NOT_FOUND)) {
+ // for orphan link bucket, return layout as link bucket
+ bucket.setSourcePathExist(false);
+ LOG.error("Source Bucket is not found, its orphan bucket and " +
+ "used link bucket {} layout {}", bucket.getName(),
+ bucket.getBucketLayout());
+ return bucket.getBucketLayout();
Review Comment:
Its configured OM default layout "ozone.default.bucket.layout"
--
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] smengcl commented on pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #4405:
URL: https://github.com/apache/ozone/pull/4405#issuecomment-1486346652
@neils-dev Looks like [the test case failure](https://github.com/apache/ozone/pull/4405#issuecomment-1471145558) is also related to #4246 ?
--
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] neils-dev commented on a diff in pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on code in PR #4405:
URL: https://github.com/apache/ozone/pull/4405#discussion_r1155258333
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java:
##########
@@ -657,8 +657,9 @@ private FileStatusAdapter getFileStatusForKeyOrSnapshot(
OFSPath ofsPath, URI uri, Path qualifiedPath, String userName)
throws IOException {
String key = ofsPath.getKeyName();
+ OzoneBucket bucket = null;
Review Comment:
Nit: unnecessary change. Variable `bucket` only used in scope of try block and can remain declared there.
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -720,6 +670,101 @@ public boolean delete(Path f, boolean recursive) throws IOException {
return result;
}
+ private boolean deleteBucket(Path f, boolean recursive, OFSPath ofsPath)
+ throws IOException {
+ // check status of normal bucket
+ try {
+ getFileStatus(f);
+ } catch (FileNotFoundException ex) {
+ // remove orphan link bucket directly
+ if (isLinkBucket(f, ofsPath)) {
Review Comment:
Within this catch block we can just check `if isLinkBucket == false` and return `LOG.warn(Path does not exist)` with `false`. Outside the catch block the bucket is checked if link, if true, then the orphan bucket is removed from volume and returned.
--
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] neils-dev commented on a diff in pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on code in PR #4405:
URL: https://github.com/apache/ozone/pull/4405#discussion_r1169365864
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/OzoneClientUtils.java:
##########
@@ -69,9 +71,24 @@ public static BucketLayout resolveLinkBucketLayout(OzoneBucket bucket,
DETECTED_LOOP_IN_BUCKET_LINKS);
}
- OzoneBucket sourceBucket =
- objectStore.getVolume(bucket.getSourceVolume())
- .getBucket(bucket.getSourceBucket());
+ OzoneBucket sourceBucket;
+ try {
+ sourceBucket =
+ objectStore.getVolume(bucket.getSourceVolume())
+ .getBucket(bucket.getSourceBucket());
+ } catch (OMException ex) {
+ if (ex.getResult().equals(VOLUME_NOT_FOUND)
+ || ex.getResult().equals(BUCKET_NOT_FOUND)) {
+ // for orphan link bucket, return layout as link bucket
+ bucket.setSourcePathExist(false);
+ LOG.error("Source Bucket is not found, its orphan bucket and " +
+ "used link bucket {} layout {}", bucket.getName(),
+ bucket.getBucketLayout());
+ return bucket.getBucketLayout();
Review Comment:
Thanks. I ran it the `OzoneClientUtils.resolveLinkBucketLayout` with default, and user specified layouts on bucket create. No problems - the linked bucket object layout is set to the source bucket type.
--
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] neils-dev merged pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev merged PR #4405:
URL: https://github.com/apache/ozone/pull/4405
--
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] kerneltime commented on pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #4405:
URL: https://github.com/apache/ozone/pull/4405#issuecomment-1476531675
@SaketaChalamchala @tanvipenumudy can you please take a look?
--
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] neils-dev commented on pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on PR #4405:
URL: https://github.com/apache/ozone/pull/4405#issuecomment-1477169161
Hi @sumitagrawl , I saw a problem with the symlink delete problem with orphaned link while reviewing this PR. It occurs when the src volume/bucket for the link is deleted. When the src for the link is deleted it causes an orphaned bucket link. This in turn currently causes an error when the target link volume/bucket is recursively listed. This should be handled. It can be reproduced by,
_create src and target link._
`ozone sh bucket link /vol1/abc /s3v/abc`
_delete src volume/bucket_
`ozone fs -rm -R -skipTrash ofs://om/vol1`
_list recursively the target link:_
`ozone fs -ls -R ofs://om/s3v`
**drwxrwxrwx - hadoop hadoop 0 2023-03-20 19:20 ofs://localhost/s3v/abc
ls: Volume not found when getting bucket info**
--
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] neils-dev commented on a diff in pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on code in PR #4405:
URL: https://github.com/apache/ozone/pull/4405#discussion_r1169287301
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -720,6 +670,101 @@ public boolean delete(Path f, boolean recursive) throws IOException {
return result;
}
+ private boolean deleteBucket(Path f, boolean recursive, OFSPath ofsPath)
+ throws IOException {
+ // check status of normal bucket
+ try {
+ getFileStatus(f);
+ } catch (FileNotFoundException ex) {
+ // remove orphan link bucket directly
+ if (isLinkBucket(f, ofsPath)) {
Review Comment:
Thanks for pointing out the remote calls. Makes sense.
--
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] neils-dev commented on pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on PR #4405:
URL: https://github.com/apache/ozone/pull/4405#issuecomment-1471145558
Thanks @sumitagrawl for this PR and for your comments on the jira. Pls note, looks like an integration test is failing when testing the sym links in CI workflow:
`TestOzoneFileSystemWithLinks.testLoopInLinkBuckets:228 Should throw Exception due to loop in Link Buckets`
--
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] sumitagrawl commented on a diff in pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #4405:
URL: https://github.com/apache/ozone/pull/4405#discussion_r1154592681
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/OzoneClientUtils.java:
##########
@@ -69,9 +71,21 @@ public static BucketLayout resolveLinkBucketLayout(OzoneBucket bucket,
DETECTED_LOOP_IN_BUCKET_LINKS);
}
- OzoneBucket sourceBucket =
- objectStore.getVolume(bucket.getSourceVolume())
- .getBucket(bucket.getSourceBucket());
+ OzoneBucket sourceBucket;
+ try {
+ sourceBucket =
+ objectStore.getVolume(bucket.getSourceVolume())
+ .getBucket(bucket.getSourceBucket());
+ } catch (OMException ex) {
+ if (ex.getResult().equals(VOLUME_NOT_FOUND)
+ || ex.getResult().equals(BUCKET_NOT_FOUND)) {
+ // for orphan link bucket, return layout as link bucket
+ bucket.setSourcePathExist(false);
+ return bucket.getBucketLayout();
Review Comment:
@sadanand48 Updated...
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java:
##########
@@ -888,8 +889,14 @@ public List<FileStatusAdapter> listStatus(String pathStr, boolean recursive,
String startKey = ofsStartPath.getKeyName();
try {
OzoneBucket bucket = getBucket(ofsPath, false);
- List<OzoneFileStatus> statuses = bucket
- .listStatus(keyName, recursive, startKey, numEntries);
+ List<OzoneFileStatus> statuses;
+ if (bucket.isSourcePathExist()) {
+ statuses = bucket
+ .listStatus(keyName, recursive, startKey, numEntries);
+ } else {
+ statuses = Collections.emptyList();
Review Comment:
@sadanand48 Updated...
--
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] sumitagrawl commented on a diff in pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #4405:
URL: https://github.com/apache/ozone/pull/4405#discussion_r1154593059
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -720,6 +670,99 @@ public boolean delete(Path f, boolean recursive) throws IOException {
return result;
}
+ private boolean deleteBucket(Path f, boolean recursive, OFSPath ofsPath)
+ throws IOException {
+ // check status of normal bucket
+ try {
+ getFileStatus(f);
+ } catch (FileNotFoundException ex) {
+ // remove orpahan link bucket directly
+ if (isLinkBucket(f, ofsPath)) {
+ deleteBucketFromVolume(f, ofsPath);
+ return true;
+ }
+ LOG.warn("delete: Path does not exist: {}", f);
+ return false;
+ }
+
+ // remove link bucket directly
+ if (isLinkBucket(f, ofsPath)) {
+ deleteBucketFromVolume(f, ofsPath);
+ return true;
+ }
+
+ // delete inner content of bucket
+ boolean result = innerDelete(f, recursive);
+
+ // Handle delete bucket
+ deleteBucketFromVolume(f, ofsPath);
+ return result;
+ }
+
+ private boolean isLinkBucket(Path f, OFSPath ofsPath) {
+ try {
+ OzoneBucket bucket = adapterImpl.getBucket(ofsPath, false);
+ if (bucket.isLink()) {
+ return true;
+ }
+ } catch (Exception ex) {
+ return false;
Review Comment:
@sadanand48 Updated...
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -720,6 +670,99 @@ public boolean delete(Path f, boolean recursive) throws IOException {
return result;
}
+ private boolean deleteBucket(Path f, boolean recursive, OFSPath ofsPath)
+ throws IOException {
+ // check status of normal bucket
+ try {
+ getFileStatus(f);
+ } catch (FileNotFoundException ex) {
+ // remove orpahan link bucket directly
Review Comment:
Updated...
--
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] sumitagrawl commented on pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on PR #4405:
URL: https://github.com/apache/ozone/pull/4405#issuecomment-1521943569
> Thanks @sumitagrawl for reaching out. Let me take a look and also let me coordinate with @smengcl having this update and [PR #4246](https://github.com/apache/ozone/pull/4246) added as they will overlap a bit and conflict when we merge.
@neils-dev @smengcl Please check if the PR can be merged
--
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] neils-dev commented on a diff in pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on code in PR #4405:
URL: https://github.com/apache/ozone/pull/4405#discussion_r1155393125
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java:
##########
@@ -647,6 +647,68 @@ public void testDeleteToTrashOrSkipTrash() throws Exception {
}
}
+ @Test
Review Comment:
Can this test be placed in the integration test suite, `TestRootedOzoneFileSystem`? Grouped with symlink bucket tests as,
https://github.com/apache/ozone/blob/0c84ff1cdb5208e4932bcc6195b26fcaaeece22b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java#L1520
--
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] neils-dev commented on a diff in pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on code in PR #4405:
URL: https://github.com/apache/ozone/pull/4405#discussion_r1155397974
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/OzoneClientUtils.java:
##########
@@ -69,9 +71,24 @@ public static BucketLayout resolveLinkBucketLayout(OzoneBucket bucket,
DETECTED_LOOP_IN_BUCKET_LINKS);
}
- OzoneBucket sourceBucket =
- objectStore.getVolume(bucket.getSourceVolume())
- .getBucket(bucket.getSourceBucket());
+ OzoneBucket sourceBucket;
+ try {
+ sourceBucket =
+ objectStore.getVolume(bucket.getSourceVolume())
+ .getBucket(bucket.getSourceBucket());
+ } catch (OMException ex) {
+ if (ex.getResult().equals(VOLUME_NOT_FOUND)
+ || ex.getResult().equals(BUCKET_NOT_FOUND)) {
+ // for orphan link bucket, return layout as link bucket
+ bucket.setSourcePathExist(false);
+ LOG.error("Source Bucket is not found, its orphan bucket and " +
+ "used link bucket {} layout {}", bucket.getName(),
+ bucket.getBucketLayout());
+ return bucket.getBucketLayout();
Review Comment:
What is the layout of the linked bucket? Is it the layout of the original source bucket or is it defaulted to some value?
--
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] sumitagrawl commented on a diff in pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #4405:
URL: https://github.com/apache/ozone/pull/4405#discussion_r1154577227
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystemWithLinks.java:
##########
@@ -225,11 +225,9 @@ public void testLoopInLinkBuckets() throws Exception {
try {
FileSystem.get(conf);
- Assert.fail("Should throw Exception due to loop in Link Buckets");
Review Comment:
@sadanand48 Thanks for review, as part of this PR, delete and ls is fixed. rename is not allowed for bucket. other file/directory operation will fail as source bucket is not present, which is expected.
--
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] neils-dev commented on pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on PR #4405:
URL: https://github.com/apache/ozone/pull/4405#issuecomment-1487972718
> @neils-dev Looks like https://github.com/apache/ozone/pull/4405#issuecomment-1471145558 is also related to https://github.com/apache/ozone/pull/4246 ?
Thanks @smengcl . Actually, the failure is something that should be handled here. The other PR we have open, [#4246](https://github.com/apache/ozone/pull/4246) is for recursive deletes with symlinks where the src bucket _is_ intact, however in this case the src bucket is _removed_ leaving an orphan symlink. Failures occurring with operations on this orphan symlink are handled by Sumit's 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] sumitagrawl commented on a diff in pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #4405:
URL: https://github.com/apache/ozone/pull/4405#discussion_r1155450603
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -720,6 +670,101 @@ public boolean delete(Path f, boolean recursive) throws IOException {
return result;
}
+ private boolean deleteBucket(Path f, boolean recursive, OFSPath ofsPath)
+ throws IOException {
+ // check status of normal bucket
+ try {
+ getFileStatus(f);
+ } catch (FileNotFoundException ex) {
+ // remove orphan link bucket directly
+ if (isLinkBucket(f, ofsPath)) {
Review Comment:
@neils-dev
isLinkBucket performs remote call to OM. So checking first as false in exception, and again checking outside for link bucket bucket will have 2 remote call. So I think this is not required to be refactored for performance.
--
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] sadanand48 commented on a diff in pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "sadanand48 (via GitHub)" <gi...@apache.org>.
sadanand48 commented on code in PR #4405:
URL: https://github.com/apache/ozone/pull/4405#discussion_r1154067503
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -720,6 +670,99 @@ public boolean delete(Path f, boolean recursive) throws IOException {
return result;
}
+ private boolean deleteBucket(Path f, boolean recursive, OFSPath ofsPath)
+ throws IOException {
+ // check status of normal bucket
+ try {
+ getFileStatus(f);
+ } catch (FileNotFoundException ex) {
+ // remove orpahan link bucket directly
Review Comment:
typo: orphan
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java:
##########
@@ -888,8 +889,14 @@ public List<FileStatusAdapter> listStatus(String pathStr, boolean recursive,
String startKey = ofsStartPath.getKeyName();
try {
OzoneBucket bucket = getBucket(ofsPath, false);
- List<OzoneFileStatus> statuses = bucket
- .listStatus(keyName, recursive, startKey, numEntries);
+ List<OzoneFileStatus> statuses;
+ if (bucket.isSourcePathExist()) {
+ statuses = bucket
+ .listStatus(keyName, recursive, startKey, numEntries);
+ } else {
+ statuses = Collections.emptyList();
Review Comment:
better to add a log here to communicate to the user that source bucket doesn’t exist
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/OzoneClientUtils.java:
##########
@@ -69,9 +71,21 @@ public static BucketLayout resolveLinkBucketLayout(OzoneBucket bucket,
DETECTED_LOOP_IN_BUCKET_LINKS);
}
- OzoneBucket sourceBucket =
- objectStore.getVolume(bucket.getSourceVolume())
- .getBucket(bucket.getSourceBucket());
+ OzoneBucket sourceBucket;
+ try {
+ sourceBucket =
+ objectStore.getVolume(bucket.getSourceVolume())
+ .getBucket(bucket.getSourceBucket());
+ } catch (OMException ex) {
+ if (ex.getResult().equals(VOLUME_NOT_FOUND)
+ || ex.getResult().equals(BUCKET_NOT_FOUND)) {
+ // for orphan link bucket, return layout as link bucket
+ bucket.setSourcePathExist(false);
+ return bucket.getBucketLayout();
Review Comment:
Inform the user that we are returning the layout of link bucket as source bucket doesn't exist.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystemWithLinks.java:
##########
@@ -225,11 +225,9 @@ public void testLoopInLinkBuckets() throws Exception {
try {
FileSystem.get(conf);
- Assert.fail("Should throw Exception due to loop in Link Buckets");
Review Comment:
Before this change, filesystem wouldn't get initialised for orphan buckets so no operations could be performed for these buckets, Now that we are allowing orphan buckets to be accessed via FS, it would require to handle all operations like delete/rename/ls etc for such buckets , should we allow this or just restrict FS access? To delete the orphan bucket we could use the `ozone sh bucket delete `
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -720,6 +670,99 @@ public boolean delete(Path f, boolean recursive) throws IOException {
return result;
}
+ private boolean deleteBucket(Path f, boolean recursive, OFSPath ofsPath)
+ throws IOException {
+ // check status of normal bucket
+ try {
+ getFileStatus(f);
+ } catch (FileNotFoundException ex) {
+ // remove orpahan link bucket directly
+ if (isLinkBucket(f, ofsPath)) {
+ deleteBucketFromVolume(f, ofsPath);
+ return true;
+ }
+ LOG.warn("delete: Path does not exist: {}", f);
+ return false;
+ }
+
+ // remove link bucket directly
+ if (isLinkBucket(f, ofsPath)) {
+ deleteBucketFromVolume(f, ofsPath);
+ return true;
+ }
+
+ // delete inner content of bucket
+ boolean result = innerDelete(f, recursive);
+
+ // Handle delete bucket
+ deleteBucketFromVolume(f, ofsPath);
+ return result;
+ }
+
+ private boolean isLinkBucket(Path f, OFSPath ofsPath) {
+ try {
+ OzoneBucket bucket = adapterImpl.getBucket(ofsPath, false);
+ if (bucket.isLink()) {
+ return true;
+ }
+ } catch (Exception ex) {
+ return false;
Review Comment:
can log the exception in case of failure.
--
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] sumitagrawl commented on pull request #4405: HDDS-7884. ofs: Recursive delete of path of symlink with empty source fails
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on PR #4405:
URL: https://github.com/apache/ozone/pull/4405#issuecomment-1481147309
> Hi @sumitagrawl , I saw a problem with the symlink delete problem with orphaned link while reviewing this PR. It occurs when the src volume/bucket for the link is deleted. When the src for the link is deleted it causes an orphaned bucket link. This in turn currently causes an error when the target link volume/bucket is recursively listed. This should be handled. It can be reproduced by,
>
> _create src and target link._ `ozone sh bucket link /vol1/abc /s3v/abc` _delete src volume/bucket_ `ozone fs -rm -R -skipTrash ofs://om/vol1`
>
> _list recursively the target link:_ `ozone fs -ls -R ofs://om/s3v` **drwxrwxrwx - hadoop hadoop 0 2023-03-20 19:20 ofs://localhost/s3v/abc ls: Volume not found when getting bucket info**
@neils-dev Above case is handled and PR is updated
--
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