[
https://issues.apache.org/jira/browse/HBASE-17576?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15987868#comment-15987868
]
Enis Soztutar commented on HBASE-17576:
---------------------------------------
- From a coding practice, this is not how we check preconditions:
{code}
+ if (conn_) {
+ location_cache_ = conn_->location_cache();
+ rpc_client_ = conn_->rpc_client();
+ cpu_pool_ = conn_->cpu_executor();
+ } else {
+ throw std::runtime_error("Passed null AsyncConnection. Unexpected.");
+ }
{code}
You should use CHECK macros from glog like this:
CHECK(conn_ != nullptr);
- Same thing for the below line (and elsewhere as well):
{code}
if ((nullptr == location_cache_) || (nullptr == rpc_client_) || (nullptr ==
cpu_pool_))
{code}
- It is fine to not fix the retries in this patch:
{code}
// TODO This gives segfault @ present, when retried
{code}
Please do a follow up patch. I'll commit HBASE-17800 shortly which should help
with this.
- When formatting the code with bin/format-code.sh, it is interesting that your
version of clang-format does it slightly differently. Are you running inside
the docker env? Have you upgraded any of the LLVM toolchain? This constant
change in formatting makes the patches harder to read.
- Remove these commented out code (and others if there are any):
{code}
+//#include "core/async-rpc-retrying-caller-factory.h"
{code}
{code}
+ const /*std::vector<TryResult>*/auto tresults =
collectAll(*fresults.get()).get(
{code}
{code}
+ // typedef BatchCallerBuilder GenenericThisType;
{code}
- Simple client does not write test2 and testextra columns, why are you adding
them to the list of gets:
{code}
+ gets.push_back(std::make_shared<hbase::Get>(hbase::Get("test2")));
+ gets.push_back(std::make_shared<hbase::Get>(hbase::Get("testextra")));
{code}
- Result->DebugString() already prints out the Cell information, no? Why are
you doing extra logging inside this if:
{code}
+ if(!result->IsEmpty()) {
{code}
- No need for checking the exception type in table.cc:
{code}
+ if
(result.exception().is_compatible_with<hbase::RetriesExhaustedException>()) {
{code}
We need to re-throw the exception to the application if any coming from one of
the Get results.
Will do more review in the next version.
> [C++] Implement request retry mechanism over RPC for Multi calls.
> -----------------------------------------------------------------
>
> Key: HBASE-17576
> URL: https://issues.apache.org/jira/browse/HBASE-17576
> Project: HBase
> Issue Type: Sub-task
> Reporter: Sudeep Sunthankar
> Assignee: Sudeep Sunthankar
> Attachments: HBASE-17576.HBASE-14850.v1.patch,
> HBASE-17576.HBASE-14850.v2.patch, HBASE-17576.HBASE-14850.v3.patch,
> HBASE-17576.HBASE-14850.v4.patch, HBASE-17576.HBASE-14850.v5.patch,
> HBASE-17576.HBASE-14850.v6.patch, HBASE-17576.HBASE-14850.v7.patch,
> HBASE-17576.HBASE-14850.v8.patch, HBASE-17576.HBASE-14850.v9.patch
>
>
> This work is based on top of HBASE-17465. Multi Calls will be based on this.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
