Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11449 )

Change subject: KUDU-2580 [c++ client] authn token reacquisition fix
......................................................................


Patch Set 1:

(4 comments)

If you want to backport it to older branches, I suggest making a more pristine 
patch with the various bits of cleanup removed.

http://gerrit.cloudera.org:8080/#/c/11449/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11449/1//COMMIT_MSG@9
PS1, Line 9: Updated the authn token reacquisition logic to handle the following
Could you add more detail here to make it more obvious why this leads to a 
failure? We talked about it over lunch yesterday and only at the very end of 
the discussion did I really understand what was happening.


http://gerrit.cloudera.org:8080/#/c/11449/1/src/kudu/client/client-internal.h
File src/kudu/client/client-internal.h:

http://gerrit.cloudera.org:8080/#/c/11449/1/src/kudu/client/client-internal.h@165
PS1, Line 165:   void ReconnectToCluster(
Doc this. Maybe move the definition of ReconnectionReason down here too, since 
it's only relevant for this function.


http://gerrit.cloudera.org:8080/#/c/11449/1/src/kudu/client/client-internal.h@166
PS1, Line 166:       const MonoTime& deadline,
             :       KuduClient* client,
Nit: can you switch the argument order of these two, to reduce the delta 
between the new code and old code?


http://gerrit.cloudera.org:8080/#/c/11449/1/src/kudu/client/client-internal.cc
File src/kudu/client/client-internal.cc:

http://gerrit.cloudera.org:8080/#/c/11449/1/src/kudu/client/client-internal.cc@158
PS1, Line 158: Status KuduClient::Data::SyncLeaderMasterRpc(
So to repro the bug, which code branches down here are taken, in what order?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4477d0f2bb36ee5ef580b585cae189a634d5002f
Gerrit-Change-Number: 11449
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <[email protected]>
Gerrit-Comment-Date: Tue, 18 Sep 2018 16:54:03 +0000
Gerrit-HasComments: Yes

Reply via email to