[jira] [Commented] (HBASE-14521) Unify the semantic of hbase.client.retries.number

2015-10-15 Thread Hudson (JIRA)

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

Hudson commented on HBASE-14521:


SUCCESS: Integrated in HBase-TRUNK #6915 (See 
[https://builds.apache.org/job/HBase-TRUNK/6915/])
HBASE-14521 Unify the semantic of hbase.client.retries.number (Yu Li) (nkeywal: 
rev e7defd7d9a76f44e3089db3fe522fe400fe6dcd7)
* 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/RetriesExhaustedException.java
* hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncProcess.java
* 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTableMultiplexer.java
* 
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMasterCommandLine.java
* hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHCM.java
* 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/RpcRetryingCallerImpl.java
* 
hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncProcess.java
* 
hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestBlockEvictionFromClient.java
* 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java


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


[jira] [Commented] (HBASE-14521) Unify the semantic of hbase.client.retries.number

2015-10-15 Thread Yu Li (JIRA)

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

Yu Li commented on HBASE-14521:
---

Thanks for review and help commit [~nkeywal]!

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


[jira] [Commented] (HBASE-14521) Unify the semantic of hbase.client.retries.number

2015-10-13 Thread Yu Li (JIRA)

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

Yu Li commented on HBASE-14521:
---

{quote}
I found a typo that I will fix on commit
{quote}
Yes please, thanks for the careful review [~nkeywal]!

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


[jira] [Commented] (HBASE-14521) Unify the semantic of hbase.client.retries.number

2015-10-13 Thread Nicolas Liochon (JIRA)

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

Nicolas Liochon commented on HBASE-14521:
-

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)


[jira] [Commented] (HBASE-14521) Unify the semantic of hbase.client.retries.number

2015-10-09 Thread Yu Li (JIRA)

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

Yu Li commented on HBASE-14521:
---

Thanks for the comments Nicolas.

