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

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

I think we also need to cover checkAndMutate - it has the same logic as mutate. 
Otherwise, yeah, this seems like an improvement. Couldn't we also drop the 
StatsTrackingRpcRetryingCaller then as well?

Minor nit, IMO its a bit 'nicer' to use static checking (especially in IDEs 
that don't catch strings when doing renames) for the error handling, so 
something like:
{code}
    } catch(RetriesExhaustedWithDetailsException e) {
      for(Throwable rootCause: e.getCauses()){
        if(rootCause instanceof NoSuchColumnFamilyException){
          return;
        }
      }
      throw e;
{code}

bq. I found two difference point between original code and the patch.

It doesn't seem to far fetch though to add support with another 2 parameters 
when doing the submission to the async process. If not, at the very least we 
could refactor the mutate/checkAndMutate code to use the same path and fix the 
stats collecting there

> 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
>         Attachments: HBASE-14703-async.patch, HBASE-14703-start.patch, 
> HBASE-14703.patch, HBASE-14703_v1.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