ivandika3 commented on code in PR #7558:
URL: https://github.com/apache/ozone/pull/7558#discussion_r1883699785
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -1765,16 +1765,7 @@ public OzoneKeyDetails getS3KeyDetails(String
bucketName, String keyName)
@Override
public OzoneKeyDetails getS3KeyDetails(String bucketName, String keyName,
int partNumber) throws IOException {
- OmKeyInfo keyInfo = getS3KeyInfo(bucketName, keyName, false);
- List<OmKeyLocationInfo> filteredKeyLocationInfo = keyInfo
- .getLatestVersionLocations().getBlocksLatestVersionOnly().stream()
- .filter(omKeyLocationInfo -> omKeyLocationInfo.getPartNumber() ==
- partNumber)
- .collect(Collectors.toList());
- keyInfo.updateLocationInfoList(filteredKeyLocationInfo, false);
- keyInfo.setDataSize(filteredKeyLocationInfo.stream()
- .mapToLong(OmKeyLocationInfo::getLength)
- .sum());
+ OmKeyInfo keyInfo = getS3PartKeyInfo(bucketName, keyName, partNumber);
return getOzoneKeyDetails(keyInfo);
Review Comment:
> Logic must moved to the server side. The second variant is not acceptable
The second variant ensure that if the server-side does not filter the
location list based on parts, the client-side will do it instead. Therefore it
doesn't matter whether S3G is talking to old OM or new OM, the returned result
should be the same. However, I prefer to explicitly use the
`OzoneManagerVersion` (i.e. first variant) since there are more in-built
guarantees.
> Do you offer to create a new method with server-side filtering? Also, need
to create the same request/response model but with a different name? I suspect
that the reason is backward compatibility. But does current realization break
something? Is it possible, when the client was updated and the server-side part
was not?
My suggestion is to create a new `OzoneManagerVersion` and make the client
(S3G) behavior adapts depending on the OM version.
In `OzoneManagerVersion`, add a new enum:
```java
S3_PART_AWARE_GET(10, "OzoneManager version that supports S3 get for a
specific multipart upload part number")
```
In `RpcClien#getS3KeyDetails`, take into account the OM version the client
is talking to.
```java
@Override
public OzoneKeyDetails getS3KeyDetails(String bucketName, String keyName,
int partNumber) throws IOException {
OmKeyInfo keyInfo;
if (omVersion.compareTo(OzoneManagerVersion.S3_PART_AWARE_GET) >= 0) {
keyInfo = getS3PartKeyInfo(bucketName, keyName, partNumber);
} else {
keyInfo = getS3KeyInfo(bucketName, keyName, false);
List<OmKeyLocationInfo> filteredKeyLocationInfo = keyInfo
.getLatestVersionLocations().getBlocksLatestVersionOnly().stream()
.filter(omKeyLocationInfo -> omKeyLocationInfo.getPartNumber() ==
partNumber)
.collect(Collectors.toList());
keyInfo.updateLocationInfoList(filteredKeyLocationInfo, false);
keyInfo.setDataSize(filteredKeyLocationInfo.stream()
.mapToLong(OmKeyLocationInfo::getLength)
.sum());
}
return getOzoneKeyDetails(keyInfo);
```
You can then optionally extract the filtering logic of the server and client
to a common utility method.
--
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]
