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: [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]
