You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2017/04/06 23:45:38 UTC

[kudu-CR] linked list-test: dump a histogram of Update performance

Hello David Ribeiro Alves,

I'd like you to do a code review.  Please visit

    http://gerrit.cloudera.org:8080/6576

to review the following change.

Change subject: linked_list-test: dump a histogram of Update performance
......................................................................

linked_list-test: dump a histogram of Update performance

This makes the updater thread in linked_list-test update a histogram of
its latencies before exiting.

In order to get accurate latency readings, I had to switch it back to
MANUAL_FLUSH mode.

This was useful in diagnosing an UPDATE performance regression in a
recent patch.

Change-Id: I6d82d495d55091ee597995c2e5963c573401f624
---
M src/kudu/integration-tests/linked_list-test-util.h
1 file changed, 36 insertions(+), 16 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/6576/1
-- 
To view, visit http://gerrit.cloudera.org:8080/6576
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6d82d495d55091ee597995c2e5963c573401f624
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>

[kudu-CR] linked list-test: dump a histogram of Update performance

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: linked_list-test: dump a histogram of Update performance
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6576/1/src/kudu/integration-tests/linked_list-test-util.h
File src/kudu/integration-tests/linked_list-test-util.h:

PS1, Line 181: static void DumpHistogram(const HdrHistogram& h);
DumpLatencyHistogram? Also docs


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6d82d495d55091ee597995c2e5963c573401f624
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] linked list-test: dump a histogram of Update performance

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: linked_list-test: dump a histogram of Update performance
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6576/1/src/kudu/integration-tests/linked_list-test-util.h
File src/kudu/integration-tests/linked_list-test-util.h:

PS1, Line 312: 1000000, 3
magic numbers? at least doc


PS1, Line 547: void LinkedListTester::DumpHistogram(const HdrHistogram& h) {
             :   using std::cout;
             :   using std::endl;
             :   cout << "Count: " << h.TotalCount() << endl;
             :   cout << "Mean: " << h.MeanValue() << endl;
             :   cout << "Percentiles:" << endl;
             :   cout << "   0%  (min) = " << h.MinValue() << endl;
             :   cout << "  25%        = " << h.ValueAtPercentile(25) << endl;
             :   cout << "  50%  (med) = " << h.ValueAtPercentile(50) << endl;
             :   cout << "  75%        = " << h.ValueAtPercentile(75) << endl;
             :   cout << "  95%        = " << h.ValueAtPercentile(95) << endl;
             :   cout << "  99%        = " << h.ValueAtPercentile(99) << endl;
             :   cout << "  99.9%      = " << h.ValueAtPercentile(99.9) << endl;
             :   cout << "  99.99%     = " << h.ValueAtPercentile(99.99) << endl;
             :   cout << "  100% (max) = " << h.MaxValue() << endl;
             :   if (h.MaxValue() >= h.highest_trackable_value()) {
             :     cout << "*NOTE: some values were greater than highest trackable value" << endl;
             :   }
             : }
maybe dumping the histo to string should be on the histogram class and then you'd dump it to stdout here?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6d82d495d55091ee597995c2e5963c573401f624
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes