Mailing-List: contact issues-help@hbase.apache.org; run by ezmlm
Precedence: bulk
Date: Wed, 21 Dec 2016 17:34:58 +0000 (UTC)
From: "stack (JIRA)" <jira@apache.org>
To: issues@hbase.apache.org
Message-ID: <JIRA.13029441.1482239016000.573388.1482341698365@Atlassian.JIRA>
In-Reply-To: <JIRA.13029441.1482239016000@Atlassian.JIRA>
References: <JIRA.13029441.1482239016000@Atlassian.JIRA> <JIRA.13029441.1482239016602@arcas>
Subject: [jira] [Commented] (HBASE-17345) Implement batch
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
archived-at: Wed, 21 Dec 2016 17:35:00 -0000


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

stack commented on HBASE-17345:
-------------------------------

{code}
....
72	 * And the {@link #maxAttempts} is a limit for each single operation in the batch logically. In the
73	 * implementation, we will record a {@code tries} parameter for each operation group, and if it is
74	 * split to several groups when retrying, the sub groups will inherit the {@code tries}. You can
75	 * imagine that the whole retrying process is a tree, and the {@link #maxAttempts} is the limit of
76	 * the depth of the tree.
77	 */
{code}

Trying to understand, the tree will only have a depth of one; i.e. a branch for each regionserver the batch is going against? Each branch can run up its own maxAttempts?  The tries is not shared amongst the branches? Regardless of how many retries, the operation will stop after operationTimeoutNs? If so, sounds good.

It has to be an Impl in the below, it can't be Interface?

  private final AsyncConnectionImpl conn;

What is up w/ below?

    this.startLogErrorsCnt = 0;// startLogErrorsCnt;

We take startLogErrorsCnt as a param but ignore it?

You make a new Action from passed-in Action because you don't want to modify passed-in params?

	139	      Action action = new Action(rawAction, i);

super nit: you can presize the following 	148	    this.action2Errors = new IdentityHashMap<>();

Perhaps if TRACE-level logging, log every attempt: 164	    if (tries > startLogErrorsCnt) { ?

Is it right to set this to WARN since it might succeed on next attempt? 

LOG.warn("Process batch for "

... maybe I'm reading it wrong though?

nit: give this method a better name:

	174	  private String getExtras(ServerName serverName) {
175	    return serverName != null ? serverName.getServerName() : "";
176	  }

YOu should use the above method here?

4	        serverName != null ? serverName.toString() : ""));

This is just to log? 208	    long currentTime = System.currentTimeMillis();   i.e. all timing is with nanos but millis is just for logging?

This is a crazy amount of work! I like how this patch is getting better on each iteration; i.e. 	  public MultiGetCallerBuilder multiGet() { becomes   public BatchCallerBuilder batch() {

Skimmed after reading 1/4.

What do you see AsyncBatchRpcRetryingCaller replacing in our current stack? It seems to do AP and a bunch of our Callable infra. Should AsyncBatchRpcRetryingCaller  implement Callable? Or what you thinking?

Generally no * imports

1	import static org.apache.hadoop.hbase.client.ConnectionUtils.*;

Why we have AsyncTable and AsyncTableBase again? Do we have to have the two Interfaces?

Do you have to rename TestAsyncGetMultiThread ?  And/or TestAsyncTableMultiGet?

This is nice work


> Implement batch
> ---------------
>
>                 Key: HBASE-17345
>                 URL: https://issues.apache.org/jira/browse/HBASE-17345
>             Project: HBase
>          Issue Type: Sub-task
>          Components: asyncclient, Client
>    Affects Versions: 2.0.0
>            Reporter: Duo Zhang
>            Assignee: Duo Zhang
>             Fix For: 2.0.0
>
>         Attachments: HBASE-17345.patch
>
>
> Add the support for general batch based on the code introduced in HBASE-17142.


--
This message was sent by Atlassian JIRA
(v6.3.4#6332)