Mailing-List: contact issues-help@hbase.apache.org; run by ezmlm
Precedence: bulk
Date: Mon, 13 Mar 2017 21:55:41 +0000 (UTC)
From: "Enis Soztutar (JIRA)" <jira@apache.org>
To: issues@hbase.apache.org
Message-ID: <JIRA.13050179.1489189565000.7395.1489442141736@Atlassian.JIRA>
In-Reply-To: <JIRA.13050179.1489189565000@Atlassian.JIRA>
References: <JIRA.13050179.1489189565000@Atlassian.JIRA> <JIRA.13050179.1489189565125@jira-lw-us.apache.org>
Subject: [jira] [Commented] (HBASE-17771) [C++] Classes required for
 implementation of BatchCallerBuilder
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
archived-at: Mon, 13 Mar 2017 21:55:46 -0000


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

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

- Changes in {{core/BUCK}} undos the previous changes from HBASE-17728. This is probably due to rebasing, but we need HBASE-17728. 
- {{Action::OriginalIndex()}} should be named {{Action::original_index()}} and {{Action::GetAction()}} should be {{Action::action()}}. Maybe in the other classes as well (my reading of the conventions is that if the class method is an accessor to a class field, you can name it like foo_bar(), instead of FooBar(). 
- Class {{MultiAction}} is not needed?. Seems to be coming from the AsyncProcess based impl. I've checked the patch at HBASE-17576, not used there as well. 
- Remove commented out code: 
{code}
+  /*static std::shared_ptr<hbase::MultiResponse> GetResults(const std::shared_ptr<Request>& req,
+   std::unique_ptr<Response> resp);*/
The non-commented out version takes Request by raw-pointer which is probably not what we want. 
{code}
- The CellScanner::Advance() might return false as well for the below code: 
{code}
-    while (cell_scanner->Advance()) {
+    int num_cells = 0;
+    while (num_cells != result.associated_cell_count()) {
+      cell_scanner->Advance();
{code}
So, I think you should check for that, and throw an exception if we wanted to read that many cells, but the CellScanner::Advance() is returning false.
- Are you gonna address this? {{+// TODO Revisit this class once}} 
- RegionRequest::ActionList is a shared_ptr, while ServerRequest::ActionsByRegion is not. I think you don't need the shared_ptr in RegionRequest::ActionList, especially because you are returning via {{const &}}. 
- RequestConverter::ToGet() is good, but it is duplicating the code. Maybe make it so that ToGetRequest() uses that. 


> [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
>
>
> Separating depedencies of BatchCallerBuilder.


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