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