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

Jesse Yates commented on HBASE-14703:
-------------------------------------

I think that right now, its not the best solution. The cost comes not 
immediately, but suppose we don't finish changing the other interfaces. Then we 
have mutateRows and checkAndMutate being the only two cases where we use the 
new class and they way it looks is substantially different than every other 
method call. By keeping it simple, it makes it easier for people to understand 
what is happening as it is all written out and not hidden away.

We should either be refactoring or changing behavior; when you combine both its 
hard to understand what the 'correct' solution is and makes it harder for 
others to grok the change. In this case, we are making a functionality change, 
so we shouldn't be too changing how things are organized (note 
PayloadCarryingCallable is a _functionality_ change as we needed a new base 
class to run the mutate callables). I think we should be trying for the 
smallest possible change to accomplish our goals.

Is it so bad to just hold off on adding the new layer until we do the other 
methods as part of async? I bet we will actually figure out that we want a 
different heriarchy and mechanism for managing the callables once we look at it 
holistically, which will make the work here pointless. If I'm wrong, then we 
can just put the class from here and use it directly.

Fair enough?

> not collect stats when call HTable.mutateRow 
> ---------------------------------------------
>
>                 Key: HBASE-14703
>                 URL: https://issues.apache.org/jira/browse/HBASE-14703
>             Project: HBase
>          Issue Type: Bug
>            Reporter: Heng Chen
>            Assignee: Heng Chen
>             Fix For: 2.0.0
>
>         Attachments: HBASE-14702_v5.2_addendum-addendum.patch, 
> HBASE-14703-5.2-addendum.patch, HBASE-14703-async.patch, 
> HBASE-14703-start.patch, HBASE-14703-v4.1.patch, HBASE-14703-v4.patch, 
> HBASE-14703-v6_with-check-and-mutate.patch, HBASE-14703.patch, 
> HBASE-14703_v1.patch, HBASE-14703_v2.patch, HBASE-14703_v3.patch, 
> HBASE-14703_v5.1.patch, HBASE-14703_v5.2.patch, HBASE-14703_v5.patch, 
> HBASE-14703_v6-addendum.patch, HBASE-14703_v6.patch, HBASE-14703_v7.patch
>
>
> In {{AsyncProcess.SingleServerRequestRunnable}}, it seems we update 
> serverStatistics twice.
> The first one is that we wrapper {{RetryingCallable}}  by 
> {{StatsTrackingRpcRetryingCaller}}, and do serverStatistics update when we 
> call {{callWithRetries}} and {{callWithoutRetries}}. Relates code like below:
> {code}
>   @Override
>   public T callWithRetries(RetryingCallable<T> callable, int callTimeout)
>       throws IOException, RuntimeException {
>     T result = delegate.callWithRetries(callable, callTimeout);
>     return updateStatsAndUnwrap(result, callable);
>   }
>   @Override
>   public T callWithoutRetries(RetryingCallable<T> callable, int callTimeout)
>       throws IOException, RuntimeException {
>     T result = delegate.callWithRetries(callable, callTimeout);
>     return updateStatsAndUnwrap(result, callable);
>   }
> {code}
> The secondary one is after we get response, in {{receiveMultiAction}}, we do 
> update again. 
> {code}
> // update the stats about the region, if its a user table. We don't want to 
> slow down
> // updates to meta tables, especially from internal updates (master, etc).
> if (AsyncProcess.this.connection.getStatisticsTracker() != null) {
>   result = ResultStatsUtil.updateStats(result,
>   AsyncProcess.this.connection.getStatisticsTracker(), server, regionName);
> }
> {code}
> It seems that {{StatsTrackingRpcRetryingCaller}} is NOT necessary,  remove it?



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

Reply via email to