ArafatKhan2198 commented on code in PR #10553:
URL: https://github.com/apache/ozone/pull/10553#discussion_r3534341551
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneClientAdapterImpl.java:
##########
@@ -199,11 +197,7 @@ public BasicOzoneClientAdapterImpl(String omHost, int
omPort,
bucketReplicationConfig = this.bucket.getReplicationConfig();
nextReplicationConfigRefreshTime = clock.millis() +
bucketRepConfigRefreshPeriodMS;
- // resolve the bucket layout in case of Link Bucket
- BucketLayout resolvedBucketLayout =
- OzoneClientUtils.resolveLinkBucketLayout(bucket, objectStore, new
HashSet<>());
Review Comment:
The existing tests only check the server part, which was already done before
this PR. Nothing tests the client changes this PR actually makes.
If we had a test that lists a broken shortcut bucket, it would have
immediately failed and exposed Concern of resolveLinkBucketLayout.
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java:
##########
@@ -298,12 +297,7 @@ private OzoneBucket getBucket(String volumeStr, String
bucketStr,
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
- // resolve the bucket layout in case of Link Bucket
Review Comment:
This PR makes the client fully trust the server to do the
shortcut-resolving. That's fine if the server actually does it.
But Ozone clusters sometimes run mixed versions during an upgrade a
brand-new client might talk to an older server that doesn't do the resolving
yet.
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneClientAdapterImpl.java:
##########
@@ -199,11 +197,7 @@ public BasicOzoneClientAdapterImpl(String omHost, int
omPort,
bucketReplicationConfig = this.bucket.getReplicationConfig();
nextReplicationConfigRefreshTime = clock.millis() +
bucketRepConfigRefreshPeriodMS;
- // resolve the bucket layout in case of Link Bucket
- BucketLayout resolvedBucketLayout =
- OzoneClientUtils.resolveLinkBucketLayout(bucket, objectStore, new
HashSet<>());
Review Comment:
The old code had a helpful side job that this PR deleted.
The old `resolveLinkBucketLayout` method did two things at once:
1. Figure out the bucket's layout.
2. If it noticed the shortcut was broken (target missing), it flipped the
flag `sourcePathExist = false`.
That flag was used later, when someone lists the contents of the bucket:
- If `sourcePathExist` is **true** → "go ahead, list the files."
- If `sourcePathExist` is **false** → "this shortcut is broken, just return
an empty list and log a warning." (Graceful, no error.)
This PR deleted the whole `resolveLinkBucketLayout` method. But **that was
the only place in the entire codebase that ever set the flag to false.** So now
the flag is *always* true.
Result: when you list a broken shortcut over the OFS filesystem, the code
thinks "the target exists, go list it" and tries to actually read from a target
that isn't there. Instead of the old friendly "empty list," the user likely
gets an **error**.
Please let me know if I am missing something here.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
