Dan Burkert has posted comments on this change. Change subject: [c++ client] re-acquire authn token if expired ......................................................................
Patch Set 8: (7 comments) http://gerrit.cloudera.org:8080/#/c/6648/8/src/kudu/client/meta_cache.cc File src/kudu/client/meta_cache.cc: Line 704: if (new_status.ok()) { This can be kept the same as it was. http://gerrit.cloudera.org:8080/#/c/6648/5/src/kudu/client/scanner-internal.cc File src/kudu/client/scanner-internal.cc: Line 134: c->data_->messenger_->reset_authn_token(); > There would be a race if any pair of methods like KuduScanner::{OpenTable() I meant a race between two threads with two separate KuduScanner instances from the same client (which I think is allowed). http://gerrit.cloudera.org:8080/#/c/6648/8/src/kudu/client/scanner-internal.h File src/kudu/client/scanner-internal.h: PS8, Line 63: athentication typo http://gerrit.cloudera.org:8080/#/c/6648/8/src/kudu/integration-tests/authn_token_expire-itest.cc File src/kudu/integration-tests/authn_token_expire-itest.cc: PS8, Line 71: tsrevers typo http://gerrit.cloudera.org:8080/#/c/6648/8/src/kudu/rpc/reactor.h File src/kudu/rpc/reactor.h: Line 185: std::unique_ptr<ErrorStatusPB> rpc_error); Are these PBs wrapped in a unique_ptr just so they can be moved? That's fine if they are, just trying to understand. http://gerrit.cloudera.org:8080/#/c/6648/8/src/kudu/rpc/retriable_rpc.h File src/kudu/rpc/retriable_rpc.h: PS8, Line 92: autentication typo PS8, Line 209: resp_.Clear(); : current_ = nullptr; Are these two lines necessary for the token retry case as well? -- To view, visit http://gerrit.cloudera.org:8080/6648 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I418497ac59cfd4e476e9bfc6abe6b10b487712f5 Gerrit-PatchSet: 8 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: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
