fapifta commented on pull request #3155: URL: https://github.com/apache/ozone/pull/3155#issuecomment-1062041288
Thank you for the reviews, I went down deeper into the code that is touched by this PR, and I found a couple of issues that we have to address, and I think I have kind of a plan. So, currently we have the problem listed by Attila in HDDS-6418. Besides that I have found that the container operation client uses the client version, but in that case the Ozone Manager is the client, so that has to be changed if we have committed the OzoneManagerVersion class. There is the Balancer, there is the ReplicationManager which are also using the ClientVersion now, and I am 99% sure that they should not. There is PipelineManager which also provides the ClientVersion.CURRENT_VERSION to a toProto method, which is certainly not right similarly as it is not right in HDDS-6418. On the other hand, you are right, the problem when a new client is connecting is real, and a FUTURE_VERSION or something has to be added. I think the version number of the FUTURE_VERSION constant should be -1, and that should be the 0th ordinal in the enum, so that does not leads extra code during latest version calculation. The version difference check is a bit tricky this way though, but it can be done for example this way: (clientversion & Integer.MAX_VALUE) > serverversion -> newer client I am adding utility methods to compare the versions into the base class that is proposed for the enums, and refresh the PR shortly. Because of the problems with the usage of ClientVersion and DatanodeVersion, I would like to suggest to go and commit this PR first, and in the meantime I will file follow up JIRAs to review the usage of these, and then while we are unblocked to continue with FSO and EC and start to use these changes, while we also can aim to switch to enum usage wherever it is appropriate in the current code where we need to fix things anyway. What do you think? -- 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]
