Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9176 )

Change subject: KUDU-2279 (part 2): metrics: only emit changed metrics in 
metrics log
......................................................................


Patch Set 2:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/9176/2/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:

http://gerrit.cloudera.org:8080/#/c/9176/2/src/kudu/server/server_base.cc@630
PS2, Line 630: opts.only_modified_since_epoch = prev_log_epoch + 1;
             :     int64_t this_log_epoch = Metric::current_epoch();
             :     Metric::IncrementEpoch();
> the reason for the 'prev_log_epoch' thing is that, in the odd case that we
Done


http://gerrit.cloudera.org:8080/#/c/9176/2/src/kudu/util/metrics-test.cc
File src/kudu/util/metrics-test.cc:

http://gerrit.cloudera.org:8080/#/c/9176/2/src/kudu/util/metrics-test.cc@332
PS2, Line 332: epoch_before_modify
> This is the epoch of/containing the modification, not the epoch before.
Done


http://gerrit.cloudera.org:8080/#/c/9176/2/src/kudu/util/metrics-test.cc@347
PS2, Line 347: epoch_before_modify
> This should be `new_epoch`.
Done


http://gerrit.cloudera.org:8080/#/c/9176/2/src/kudu/util/metrics.h
File src/kudu/util/metrics.h:

http://gerrit.cloudera.org:8080/#/c/9176/2/src/kudu/util/metrics.h@567
PS2, Line 567: it's
> Remove.
Done


http://gerrit.cloudera.org:8080/#/c/9176/2/src/kudu/util/metrics.h@570
PS2, Line 570: base::subtle::NoBarrier_Load(&m_epoch_)
> will see if I can switch. I get nervous abotu the C++11 atomics because I d
Done


http://gerrit.cloudera.org:8080/#/c/9176/2/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:

http://gerrit.cloudera.org:8080/#/c/9176/2/src/kudu/util/metrics.cc@357
PS2, Line 357: Metric::IncrementEpoch();
> hm, yea I think this can just be removed. I think it's a holdover from a pr
Done


http://gerrit.cloudera.org:8080/#/c/9176/2/src/kudu/util/metrics.cc@499
PS2, Line 499: 1
> will see if I can do that.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia26be99a1fa96d52e2ca0905844d56c096d3778e
Gerrit-Change-Number: 9176
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <t...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com>
Gerrit-Comment-Date: Sat, 03 Feb 2018 01:49:40 +0000
Gerrit-HasComments: Yes

Reply via email to