Adar Dembo has posted comments on this change.

Change subject: KUDU-1358 (part 2): heartbeat to every master
......................................................................


Patch Set 9:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/3610/9/src/kudu/tserver/heartbeater.cc
File src/kudu/tserver/heartbeater.cc:

Line 164:   mutable std::atomic_int next_report_seq_;
> hrm, this really has to be mutable?
For the tablet report generation functions, yes. But as discussed below, it's 
probably less confusing to mark them non-const and remove this.


Line 522:     TabletReportState state;
> maybe use = { seqno }; here? that way you'll get a warning at some point la
Done


PS9, Line 528: const 
> maybe makes more sense for this to not be 'const' since it changes the sequ
I agree that mutable and const here is kind of "cheating". Will remove both.


http://gerrit.cloudera.org:8080/#/c/3610/9/src/kudu/tserver/heartbeater.h
File src/kudu/tserver/heartbeater.h:

Line 57:   // not dirty once the report has been acknowledged by every master.
> this makes it sound like it will keep reporting as "dirty" to all masters u
Yeah. I'll clarify the comment.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic85ac4193462d21c989dbd7874b451e8eaab8e3e
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to