bharathv commented on pull request #1593: URL: https://github.com/apache/hbase/pull/1593#issuecomment-620120910
Trying to understand the motivation for this refactor. > The function here is like region replica so I do not think we should implement it in the rpc framework, as the prepered way in HBase, is to implement a special RpcRetryingCaller. I think hedging RPC support is a standard RPC layer thing (for ex grpc: [1]). We can probably make a case to port region replica to hedging framework in the RPC layer. Fun fact, when I first implemented this as a prototype, I had all the logic in the master registry itself and then the review comments from @apurtell and @saintstack were to push it into a layer below, which is RPC and it made sense to me. Also, anyone who wants to implement hedging means they have to write a lot of boiler plate code around synchronizing responses from multiple RPC threads and issuing cancellations etc. It is not trivial and why would anyone want to do that all over again? There are some issues we can fix, like the one you noted in the jira, to fix the usage of common fork join pool (I'm trying to think why I did that, I initially implemented it like a regular async code implementation, but revered it later). Let me think a little bit about why I did this and get back to you. > And I dropped the fan out parameter. This is because that, usually we will only have 2 masters, and even if we have more, I do not think it makes much difference. So I only support two options, send the request one by one, or send them altogether at the same time. We use > 5 masters for redundancy in our critical production deployments. If we were to use this, we definitely don't want to spam every master for each of the request. -1 on removing it unless you have a strong counter argument. [1] https://github.com/grpc/proposal/blob/master/A6-client-retries.md#hedging-policy ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
