[
https://issues.apache.org/jira/browse/HBASE-17465?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15840871#comment-15840871
]
Enis Soztutar commented on HBASE-17465:
---------------------------------------
Thanks [~xiaobingo] for working on this. It is a pretty good start.
A couple of comments for the patch:
- HBaseRpcController needs to extend {{google::protobuf::RpcController}}, no?
- no need to define hconstants.h. You can just put the RETRY_BACKOFF in one of
the Rpc related headers.
- If you need, you can add some logging via VLOG or DLOG. We will probably
need trace-level logs once we try this on clusters. Feel free to add it only if
you need it to debug now. Otherwise, we can do that later.
- At this layer (asynccallablefactory, AsyncSingleRequestRpcRetryingCaller),
we do not need the template parameters for {{<REQ, PREQ, PRESP, RESP>}}. The
callables can operate as {{<RESP, REQ>}}, where both the request the response
are PB objects. The request / response conversion happens at the
RawAsyncTableImpl side in java. We can leave that to a later patch I think.
- You can just use the HBaseRpcController here, no need to make custom
RpcContollers yet:
{code}
conn_->get_rpc_controller_factory()->NewController();
{code}
- RegionLocateType can go to region-location.h. You can remove
hregion-location.h and .cc. We can hook up the logic in location-cache.cc after
this patch when needed.
- You can directly pass the location-cache instead of doing this, if you need
it:
{code}
conn_->get_locator()
{code}
Actually, the location-cache is not mockable right now, but we probably will
need that. The location-cache is also doing both caching, and also zk + meta
requests. Maybe we should break them up. Let me know if you depend on that for
mocking.
- For exception.h, it seems that the underlying rpc channels should be throwing
these exceptions coming from exceptions at the java server layer. We usually
serialize the exception trace as a string, and pass it back to in the
RpcResponse. Let me take a look at what happens when Rpc's fail. If you do not
need to rely on the InstanceOf kind of exception hierarchy, I think we return a
response indicating whether to retry or not. We can build the exceptions,
without using the cpp exception hierarchy. Just an option.
> [C++] implement request retry mechanism over RPC
> ------------------------------------------------
>
> Key: HBASE-17465
> URL: https://issues.apache.org/jira/browse/HBASE-17465
> Project: HBase
> Issue Type: Sub-task
> Reporter: Xiaobing Zhou
> Assignee: Xiaobing Zhou
> Attachments: HBASE-17465-HBASE-14850.000.patch,
> HBASE-17465-HBASE-14850.001.patch, HBASE-17465-HBASE-14850.002.patch,
> HBASE-17465-HBASE-14850.003.patch, HBASE-17465-HBASE-14850.004.patch
>
>
> HBASE-17051 implemented RPC layer. Requests retries will make system
> reliable. This JIRA proposes adding it, which corresponds to similar
> implementation in SingleRequestCallerBuilder (or BatchCallerBuilder,
> ScanSingleRegionCallerBuilder, SmallScanCallerBuilder, etc.) and
> AsyncSingleRequestRpcRetryingCaller. As a bonus, retry should be more
> generic, decoupled with HRegionLocation.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
