Dan Burkert has posted comments on this change. Change subject: [java] KUDU-2013 re-acquire authn token if expired ......................................................................
Patch Set 12: (3 comments) http://gerrit.cloudera.org:8080/#/c/7250/12/java/kudu-client/src/main/java/org/apache/kudu/client/AuthnTokenReacquirer.java File java/kudu-client/src/main/java/org/apache/kudu/client/AuthnTokenReacquirer.java: Line 37: public static final class AffectedRpcInfo { > I keep the exception due to the 'technical' reasons: it's needed to retry t Right, so I'm worried this is actually going to distort the traces. First off, the timestamp is no longer going to correspond to the actual failure time. I think we may want to add a new trace state ('queued waiting for token acquire' or something), so that we can see that in RPC traces, and marked with timestamp of when it's popped from the queue. As far as having to pass the exception to handleRetryableErrorNoDelay, isn't that a method that is only called here? Seems fixable. Line 145: * Errback to retry authn token re-acquisition and notify the handle the affected RPCs if the > That's a good idea. What if I just it just a TODO here for now? That's fine with me. http://gerrit.cloudera.org:8080/#/c/7250/12/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java File java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java: Line 669: NEGOTIATION_FAILED, > Yes, we could do that. But If possible it would be nice to do in this commit, because this commit is introducing the state. If it proves to be more complex I'm fine skipping it though (but I suspect it will make the control flow simpler). -- To view, visit http://gerrit.cloudera.org:8080/7250 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0be620629c9a8345ecd5e5679c80ee76ca4eaa57 Gerrit-PatchSet: 12 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-HasComments: Yes
