Dan Burkert has posted comments on this change. Change subject: [client] avoid circular deps in time-based flusher ......................................................................
Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/4395/1/src/kudu/client/session-internal.cc File src/kudu/client/session-internal.cc: Line 514: if (PREDICT_TRUE(messenger)) { This can't ever be false, right? I think the callstack of this method includes the KuduSession which has a strong reference. Line 517: _1, weak_messenger, weak_session, false), std::move both messenger and session. http://gerrit.cloudera.org:8080/#/c/4395/1/src/kudu/client/session-internal.h File src/kudu/client/session-internal.h: Line 58: const sp::weak_ptr<rpc::Messenger>& messenger); I think this should be std::, and why use a reference? -- To view, visit http://gerrit.cloudera.org:8080/4395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
