amaliujia commented on code in PR #10062:
URL: https://github.com/apache/ozone/pull/10062#discussion_r3377815149
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCommitPartRequest.java:
##########
@@ -161,6 +161,33 @@ public OMClientResponse
validateAndUpdateCache(OzoneManager ozoneManager, Execut
openKey + "entry is not found in the openKey table",
KEY_NOT_FOUND);
}
+
+ if (multipartKeyInfo == null) {
+ // This can occur when user started uploading part by the time commit
+ // of that part happens, in between the user might have requested
+ // abort multipart upload. If we just throw exception, then the data
+ // will not be garbage collected, so move this part to delete table
+ // and throw error.
+ throw new OMException("No such Multipart upload is with specified " +
+ "uploadId " + uploadID,
+ OMException.ResultCodes.NO_SUCH_MULTIPART_UPLOAD_ERROR);
+ }
+
+ // This gate runs in the replicated apply path, so the check must remain
+ // deterministic across replicas for a given log index. MLV advances only
+ // via the Ratis-logged finalize-upgrade request.
+ if (!ozoneManager.getVersionManager()
+ .isAllowed(OMLayoutFeature.MPU_PARTS_TABLE_SPLIT)
+ && multipartKeyInfo.getSchemaVersion() != 0) {
+ throw new OMException("MPU parts-table split behavior is not allowed "
+
+ "before cluster finalization for commit part request.",
+
OMException.ResultCodes.NOT_SUPPORTED_OPERATION_PRIOR_FINALIZATION);
+ }
+ if (multipartKeyInfo.getSchemaVersion() == 1) {
Review Comment:
Trying to learn the context:
Do we need this to be
```
if (ozoneManager.getVersionManager()
.isAllowed(OMLayoutFeature.MPU_PARTS_TABLE_SPLIT) &&
multipartKeyInfo.getSchemaVersion() == 1)
```
?
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmMultipartKeyInfo.java:
##########
@@ -507,12 +514,21 @@ public MultipartKeyInfo getProto() {
}
builder.addAllAcls(OzoneAclUtil.toProtobuf(acls));
- if (schemaVersion == 0) {
+ if (schemaVersion == LEGACY_SCHEMA_VERSION) {
builder.addAllPartKeyInfoList(partKeyInfoMap);
}
return builder.build();
}
+ private static byte validateAndConvertSchemaVersion(int schemaVersion) {
+ if (schemaVersion != LEGACY_SCHEMA_VERSION
+ && schemaVersion != SPLIT_PARTS_SCHEMA_VERSION) {
+ throw new IllegalArgumentException("Unsupported schemaVersion: "
+ + schemaVersion + ". Expected one of [0, 1].");
+ }
+ return (byte) schemaVersion;
Review Comment:
Any reason why we still keep `SchemaVersion` as a byte in JVM? In its proto
definition schemaVersion is `optional uint32 schemaVersion = 10;`.
--
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]
