ivandika3 commented on code in PR #5668:
URL: https://github.com/apache/ozone/pull/5668#discussion_r1469215312
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCompleteRequest.java:
##########
@@ -80,6 +81,29 @@ public class S3MultipartUploadCompleteRequest extends
OMKeyRequest {
private static final Logger LOG =
LoggerFactory.getLogger(S3MultipartUploadCompleteRequest.class);
+ private BiFunction<OzoneManagerProtocolProtos.Part, PartKeyInfo,
MultipartCommitRequestPart> eTagBasedValidator =
+ (part, partKeyInfo) -> {
+ String eTag = part.getETag();
+ String dbPartETag = null;
+ String dbPartName = null;
+ if (partKeyInfo != null) {
+ dbPartETag = partKeyInfo.getPartKeyInfo().getMetadata(0).getValue();
Review Comment:
Regarding this, I might miss something, but this assumes that the first
element in the metadata is the eTag? Maybe we can explicitly get the metadata
which key is equal to `OzoneConsts.ETAG`?
Currently it seems that only ETAG is the only metadata stored on the
multipart upload part so it should be fine for now.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCompleteRequest.java:
##########
@@ -642,10 +661,36 @@ private String multipartUploadedKeyHash(
StringBuffer keysConcatenated = new StringBuffer();
for (PartKeyInfo partKeyInfo: partsList) {
keysConcatenated.append(KeyValueUtil.getFromProtobuf(partKeyInfo
- .getPartKeyInfo().getMetadataList()).get("ETag"));
+ .getPartKeyInfo().getMetadataList()).get(OzoneConsts.ETAG));
Review Comment:
Regarding the incomplete multipart uploads, the parts that do not have
"eTag" yet will return null. In `StringBuffer`, it will append four characters
"null". However, I think there is little we can do here, so I think it should
be fine to handle the incompatibility.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCompleteRequest.java:
##########
@@ -491,24 +515,19 @@ private long getMultipartDataSize(String requestedVolume,
OzoneManager ozoneManager) throws OMException {
long dataSize = 0;
int currentPartCount = 0;
+ boolean eTagBasedValidationAvailable =
partsList.stream().allMatch(OzoneManagerProtocolProtos.Part::hasETag);
// Now do actual logic, and check for any Invalid part during this.
for (OzoneManagerProtocolProtos.Part part : partsList) {
currentPartCount++;
int partNumber = part.getPartNumber();
- String partName = part.getPartName();
-
PartKeyInfo partKeyInfo = partKeyInfoMap.get(partNumber);
-
- String dbPartName = null;
- if (partKeyInfo != null) {
- dbPartName = partKeyInfo.getPartName();
- }
- if (!StringUtils.equals(partName, dbPartName)) {
- String omPartName = partKeyInfo == null ? null : dbPartName;
+ MultipartCommitRequestPart requestPart = eTagBasedValidationAvailable ?
+ eTagBasedValidator.apply(part, partKeyInfo) :
partNameBasedValidator.apply(part, partKeyInfo);
+ if (!requestPart.isValid()) {
throw new OMException(
failureMessage(requestedVolume, requestedBucket, keyName) +
- ". Provided Part info is { " + partName + ", " + partNumber +
- "}, whereas OM has partName " + omPartName,
+ ". Provided Part info is { " + requestPart.getRequestPartId() + ",
" + partNumber +
+ "}, whereas OM has eTag " + requestPart.getOmPartId(),
OMException.ResultCodes.INVALID_PART);
Review Comment:
For my understanding: the idea seems to pass validation even if some of the
completed parts do not have eTag field, in that case, we will only validate on
the partName (since it always exist in both old and new versions), if all
completed parts have eTag, the MPU complete will pass validation if the eTag
metadata is equal to either the persisted partName and the eTag field (both
partName and eTag should have the same 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: [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]
