[ https://issues.apache.org/jira/browse/HBASE-14521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14955354#comment-14955354 ]
Nicolas Liochon edited comment on HBASE-14521 at 10/13/15 5:52 PM: ------------------------------------------------------------------- Yep [~carp84], I think your analysis is correct: it was a workaround. While looking again at the patch, I found a typo that I will fix on commit > public RetriesExhaustedException(final int numReries, I'm +1, I will commit on master tomorrow my time if nobody disagrees. was (Author: nkeywal): Yep [~carp84], I think your analysis is correct: it was a workaround. While looking again at the patch, I found a typo that I will fix on commit > public RetriesExhaustedException(final int numReries, I'm +1, I will commit on branch2 tomorrow my time if nobody disagrees. > Unify the semantic of hbase.client.retries.number > ------------------------------------------------- > > Key: HBASE-14521 > URL: https://issues.apache.org/jira/browse/HBASE-14521 > Project: HBase > Issue Type: Bug > Affects Versions: 0.98.14, 1.1.2 > Reporter: Yu Li > Assignee: Yu Li > Fix For: 2.0.0, 1.3.0 > > Attachments: HBASE-14521.patch, HBASE-14521_v2.patch, > HBASE-14521_v3.patch > > > From name of the _hbase.client.retries.number_ property, it should be the > number of maximum *retries*, or say if we set the property to 1, there should > be 2 attempts in total. However, there're two different semantics when using > it in current code base. > For example, in ConnectionImplementation#locateRegionInMeta: > {code} > int localNumRetries = (retry ? numTries : 1); > for (int tries = 0; true; tries++) { > if (tries >= localNumRetries) { > throw new NoServerForRegionException("Unable to find region for " > + Bytes.toStringBinary(row) + " in " + tableName + > " after " + numTries + " tries."); > } > {code} > the retries number is regarded as max times for *tries* > While in RpcRetryingCallerImpl#callWithRetries: > {code} > for (int tries = 0;; tries++) { > long expectedSleep; > try { > callable.prepare(tries != 0); // if called with false, check table > status on ZK > interceptor.intercept(context.prepare(callable, tries)); > return callable.call(getRemainingTime(callTimeout)); > } catch (PreemptiveFastFailException e) { > throw e; > } catch (Throwable t) { > ... > if (tries >= retries - 1) { > throw new RetriesExhaustedException(tries, exceptions); > } > {code} > it's regarded as exactly for *REtry* (try a call first with no condition and > then check whether to retry or exceeds maximum retry number) > This inconsistency will cause misunderstanding in usage, such as one of our > customer set the property to zero expecting one single call but finally > received NoServerForRegionException. > We should unify the semantic of the property, and I suggest to keep the > original one for retry rather than total tries. -- This message was sent by Atlassian JIRA (v6.3.4#6332)