errose28 commented on PR #6919:
URL: https://github.com/apache/ozone/pull/6919#issuecomment-2221027145

   > Until then, I would suggest removing the version condition from usages of 
the annotation as part of this fix. (So only keep it in the method body.)
   
   If I'm understanding correctly, this would look something like this:
   ```
     @RequestFeatureValidator(
         processingPhase = RequestProcessingPhase.PRE_PROCESS,
         requestType = Type.CreateBucket
     )
     public static OMRequest setDefaultBucketLayoutForOlderClients(OMRequest 
req,
         ValidationContext ctx) {
       if (ClientVersion.fromProtoValue(req.getVersion())
           .compareTo(ClientVersion.BUCKET_LAYOUT_SUPPORT) < 0) {
   ...
   ```
   
   We can do that. It does run more code in the common case where client and 
server are the same version. There is a 
[reflection](https://github.com/apache/ozone/blob/47d41da8015d9549a21e22e3138ad9712df52008/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java#L75)
 call on this code path that the annotation condition is able to 
[bypass](https://github.com/apache/ozone/blob/47d41da8015d9549a21e22e3138ad9712df52008/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java#L110)
 but I'm not sure if that's a legitimate concern for performance or not. We do 
have 
[metrics](https://github.com/apache/ozone/blob/f9e5178cddf01d4bd8804ee1247b0f9a23de65c9/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java#L143)
 there fwiw. I'm not opposed to removing the `OLDER_CLIENT` condition from the 
annotation in this change, 
 but it might be safer to leave it in place.


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

Reply via email to