[ 
https://issues.apache.org/jira/browse/HBASE-17771?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15937128#comment-15937128
 ] 

Enis Soztutar commented on HBASE-17771:
---------------------------------------

Thanks for the updated patch. 
- Remove the commented-out code here:
{code}
+  std::shared_ptr<RegionLoadStats> stat_;  // = 
std::make_shared<RegionLoadStats>();
{code}
- Is this conditional coming from your custom testing? Need to remove it. 
{code}
+      if (action_num % 3 == 0)
+        pb_action->set_allocated_get(nullptr);
+      else
+        pb_action->set_allocated_get(pb_get.release());
{code}
- Use {{auto unique = std::make_unique<Request>(*pb_req)}} here:
{code}
+  auto unique = std::unique_ptr<Request>(new Request(*pb_req));
{code}
- And maybe replace 
{code}
+  auto region_specifier = new hbase::pb::RegionSpecifier();
+  RequestConverter::SetRegion(region_name, region_specifier);
+  auto pb_region_action = pb_msg->add_regionaction();
+  pb_region_action->set_allocated_region(region_specifier);
{code} 
with something like: 
{code}
+  auto pb_region_action = pb_msg->add_regionaction();
+  RequestConverter::SetRegion(region_name, pb_region_action->mutable_region());
{code}
- Are these statements even necessary? I don't think so: 
{code}
+  auto unique = std::unique_ptr<Request>(new Request(*pb_req));
+  VLOG(8) << "pb_req Addr:-" << pb_req.get() << "; unique Addr:-" << 
unique.get();
+  multi_req = std::move(unique);
+  VLOG(8) << "multi_req Addr:-" << multi_req.get();
{code} 
Seems coming from debugging. 
- Rename this var to be {{multi_response}}. 
+  auto multi_results = std::make_unique<hbase::MultiResponse>();
- Can we have some consistency in naming these (and related methods): 
{code}
MultiResponse::Add() 
RegionRequest::add_action()
RegionResult::add_result()
ServerRequest::AddAction() 
{code}
Either it should be add_action and add_result or AddAction and AddResult. 
- Are you gonna handle thread safety in this patch, or in a follow up? 





> [C++] Classes required for implementation of BatchCallerBuilder
> ---------------------------------------------------------------
>
>                 Key: HBASE-17771
>                 URL: https://issues.apache.org/jira/browse/HBASE-17771
>             Project: HBase
>          Issue Type: Sub-task
>            Reporter: Sudeep Sunthankar
>            Assignee: Sudeep Sunthankar
>             Fix For: HBASE-14850
>
>         Attachments: HBASE-17771.HBASE-14850.v1.patch, 
> HBASE-17771.HBASE-14850.v2.patch, HBASE-17771.HBASE-14850.v3.patch
>
>
> Separating depedencies of BatchCallerBuilder.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Reply via email to