Alexey Serbin has posted comments on this change. Change subject: [client] avoid circular deps in time-based flusher ......................................................................
Patch Set 1: (4 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)) { > Can't it be false if it's called out of ScheduleOnReactor()? Ah, right observation. And it will get at this point only in case of AUTO_FLUSH_BACKGROUND mode, btw. Line 517: _1, weak_messenger, weak_session, false), > Yeah, it doesn't affect correctness. ok, thanks! 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 agree with Dan that it'd be cleanest to pass the messenger and the client Done PS1, Line 161: // The reference to the client's messenger (keeping the reference instead of : // declaring friendship to KuduClient and accessing it via the client_). > Should update this comment to explain why it's a weak pointer. Will add some info into the comment, OK. I have the following reasoning: we pass it around as a weak reference anyway, and then convert into a shared one. Why would we store it as shared? -- 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: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