{quote}
We should continue to count the calls, no?
{quote}
>From codes in AsyncProcess, after getting AsyncRequestFutureImpl instance from 
>createAsyncRequestFuture call, the code flow is like:
{noformat}
AsyncRequestFutureImpl#sendMultiAction->SingleServerRequestRunnable#run
{noformat}
where it will createCaller first and invoke callWithoutRetries to issue the 
real call:
{code}
try {
  callable = createCallable(server, tableName, multiAction);
  try {
RpcRetryingCaller caller = createCaller(callable);
if (callsInProgress != null) callsInProgress.add(callable);
res = caller.callWithoutRetries(callable, timeout);
{code}
Actually the ideal way is to increase the callsCt inside callWithoutRetries, 
but it's also ok to do the increment in createCaller, just as we do in 
TestAsyncProcess.
If the above analysis is correct (plz correct me if I stated anything wrong), 
then the count of calls in createAsyncRequestFuture is redundant. I'd also 
regard it as a trick to work around the inconsistency of 
hbase.client.retries.number before.

{quote}
Note that setting retries to zero is most of the time an error as we can have a 
retry in many cases, for example iif the client cache is not up to date 
(contains the wrong region server for a region).
{quote}
Agree that retry is necessary for client cache update, and we shouldn't set 
retries to zero under most common cases, but in some corner cases we may still 
don't want any retry, just like we did in some of the UT cases. Let me add a 
line in the release note.

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


[jira] [Commented] (HBASE-14521) Unify the semantic of hbase.client.retries.number

2015-10-09 Thread Nicolas Liochon (JIRA)

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

Nicolas Liochon commented on HBASE-14521:
-

It's a good point: the existing implementation is confusing.
The patch looks good. It contains a lot of cleanup that will make the code 
easier to read (thanks, Yu!)

I'm surprised by this:
{code}
@@ -137,7 +137,6 @@ public class TestAsyncProcess {
   AsyncRequestFutureImpl r = super.createAsyncRequestFuture(
   DUMMY_TABLE, actions, nonceGroup, pool, callback, results, 
needResults);
   allReqs.add(r);
-  callsCt.incrementAndGet();  <=== We should continue to count the 
calls, no?
   return r;
 }
{code}

Note that setting retries to zero is most of the time an error as we can have a 
retry in many cases, for example iif the client cache is not up to date 
(contains the wrong region server for a region). 

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


[jira] [Commented] (HBASE-14521) Unify the semantic of hbase.client.retries.number

2015-10-08 Thread Hadoop QA (JIRA)

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

Hadoop QA commented on HBASE-14521:
---

{color:green}+1 overall{color}.  Here are the results of testing the latest 
attachment 
  http://issues.apache.org/jira/secure/attachment/12765535/HBASE-14521_v3.patch
  against master branch at commit 7e30436e3fa84525b85b05b9e23cb01b2ada7c12.
  ATTACHMENT ID: 12765535

{color:green}+1 @author{color}.  The patch does not contain any @author 
tags.

{color:green}+1 tests included{color}.  The patch appears to include 9 new 
or modified tests.

{color:green}+1 hadoop versions{color}. The patch compiles with all 
supported hadoop versions (2.4.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.6.1 2.7.0 
2.7.1)

{color:green}+1 javac{color}.  The applied patch does not increase the 
total number of javac compiler warnings.

{color:green}+1 protoc{color}.  The applied patch does not increase the 
total number of protoc compiler warnings.

{color:green}+1 javadoc{color}.  The javadoc tool did not generate any 
warning messages.

{color:green}+1 checkstyle{color}.  The applied patch does not increase the 
total number of checkstyle errors

{color:green}+1 findbugs{color}.  The patch does not introduce any  new 
Findbugs (version 2.0.3) warnings.

{color:green}+1 release audit{color}.  The applied patch does not increase 
the total number of release audit warnings.

{color:green}+1 lineLengths{color}.  The patch does not introduce lines 
longer than 100

  {color:green}+1 site{color}.  The mvn post-site goal succeeds with this patch.

{color:green}+1 core tests{color}.  The patch passed unit tests in .

Test results: 
https://builds.apache.org/job/PreCommit-HBASE-Build/15914//testReport/
Release Findbugs (version 2.0.3)warnings: 
https://builds.apache.org/job/PreCommit-HBASE-Build/15914//artifact/patchprocess/newFindbugsWarnings.html
Checkstyle Errors: 
https://builds.apache.org/job/PreCommit-HBASE-Build/15914//artifact/patchprocess/checkstyle-aggregate.html

  Console output: 
https://builds.apache.org/job/PreCommit-HBASE-Build/15914//console

This message is automatically generated.

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


[jira] [Commented] (HBASE-14521) Unify the semantic of hbase.client.retries.number

2015-10-07 Thread Hadoop QA (JIRA)

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

Hadoop QA commented on HBASE-14521:
---

{color:red}-1 overall{color}.  Here are the results of testing the latest 
attachment 
  http://issues.apache.org/jira/secure/attachment/12765500/HBASE-14521_v2.patch
  against master branch at commit 7e30436e3fa84525b85b05b9e23cb01b2ada7c12.
  ATTACHMENT ID: 12765500

{color:green}+1 @author{color}.  The patch does not contain any @author 
tags.

{color:green}+1 tests included{color}.  The patch appears to include 9 new 
or modified tests.

{color:green}+1 hadoop versions{color}. The patch compiles with all 
supported hadoop versions (2.4.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.6.1 2.7.0 
2.7.1)

{color:green}+1 javac{color}.  The applied patch does not increase the 
total number of javac compiler warnings.

{color:green}+1 protoc{color}.  The applied patch does not increase the 
total number of protoc compiler warnings.

{color:green}+1 javadoc{color}.  The javadoc tool did not generate any 
warning messages.

{color:green}+1 checkstyle{color}.  The applied patch does not increase the 
total number of checkstyle errors

{color:green}+1 findbugs{color}.  The patch does not introduce any  new 
Findbugs (version 2.0.3) warnings.

{color:green}+1 release audit{color}.  The applied patch does not increase 
the total number of release audit warnings.

{color:green}+1 lineLengths{color}.  The patch does not introduce lines 
longer than 100

  {color:green}+1 site{color}.  The mvn post-site goal succeeds with this patch.

 {color:red}-1 core tests{color}.  The patch failed these unit tests:
   org.apache.hadoop.hbase.client.TestAsyncProcess

Test results: 
https://builds.apache.org/job/PreCommit-HBASE-Build/15909//testReport/
Release Findbugs (version 2.0.3)warnings: 
https://builds.apache.org/job/PreCommit-HBASE-Build/15909//artifact/patchprocess/newFindbugsWarnings.html
Checkstyle Errors: 
https://builds.apache.org/job/PreCommit-HBASE-Build/15909//artifact/patchprocess/checkstyle-aggregate.html

  Console output: 
https://builds.apache.org/job/PreCommit-HBASE-Build/15909//console

This message is automatically generated.

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