You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Hao Hao (Code Review)" <ge...@cloudera.org> on 2018/01/20 01:22:56 UTC
[kudu-CR] KUDU-2262: allow Java client to retry if no master is a leader
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9088
Change subject: KUDU-2262: allow Java client to retry if no master is a leader
......................................................................
KUDU-2262: allow Java client to retry if no master is a leader
Currently, when authentication is required, the Java client will not
retry if encounters NOT_AUTHORIZED error. This cause a request to fail
in cases when masters were in the process of a leader selection even
the client has valid authentication tokens, since a non-leader master
only has self-signed cert if it has never been run as a leader
(KUDU-2265).
This patch makes NOT_AUTHORIZED a recoverable exception and retry the
request as long as the original call has not timed out. Corresponding
test is added in TestSecurity.
Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
A java/kudu-client/src/main/java/org/apache/kudu/client/NotAuthorizedException.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
3 files changed, 83 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/9088/1
--
To view, visit http://gerrit.cloudera.org:8080/9088
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
Gerrit-Change-Number: 9088
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
[kudu-CR] KUDU-2262: allow Java client to retry if no master is a leader
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9088 )
Change subject: KUDU-2262: allow Java client to retry if no master is a leader
......................................................................
Patch Set 1:
(8 comments)
Just some nits on the description of the patch.
http://gerrit.cloudera.org:8080/#/c/9088/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/9088/1//COMMIT_MSG@10
PS1, Line 10: cause
nit: causes
http://gerrit.cloudera.org:8080/#/c/9088/1//COMMIT_MSG@11
PS1, Line 11: a
nit: drop
http://gerrit.cloudera.org:8080/#/c/9088/1//COMMIT_MSG@11
PS1, Line 11: selection
nit: election
http://gerrit.cloudera.org:8080/#/c/9088/1//COMMIT_MSG@11
PS1, Line 11: even
nit: even if
http://gerrit.cloudera.org:8080/#/c/9088/1//COMMIT_MSG@11
PS1, Line 11: were
nit: are
http://gerrit.cloudera.org:8080/#/c/9088/1//COMMIT_MSG@11
PS1, Line 11: leader
I think that can happen only during the very first leader election after masters have been started up, right? During second and next elections, at least one master should have CA-signed TLS server certificate, right?
http://gerrit.cloudera.org:8080/#/c/9088/1//COMMIT_MSG@12
PS1, Line 12: tokens
nit: token
http://gerrit.cloudera.org:8080/#/c/9088/1//COMMIT_MSG@16
PS1, Line 16: This patch makes NOT_AUTHORIZED a recoverable exception and retry the
: request as long as the original call has not timed out. Corresponding
: test is added in TestSecurity.
Could you update this piece to reflect the actual logic of the code (as in the latest revision)?
--
To view, visit http://gerrit.cloudera.org:8080/9088
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
Gerrit-Change-Number: 9088
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 24 Jan 2018 19:49:19 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2262: allow Java client to retry if no master is a leader
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9088 )
Change subject: KUDU-2262: allow Java client to retry if no master is a leader
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9088/1/java/kudu-client/src/main/java/org/apache/kudu/client/NotAuthorizedException.java
File java/kudu-client/src/main/java/org/apache/kudu/client/NotAuthorizedException.java:
http://gerrit.cloudera.org:8080/#/c/9088/1/java/kudu-client/src/main/java/org/apache/kudu/client/NotAuthorizedException.java@23
PS1, Line 23: /**
: * Exception indicating that the request failed because is not authorized.
: * This is a recoverable exception. So the request can be retried as long
: * as the original call hasn't timed out, for cases documented in KUDU-2267,
: * e.g. masters are in the process of leader election and does not have
: * CA signed cert.
: */
: @InterfaceAudience.Private
: @InterfaceStability.Evolving
: final class NotAuthorizedException extends RecoverableException {
> And another resolution would be just not retrying in such case at all, sin
I agree with Alexey that the majority of NotAuthorized errors should be non-recoverable. I think it would be OK to change the specific case of 'client has valid 'secondary' authn credentials (such as authn token), but it does not have 'primary' authn credentials (such as Kerberos creds)', and should be straightforward, I think it only happens in one place: https://github.com/apache/kudu/blob/master/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java#L422
--
To view, visit http://gerrit.cloudera.org:8080/9088
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
Gerrit-Change-Number: 9088
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 24 Jan 2018 01:49:38 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2262: allow Java client to retry if no master is a leader
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/9088 )
Change subject: KUDU-2262: allow Java client to retry if no master is a leader
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9088/2/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java:
http://gerrit.cloudera.org:8080/#/c/9088/2/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@429
PS2, Line 429: if (authnToken != null) {
> nit: add braces around both clauses.
Done
--
To view, visit http://gerrit.cloudera.org:8080/9088
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
Gerrit-Change-Number: 9088
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 24 Jan 2018 19:42:11 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2262: allow Java client to retry if no master is a leader
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9088 )
Change subject: KUDU-2262: allow Java client to retry if no master is a leader
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9088/1/java/kudu-client/src/main/java/org/apache/kudu/client/NotAuthorizedException.java
File java/kudu-client/src/main/java/org/apache/kudu/client/NotAuthorizedException.java:
http://gerrit.cloudera.org:8080/#/c/9088/1/java/kudu-client/src/main/java/org/apache/kudu/client/NotAuthorizedException.java@23
PS1, Line 23: /**
: * Exception indicating that the request failed because is not authorized.
: * This is a recoverable exception. So the request can be retried as long
: * as the original call hasn't timed out, for cases documented in KUDU-2267,
: * e.g. masters are in the process of leader election and does not have
: * CA signed cert.
: */
: @InterfaceAudience.Private
: @InterfaceStability.Evolving
: final class NotAuthorizedException extends RecoverableException {
> Currently we are retrying if non of masters are a leader in case not all re
Yep, I think we should retry on NotAuthorized only if the following is true:
(a) all masters returned NotAuthorized exception
(b) the client has valid 'secondary' authn credentials (such as authn token), but it does not have 'primary' authn credentials (such as Kerberos creds)
However, I'm not sure whether that it's easy and appropriate to check for (b) in the handler.
Another way of addressing the issue would be making some changes on the server side, like dropping connection at the server side if there isn't CA-signed certificate yet, but it's just crazy.
So, maybe simply retrying in such cases is not such a bad idea, after all :)
Any other ideas?
--
To view, visit http://gerrit.cloudera.org:8080/9088
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
Gerrit-Change-Number: 9088
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 24 Jan 2018 00:19:57 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2262: allow Java client to retry if no master is a leader
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Dan Burkert, Kudu Jenkins, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9088
to look at the new patch set (#3).
Change subject: KUDU-2262: allow Java client to retry if no master is a leader
......................................................................
KUDU-2262: allow Java client to retry if no master is a leader
Currently, when authentication is required, the Java client will not
retry if encounters NOT_AUTHORIZED error. This cause a request to fail
in cases when masters were in the process of a leader selection even
the client has valid authentication tokens, since a non-leader master
only has self-signed cert if it has never been run as a leader
(KUDU-2265).
This patch makes NOT_AUTHORIZED a recoverable exception and retry the
request as long as the original call has not timed out. Corresponding
test is added in TestSecurity.
Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
2 files changed, 38 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/9088/3
--
To view, visit http://gerrit.cloudera.org:8080/9088
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
Gerrit-Change-Number: 9088
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2262: allow Java client to retry if no master is a leader
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/9088 )
Change subject: KUDU-2262: allow Java client to retry if no master is a leader
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9088/1/java/kudu-client/src/main/java/org/apache/kudu/client/NotAuthorizedException.java
File java/kudu-client/src/main/java/org/apache/kudu/client/NotAuthorizedException.java:
http://gerrit.cloudera.org:8080/#/c/9088/1/java/kudu-client/src/main/java/org/apache/kudu/client/NotAuthorizedException.java@23
PS1, Line 23: /**
: * Exception indicating that the request failed because is not authorized.
: * This is a recoverable exception. So the request can be retried as long
: * as the original call hasn't timed out, for cases documented in KUDU-2267,
: * e.g. masters are in the process of leader election and does not have
: * CA signed cert.
: */
: @InterfaceAudience.Private
: @InterfaceStability.Evolving
: final class NotAuthorizedException extends RecoverableException {
> I think it's not a very good idea to retry in case if a client gets not aut
Currently we are retrying if non of masters are a leader in case not all received exceptions are nonrecoverable (https://github.com/apache/kudu/blob/master/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java#L264).
If you think making not authorized response a recoverable exception is not a good a idea, maybe another alternative is to check the cause of nonrecoverable exception and retry if it is not authorized response?
--
To view, visit http://gerrit.cloudera.org:8080/9088
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
Gerrit-Change-Number: 9088
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 23 Jan 2018 22:48:47 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2262: allow Java client to retry if no master is a leader
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Dan Burkert, Kudu Jenkins, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9088
to look at the new patch set (#2).
Change subject: KUDU-2262: allow Java client to retry if no master is a leader
......................................................................
KUDU-2262: allow Java client to retry if no master is a leader
Currently, when authentication is required, the Java client will not
retry if encounters NOT_AUTHORIZED error. This cause a request to fail
in cases when masters were in the process of a leader selection even
the client has valid authentication tokens, since a non-leader master
only has self-signed cert if it has never been run as a leader
(KUDU-2265).
This patch makes NOT_AUTHORIZED a recoverable exception and retry the
request as long as the original call has not timed out. Corresponding
test is added in TestSecurity.
Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
2 files changed, 37 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/9088/2
--
To view, visit http://gerrit.cloudera.org:8080/9088
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
Gerrit-Change-Number: 9088
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2262: allow Java client to retry if no master is a leader
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9088 )
Change subject: KUDU-2262: allow Java client to retry if no master is a leader
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9088/1/java/kudu-client/src/main/java/org/apache/kudu/client/NotAuthorizedException.java
File java/kudu-client/src/main/java/org/apache/kudu/client/NotAuthorizedException.java:
http://gerrit.cloudera.org:8080/#/c/9088/1/java/kudu-client/src/main/java/org/apache/kudu/client/NotAuthorizedException.java@23
PS1, Line 23: /**
: * Exception indicating that the request failed because is not authorized.
: * This is a recoverable exception. So the request can be retried as long
: * as the original call hasn't timed out, for cases documented in KUDU-2267,
: * e.g. masters are in the process of leader election and does not have
: * CA signed cert.
: */
: @InterfaceAudience.Private
: @InterfaceStability.Evolving
: final class NotAuthorizedException extends RecoverableException {
I think it's not a very good idea to retry in case if a client gets not authorized response from master: imagine a case when a client does not have proper credentials due to some configuration issue. In that case, it's better to report on such issue right away: the 'fail fast' approach looks more appropriate here.
Maybe, it's better to address that somehow to retry and include a call to all the registered masters instead? In that case we just need to make sure we received a response with from a leader master.
--
To view, visit http://gerrit.cloudera.org:8080/9088
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
Gerrit-Change-Number: 9088
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 23 Jan 2018 19:51:37 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2262: allow Java client to retry if no master is a leader
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9088 )
Change subject: KUDU-2262: allow Java client to retry if no master is a leader
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9088/1/java/kudu-client/src/main/java/org/apache/kudu/client/NotAuthorizedException.java
File java/kudu-client/src/main/java/org/apache/kudu/client/NotAuthorizedException.java:
http://gerrit.cloudera.org:8080/#/c/9088/1/java/kudu-client/src/main/java/org/apache/kudu/client/NotAuthorizedException.java@23
PS1, Line 23: /**
: * Exception indicating that the request failed because is not authorized.
: * This is a recoverable exception. So the request can be retried as long
: * as the original call hasn't timed out, for cases documented in KUDU-2267,
: * e.g. masters are in the process of leader election and does not have
: * CA signed cert.
: */
: @InterfaceAudience.Private
: @InterfaceStability.Evolving
: final class NotAuthorizedException extends RecoverableException {
> Yep, I think we should retry on NotAuthorized only if the following is true
And another resolution would be just not retrying in such case at all, since this is a very rare condition: a client connects to the cluster with previously acquired token but all existing masters are starting up.
I'm not sure that adding workaround for this case and complicating the code is worth it.
--
To view, visit http://gerrit.cloudera.org:8080/9088
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
Gerrit-Change-Number: 9088
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 24 Jan 2018 00:29:11 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2262: allow Java client to retry if no master is a leader
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9088 )
Change subject: KUDU-2262: allow Java client to retry if no master is a leader
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9088/2/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java:
http://gerrit.cloudera.org:8080/#/c/9088/2/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@429
PS2, Line 429: if (authnToken != null)
nit: add braces around both clauses.
--
To view, visit http://gerrit.cloudera.org:8080/9088
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
Gerrit-Change-Number: 9088
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 24 Jan 2018 19:35:55 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2262: allow Java client to retry if no master is a leader
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Dan Burkert, Kudu Jenkins, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9088
to look at the new patch set (#4).
Change subject: KUDU-2262: allow Java client to retry if no master is a leader
......................................................................
KUDU-2262: allow Java client to retry if no master is a leader
Currently, when authentication is required, the Java client will not
retry if encounters NOT_AUTHORIZED error. This causes a request to fail
in cases when masters are in the process of the very first leader
election after started up, even if the client has valid authentication
token. Since the non-leader masters only have self-signed cert.
This patch updates the logic to throw a recoverable exception if client
has valid secondary authn credentials (such as authn token), but it
does not have primary authn credentials (such as Kerberos creds), and
retry the request as long as the original call has not timed out.
Corresponding test is added.
Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
2 files changed, 38 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/9088/4
--
To view, visit http://gerrit.cloudera.org:8080/9088
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
Gerrit-Change-Number: 9088
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2262: allow Java client to retry if no master is a leader
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/9088 )
Change subject: KUDU-2262: allow Java client to retry if no master is a leader
......................................................................
Patch Set 4:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/9088/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/9088/1//COMMIT_MSG@10
PS1, Line 10: cause
> nit: causes
Done
http://gerrit.cloudera.org:8080/#/c/9088/1//COMMIT_MSG@11
PS1, Line 11: irst lead
> nit: election
Done
http://gerrit.cloudera.org:8080/#/c/9088/1//COMMIT_MSG@11
PS1, Line 11: r
> nit: even if
Done
http://gerrit.cloudera.org:8080/#/c/9088/1//COMMIT_MSG@11
PS1, Line 11: h
> nit: drop
Done
http://gerrit.cloudera.org:8080/#/c/9088/1//COMMIT_MSG@11
PS1, Line 11: are
> nit: are
Done
http://gerrit.cloudera.org:8080/#/c/9088/1//COMMIT_MSG@11
PS1, Line 11: very
> I think that can happen only during the very first leader election after ma
Right. updated.
http://gerrit.cloudera.org:8080/#/c/9088/1//COMMIT_MSG@12
PS1, Line 12: he cli
> nit: token
Done
http://gerrit.cloudera.org:8080/#/c/9088/1//COMMIT_MSG@16
PS1, Line 16: has valid secondary authn credentials (such as authn token), but it
: does not have primary authn credentials (such as Kerberos creds), and
: retry the request as long as t
> Could you update this piece to reflect the actual logic of the code (as in
Done
--
To view, visit http://gerrit.cloudera.org:8080/9088
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
Gerrit-Change-Number: 9088
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 24 Jan 2018 20:52:47 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2262: allow Java client to retry if no master is a leader
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9088 )
Change subject: KUDU-2262: allow Java client to retry if no master is a leader
......................................................................
KUDU-2262: allow Java client to retry if no master is a leader
Currently, when authentication is required, the Java client will not
retry if encounters NOT_AUTHORIZED error. This causes a request to fail
in cases when masters are in the process of the very first leader
election after started up, even if the client has valid authentication
token. Since the non-leader masters only have self-signed cert.
This patch updates the logic to throw a recoverable exception if client
has valid secondary authn credentials (such as authn token), but it
does not have primary authn credentials (such as Kerberos creds), and
retry the request as long as the original call has not timed out.
Corresponding test is added.
Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
Reviewed-on: http://gerrit.cloudera.org:8080/9088
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
2 files changed, 38 insertions(+), 2 deletions(-)
Approvals:
Kudu Jenkins: Verified
Todd Lipcon: Looks good to me, but someone else must approve
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/9088
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
Gerrit-Change-Number: 9088
Gerrit-PatchSet: 5
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2262: allow Java client to retry if no master is a leader
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9088 )
Change subject: KUDU-2262: allow Java client to retry if no master is a leader
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/9088
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
Gerrit-Change-Number: 9088
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 24 Jan 2018 22:27:39 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2262: allow Java client to retry if no master is a leader
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9088 )
Change subject: KUDU-2262: allow Java client to retry if no master is a leader
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/9088
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
Gerrit-Change-Number: 9088
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 24 Jan 2018 23:57:11 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2262: allow Java client to retry if no master is a leader
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/9088 )
Change subject: KUDU-2262: allow Java client to retry if no master is a leader
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9088/1/java/kudu-client/src/main/java/org/apache/kudu/client/NotAuthorizedException.java
File java/kudu-client/src/main/java/org/apache/kudu/client/NotAuthorizedException.java:
http://gerrit.cloudera.org:8080/#/c/9088/1/java/kudu-client/src/main/java/org/apache/kudu/client/NotAuthorizedException.java@23
PS1, Line 23: /**
: * Exception indicating that the request failed because is not authorized.
: * This is a recoverable exception. So the request can be retried as long
: * as the original call hasn't timed out, for cases documented in KUDU-2267,
: * e.g. masters are in the process of leader election and does not have
: * CA signed cert.
: */
: @InterfaceAudience.Private
: @InterfaceStability.Evolving
: final class NotAuthorizedException extends RecoverableException {
> I agree with Alexey that the majority of NotAuthorized errors should be non
Sounds good to me, I will update based on the suggestion.
--
To view, visit http://gerrit.cloudera.org:8080/9088
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia39a8d77cbf58c6f2f1f97eaf5e2e17ac1fa09fa
Gerrit-Change-Number: 9088
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 24 Jan 2018 04:20:06 +0000
Gerrit-HasComments: Yes