neils-dev commented on pull request #2901: URL: https://github.com/apache/ozone/pull/2901#issuecomment-1059648696
Thank you @hanishakoneru for reviewing this PR. Thanks for your comments. > Do we really need to have _GrpcOMFailoverProxyProvider_ implement _OMFailoverProxyProvider_? A. The _GrpcOMFailoverProxyProvider_ gives the GrpcOmTransport a failover that reuses the existing failover for OmTransports and keeps the same behavior. The `GrpcOmTransport` for the s3gateway client needs to support OM HA. For this we implement the failover to reuse as much of the `OzoneManagerFailoverProvider` as possible so that the failover code is uniform for all ozone client OmTransports (good for maintenance and for switching between transports as they will behave the same). The _GrpcOMFailoverProxyProvider_ provides an implementation that uses the same _RetryPolicy_, _RetryActions_, retry logic including _shouldFailover_, both manual and conditional _performFailover_ as well as the static methods for translating ratis Leader exceptions and uses the same proxy indexes (`getCurrentProxyOMNodeId`). > there should be a base interface/ class that these two can can implement. Setting null values to the proxy maps does not seem right. A. The two failoverprovider proxy maps, omProxies and omProxyInfos for the Grpc failover provider use only the keys of the proxies since the values are hadoop rpc specific. The values are basically stubs and set to null (this is also done for testing in TestOMFailovers). Those proxy maps are used internally in the `OMFailoverProxyProvider` and not used directly by the `GrpcFailoverProxyProvider` subclass. We can open a another jira to look into refactoring the` OMFailoverProxyProvider` as you suggested with a base an two implementations. -- 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]
