You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/04/04 16:24:31 UTC
[GitHub] [ozone] kerneltime commented on a diff in pull request #3262: HDDS-5909. EC: Onboard EC into upgrade framework
kerneltime commented on code in PR #3262:
URL: https://github.com/apache/ozone/pull/3262#discussion_r841921659
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java:
##########
@@ -399,6 +432,28 @@ private LookupKeyResponse lookupKey(LookupKeyRequest request,
return resp.build();
}
+ @RequestFeatureValidator(
+ conditions = ValidationCondition.OLDER_CLIENT_REQUESTS,
+ processingPhase = RequestProcessingPhase.POST_PROCESS,
+ requestType = Type.LookupKey
+ )
+ public static OMResponse disallowLookupKeyResponseWithECReplicationConfig(
+ OMRequest req, OMResponse resp, ValidationContext ctx)
+ throws ServiceException {
+ if (!resp.hasLookupKeyResponse()) {
+ return resp;
+ }
+ if (resp.getLookupKeyResponse().getKeyInfo().hasEcReplicationConfig()) {
Review Comment:
Same as above, the client's actual version matters as much as the relative version.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java:
##########
@@ -416,6 +471,32 @@ private ListBucketsResponse listBuckets(ListBucketsRequest request)
return resp.build();
}
+ @RequestFeatureValidator(
+ conditions = ValidationCondition.OLDER_CLIENT_REQUESTS,
+ processingPhase = RequestProcessingPhase.POST_PROCESS,
+ requestType = Type.ListBuckets
+ )
+ public static OMResponse disallowListBucketsResponseWithECReplicationConfig(
Review Comment:
Why disallow the list bucket command? Ozone CLI can be an older version and it should be able to list all the buckets.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java:
##########
@@ -434,6 +515,29 @@ private ListKeysResponse listKeys(ListKeysRequest request, int clientVersion)
return resp.build();
}
+ @RequestFeatureValidator(
+ conditions = ValidationCondition.OLDER_CLIENT_REQUESTS,
+ processingPhase = RequestProcessingPhase.POST_PROCESS,
+ requestType = Type.ListKeys
+ )
+ public static OMResponse disallowListKeysResponseWithECReplicationConfig(
Review Comment:
Maybe this was discussed, but listing keys should be ok to include EC keys, a client might have logic to test the presence of a file without caring about the replication config. The replication config is orthogonal to any namespace logic.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java:
##########
@@ -378,6 +387,30 @@ private InfoBucketResponse infoBucket(InfoBucketRequest request)
return resp.build();
}
+ @RequestFeatureValidator(
+ conditions = ValidationCondition.OLDER_CLIENT_REQUESTS,
Review Comment:
+1
Either the annotation can accept the client range or we need the check for client version as part of the validation logic.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java:
##########
@@ -399,6 +432,28 @@ private LookupKeyResponse lookupKey(LookupKeyRequest request,
return resp.build();
}
+ @RequestFeatureValidator(
+ conditions = ValidationCondition.OLDER_CLIENT_REQUESTS,
+ processingPhase = RequestProcessingPhase.POST_PROCESS,
+ requestType = Type.LookupKey
+ )
+ public static OMResponse disallowLookupKeyResponseWithECReplicationConfig(
+ OMRequest req, OMResponse resp, ValidationContext ctx)
+ throws ServiceException {
+ if (!resp.hasLookupKeyResponse()) {
+ return resp;
+ }
+ if (resp.getLookupKeyResponse().getKeyInfo().hasEcReplicationConfig()) {
Review Comment:
Won't leave the same comment elsewhere, I think there are more methods that need this update.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java:
##########
@@ -378,6 +387,30 @@ private InfoBucketResponse infoBucket(InfoBucketRequest request)
return resp.build();
}
+ @RequestFeatureValidator(
+ conditions = ValidationCondition.OLDER_CLIENT_REQUESTS,
+ processingPhase = RequestProcessingPhase.POST_PROCESS,
+ requestType = Type.InfoBucket
+ )
+ public static OMResponse disallowInfoBucketResponseWithECReplicationConfig(
Review Comment:
Unless the client jar will barf, it should be ok for an older client to read the metadata it supports?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org