[jira] [Commented] (HBASE-17576) [C++] Implement request retry mechanism over RPC for Multi calls.

["Enis Soztutar (JIRA)" <ji...@apache.org>]

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

Enis Soztutar commented on HBASE-17576:
---------------------------------------

- Here, ActionsByServer is already a hash map, but you are iterating through the keys to find the element. You should construct the map with the equals and hash methods, no? 
{code}
for (auto itr = actions_by_server.begin(); itr != actions_by_server.end(); ++itr) {
+              ServerNameEquals server_name_equals;
+              if (server_name_equals(std::make_shared<ServerName>(region_loc->server_name()),
+                                     itr->first)) {
{code}
You can take a look at location-cache.h for examples. 
- This will be replaced by the caller-level mutex? 
{code}
+  std::mutex action2errors_lock_;
{code}
- In raw-async-table.cc, you may need to do the same trick we do in Get(Get &) method:
{code}
  // Return the Future we obtain from the call(). However, we do not want the Caller to go out of
  // context and get deallocated since the caller injects a lot of closures which capture [this, &]
  // which is use-after-free. We are just passing an identity closure capturing caller by value to
  // ensure  that the lifecycle of the Caller object is longer than the retry lambdas.
  return caller->Call().then([caller](const auto r) { return r; });
{code} 
This is to make sure that the Caller object is not de-allocated when you return the Future from this method.
- For this comment:
{code}
+  // TODO we need to optimize this. No need to call ToMultiRequest twice;
+  // Last patch we were passing multi_req by reference to ToMultiRequest()
+  // It is failing sometimes so doing it this way for now
{code}
it is happening because you have a unique_ptr here which you are giving ownership of it to the rpc-client when you are calling AysncCall() with std::move(). Then you need to use it again, in the ResponseConverter, but it is no longer there. Two possible solutions:
 -- you can change the all rpc end-to-end be based on shared_ptr<Request> rather than unique_ptr<Request> 
 -- you can move the Request to be inside the Response when you are done at the RPC layer so that the ResponseConverter can access the Request object from there. To me it sounds like this option is easier. The other one is best done in a different jira. 

> [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
>
>
> 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)