yandrey321 commented on code in PR #11380:
URL: https://github.com/apache/ozone/pull/11380#discussion_r4211806295
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java:
##########
@@ -1377,13 +1377,40 @@ public FileChecksum getFileChecksum(String keyName,
long length)
return null;
}
OFSPath ofsPath = new OFSPath(keyName, config);
- OzoneVolume volume = objectStore.getVolume(ofsPath.getVolumeName());
- OzoneBucket bucket = getBucket(ofsPath, false);
- return OzoneClientUtils.getFileChecksumWithCombineMode(
- volume, bucket, ofsPath.getKeyName(),
- length, combineMode,
- ozoneClient.getObjectStore().getClientProxy());
-
+ if (ofsPath.getBucketName().isEmpty()) {
+ // throw FileNotFoundException in this case to make Hadoop common happy
+ throw new FileNotFoundException(
+ "getFileChecksum: Invalid argument: given bucket string is empty.");
+ }
+ // OM LookupFile rejects OBJECT_STORE buckets (HDDS-15951), so the volume
and
+ // bucket no longer have to be fetched up front. During a rolling upgrade
a new
+ // client can talk to an older OM that lacks that check, so when the
negotiated
+ // OM version predates LOOKUP_FILE_REJECTS_OBS, fall back to LookupKey and
+ // validate the bucket layout client-side as before.
+ boolean omRejectsObs = proxy.getOmVersion()
+ .compareTo(OzoneManagerVersion.LOOKUP_FILE_REJECTS_OBS) >= 0;
+ if (!omRejectsObs) {
+ // Called for its layout validation side effect; the bucket itself is
not needed.
+ getBucket(ofsPath, false);
+ }
+ try {
+ return OzoneClientUtils.getFileChecksumWithCombineMode(
+ ofsPath.getVolumeName(), ofsPath.getBucketName(),
ofsPath.getKeyName(),
+ length, combineMode,
+ ozoneClient.getObjectStore().getClientProxy(), omRejectsObs);
+ } catch (OMException e) {
+ if (e.getResult() == OMException.ResultCodes.NOT_SUPPORTED_OPERATION) {
+ // OM rejects LookupFile on an OBJECT_STORE bucket (no file system
+ // semantics). Surface it as IllegalArgumentException, matching the
+ // pre-HDDS-15951 client-side layout check.
+ throw new IllegalArgumentException(e.getMessage());
+ } else if (e.getResult() == OMException.ResultCodes.NOT_A_FILE) {
Review Comment:
Fixed
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java:
##########
@@ -1377,13 +1377,40 @@ public FileChecksum getFileChecksum(String keyName,
long length)
return null;
}
OFSPath ofsPath = new OFSPath(keyName, config);
- OzoneVolume volume = objectStore.getVolume(ofsPath.getVolumeName());
- OzoneBucket bucket = getBucket(ofsPath, false);
- return OzoneClientUtils.getFileChecksumWithCombineMode(
- volume, bucket, ofsPath.getKeyName(),
- length, combineMode,
- ozoneClient.getObjectStore().getClientProxy());
-
+ if (ofsPath.getBucketName().isEmpty()) {
+ // throw FileNotFoundException in this case to make Hadoop common happy
+ throw new FileNotFoundException(
+ "getFileChecksum: Invalid argument: given bucket string is empty.");
+ }
+ // OM LookupFile rejects OBJECT_STORE buckets (HDDS-15951), so the volume
and
+ // bucket no longer have to be fetched up front. During a rolling upgrade
a new
+ // client can talk to an older OM that lacks that check, so when the
negotiated
+ // OM version predates LOOKUP_FILE_REJECTS_OBS, fall back to LookupKey and
+ // validate the bucket layout client-side as before.
+ boolean omRejectsObs = proxy.getOmVersion()
+ .compareTo(OzoneManagerVersion.LOOKUP_FILE_REJECTS_OBS) >= 0;
+ if (!omRejectsObs) {
+ // Called for its layout validation side effect; the bucket itself is
not needed.
+ getBucket(ofsPath, false);
+ }
+ try {
+ return OzoneClientUtils.getFileChecksumWithCombineMode(
+ ofsPath.getVolumeName(), ofsPath.getBucketName(),
ofsPath.getKeyName(),
+ length, combineMode,
+ ozoneClient.getObjectStore().getClientProxy(), omRejectsObs);
+ } catch (OMException e) {
+ if (e.getResult() == OMException.ResultCodes.NOT_SUPPORTED_OPERATION) {
+ // OM rejects LookupFile on an OBJECT_STORE bucket (no file system
+ // semantics). Surface it as IllegalArgumentException, matching the
+ // pre-HDDS-15951 client-side layout check.
+ throw new IllegalArgumentException(e.getMessage());
+ } else if (e.getResult() == OMException.ResultCodes.NOT_A_FILE) {
+ // LookupFile reports a directory this way; a checksum only exists for
a
+ // file, so report it the same way HDFS does.
+ throw new FileNotFoundException(e.getMessage());
Review Comment:
Fixed
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java:
##########
@@ -1377,13 +1377,40 @@ public FileChecksum getFileChecksum(String keyName,
long length)
return null;
}
OFSPath ofsPath = new OFSPath(keyName, config);
- OzoneVolume volume = objectStore.getVolume(ofsPath.getVolumeName());
- OzoneBucket bucket = getBucket(ofsPath, false);
- return OzoneClientUtils.getFileChecksumWithCombineMode(
- volume, bucket, ofsPath.getKeyName(),
- length, combineMode,
- ozoneClient.getObjectStore().getClientProxy());
-
+ if (ofsPath.getBucketName().isEmpty()) {
+ // throw FileNotFoundException in this case to make Hadoop common happy
+ throw new FileNotFoundException(
+ "getFileChecksum: Invalid argument: given bucket string is empty.");
+ }
+ // OM LookupFile rejects OBJECT_STORE buckets (HDDS-15951), so the volume
and
+ // bucket no longer have to be fetched up front. During a rolling upgrade
a new
+ // client can talk to an older OM that lacks that check, so when the
negotiated
+ // OM version predates LOOKUP_FILE_REJECTS_OBS, fall back to LookupKey and
+ // validate the bucket layout client-side as before.
+ boolean omRejectsObs = proxy.getOmVersion()
Review Comment:
Fixed
--
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]
