[kudu-CR] WIP [java] re-acquire authn token if expired

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: WIP [java] re-acquire authn token if expired
..


Patch Set 2:

(3 comments)

> I didn't review this but I filed a JIRA for this feature: KUDU-2013

Thanks.  Having a JIRA item makes it easier to track.

http://gerrit.cloudera.org:8080/#/c/6816/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

Line 1019: 
> In general I'm not a fan of cleanups like that that have nothing to do with
Yep, that makes sense -- let me publish it in a separate changelist.  I was not 
sure it was worth having it as a separate one, but now it's clear that's the 
way.


http://gerrit.cloudera.org:8080/#/c/6816/2/java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java:

Line 383: lock.lock();
> Minor but that should be outside of try in case any unchecked exception get
Done


PS2, Line 407: state = State.POSTPONED_DISCONNECT;
 : lock.unlock();
> Wrap in try/catch.
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/6816
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad5bef1d06708215839037741cd3bc213e130af2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] WIP [java] re-acquire authn token if expired

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: WIP [java] re-acquire authn token if expired
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/6816/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

Line 1019: 
In general I'm not a fan of cleanups like that that have nothing to do with the 
patch as it makes it more likely to conflict with other patches that actually 
have something to do with the part of the code that's being cleaned up.


Line 1327:   // TODO(aserbin): synchronize setting the token between 
different actors.
I think what we should do for ConnectToCluster is to gate all the different 
threads on the taking of some atomic object using CAS. Look at how we use the 
field `flushNotification` in AsyncKuduSession.


http://gerrit.cloudera.org:8080/#/c/6816/2/java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java:

Line 383: lock.lock();
Minor but that should be outside of try in case any unchecked exception gets 
thrown and then you might not have locked.


PS2, Line 407: state = State.POSTPONED_DISCONNECT;
 : lock.unlock();
Wrap in try/catch.


-- 
To view, visit http://gerrit.cloudera.org:8080/6816
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad5bef1d06708215839037741cd3bc213e130af2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] WIP [java] re-acquire authn token if expired

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: WIP [java] re-acquire authn token if expired
..


Patch Set 2: Verified+1

Unrelated MultiThreadedTabletTest/4.DeleteAndReinsert flake

-- 
To view, visit http://gerrit.cloudera.org:8080/6816
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad5bef1d06708215839037741cd3bc213e130af2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] WIP [java] re-acquire authn token if expired

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/6816

Change subject: WIP [java] re-acquire authn token if expired
..

WIP [java] re-acquire authn token if expired

This patch introduces automatic authn token re-acquisition when current
authn token expires.  The client automatically retries the RPC that
hits the token expiration error.

Added a test to exercise the new retry logic for token expiration in
case of a basic workload scenario.

In this commit I also added a few lines of code to retry RPC for the
case if server sends ERROR_UNAVAILABLE error code (which is a broader
version of its ERROR_SERVER_TOO_BUSY counterpart).

WIP: there are a couple of TODOs to address
  * synchronization of setting/resetting authn token in the client
context
  * a test scenario with more exhaustive workload

Change-Id: Iad5bef1d06708215839037741cd3bc213e130af2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
A 
java/kudu-client/src/test/java/org/apache/kudu/client/TestAuthnTokenReacquire.java
4 files changed, 231 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/6816/1
-- 
To view, visit http://gerrit.cloudera.org:8080/6816
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iad5bef1d06708215839037741cd3bc213e130af2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin