Apache9 commented on pull request #1593: URL: https://github.com/apache/hbase/pull/1593#issuecomment-620925029
Well, all rpc frameworks have a dream to implement everything. But unfortunately, this is a rpc framework only for HBase, not for everyone. So there is a final judgement on whether or not implement a feature in rpc framework, is that if it introduces more complexity in code. The motivation here is to support hedge read in MasterRegistry, and you can see the patch, "+186 -629", I just added 186 lines but removed 629 lines to just support the same function. OK there are about 100 lines of test code removed, any way, it is still about 200 vs. 500. So I think for supporting MasterRegistry, implement it in MaterRegistry directly is much better than imlementing it in rpc. And the reason is very easy to understand, we do not need extra abstraction... On other usage of hedged reads, the first thing is the read replica feature. If you can prove that, you can use less code to support same feature in rpc framework than RpcRetryingCaller, then I'm OK with implementing the hedged reads feature in rpc. So my suggestion here is still, remove the hedged reads feature to not block the 2.3.0 release, and open another issue for implementing it, as it has much more long term impact than the MasterRegistry feature. Thanks. ---------------------------------------------------------------- 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]
