ivandika3 commented on code in PR #7558:
URL: https://github.com/apache/ozone/pull/7558#discussion_r1879769500
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1079,6 +1079,8 @@ message KeyArgs {
// This allows a key to be created an committed atomically if the original
has not
// been modified.
optional uint64 expectedDataGeneration = 23;
+
+ optional int32 partNumber = 24;
Review Comment:
As per my previous comment, this might not be needed since we already have
`multipartNumber`
##########
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:
Please generate a new `OzoneManagerVersion` decide on whether to use old or
new client implementation based on the OM version. See `listStatusLight`.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -509,6 +509,27 @@ private OmKeyInfo readKeyInfo(OmKeyArgs args, BucketLayout
bucketLayout)
if (args.getLatestVersionLocation()) {
slimLocationVersion(value);
}
+ Integer partNumberParam = args.getPartNumber();
+ if (partNumberParam != null && partNumberParam > 0) {
+ OmKeyLocationInfoGroup latestLocationVersion =
value.getLatestVersionLocations();
+ if (latestLocationVersion != null &&
latestLocationVersion.isMultipartKey()) {
+
+ value.setKeyLocationVersions(
+ Collections.singletonList(
+ new OmKeyLocationInfoGroup(
+ latestLocationVersion.getVersion(),
+ value.getCurrentlocationsPartsMap()
+ .getOrDefault(partNumberParam,
Collections.emptyList()),
+ true
+ )
+ )
+ );
+ value.setDataSize(
+ value.getCurrentDataSizePartsMap()
+ .getOrDefault(partNumberParam, 0L)
+ );
+ }
+ }
Review Comment:
I think we can just do main logic here. Just filter based location info
based on the partNumber and calculate the data size based on the filtered part
number.
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java:
##########
@@ -210,6 +221,25 @@ public void setKeyLocationVersions(
this.keyLocationVersions = keyLocationVersions;
}
+ private void refreshCurrentLocationPartsMap() {
+ if (this.keyLocationVersions.size() > 0) {
+ this.currentlocationsPartsMap =
this.keyLocationVersions.get(keyLocationVersions.size() - 1)
+ .getLocationList()
+ .stream()
+ .filter(it -> it.getPartNumber() != 0)
+
.collect(Collectors.groupingBy(OmKeyLocationInfo::getPartNumber));
+ this.currentDataSizePartsMap =
this.currentlocationsPartsMap.entrySet().stream()
+ .map(it -> Pair.of(
+ it.getKey(),
+ it.getValue().stream().
+ mapToLong(BlockLocationInfo::getLength)
+ .sum()
+ )
+ )
+ .collect(Collectors.toMap(Pair::getKey, Pair::getValue));
+ }
+ }
+
Review Comment:
I think to just move this logic to `KeyManagerImpl`.
##########
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:
Another way is to just use `getS3PartkeyInfo` (only if we are using the
`multipartNumber` instead of the new `partNumber`, see my other comments), but
still retain the client-side filtering logic. That way, it should be compatible.
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java:
##########
@@ -351,6 +381,7 @@ public synchronized void appendNewBlocks(
if (updateTime) {
setModificationTime(Time.now());
}
+ refreshCurrentLocationPartsMap();
Review Comment:
Why is this called in `appendNewBlocks`? `appendNewBlocks` are only called
in server side and the `currentlocationsPartsMap` and `currentDataSizePartsMap`
are not persisted right?
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyArgs.java:
##########
@@ -54,6 +54,7 @@ public final class OmKeyArgs implements Auditable {
private final boolean headOp;
private final boolean forceUpdateContainerCacheFromSCM;
private final Map<String, String> tags;
+ private final Integer partNumber;
Review Comment:
Do we need this since we already have `multipartUploadPartNumber`?
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java:
##########
@@ -188,6 +191,14 @@ public String getOwnerName() {
return ownerName;
}
+ public Map<Integer, List<OmKeyLocationInfo>> getCurrentlocationsPartsMap() {
+ return currentlocationsPartsMap;
+ }
+
+ public Map<Integer, Long> getCurrentDataSizePartsMap() {
+ return currentDataSizePartsMap;
+ }
+
Review Comment:
Do we need this? It's not persisted right?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -801,7 +822,7 @@ public boolean isSstFilteringSvcEnabled() {
TimeUnit.MILLISECONDS);
return serviceInterval != DISABLE_VALUE;
}
-
+
Review Comment:
Nit: unnecessary.
--
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]
