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/03/07 05:59:45 UTC

[kudu-CR] log: improve mt-log-test benchmark

Hello David Ribeiro Alves,

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

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

to review the following change.

Change subject: log: improve mt-log-test benchmark
......................................................................

log: improve mt-log-test benchmark

- builds the entries to be appended outside of holding any lock, which
  is more realistic. The entries get their IDs set once inside the lock
  (similar to what we do in real code).
- use the same higher-level APIs that the real append paths use
- make the log segment size overridable
- a couple bug fixes here and there

Change-Id: I870dc26e2937e7c92e3f0530e2c2880178507f12
---
M src/kudu/consensus/mt-log-test.cc
1 file changed, 47 insertions(+), 43 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I870dc26e2937e7c92e3f0530e2c2880178507f12
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] log: improve mt-log-test benchmark

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello David Ribeiro Alves, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#2).

Change subject: log: improve mt-log-test benchmark
......................................................................

log: improve mt-log-test benchmark

- builds the entries to be appended outside of holding any lock, which
  is more realistic. The entries get their IDs set once inside the lock
  (similar to what we do in real code).
- use the same higher-level APIs that the real append paths use
- make the log segment size overridable
- allow the "verification" step to be disabled to serve as a better benchmark
- allow the concurrent "reader" thread to be disabled
- a couple bug fixes here and there

Change-Id: I870dc26e2937e7c92e3f0530e2c2880178507f12
---
M src/kudu/consensus/mt-log-test.cc
1 file changed, 77 insertions(+), 65 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/83/6283/2
-- 
To view, visit http://gerrit.cloudera.org:8080/6283
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I870dc26e2937e7c92e3f0530e2c2880178507f12
Gerrit-PatchSet: 2
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

[kudu-CR] log: improve mt-log-test benchmark

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged.

Change subject: log: improve mt-log-test benchmark
......................................................................


log: improve mt-log-test benchmark

- builds the entries to be appended outside of holding any lock, which
  is more realistic. The entries get their IDs set once inside the lock
  (similar to what we do in real code).
- use the same higher-level APIs that the real append paths use
- make the log segment size overridable
- allow the "verification" step to be disabled to serve as a better benchmark
- allow the concurrent "reader" thread to be disabled
- a couple bug fixes here and there

Change-Id: I870dc26e2937e7c92e3f0530e2c2880178507f12
Reviewed-on: http://gerrit.cloudera.org:8080/6283
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves <dr...@apache.org>
---
M src/kudu/consensus/mt-log-test.cc
1 file changed, 77 insertions(+), 65 deletions(-)

Approvals:
  David Ribeiro Alves: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I870dc26e2937e7c92e3f0530e2c2880178507f12
Gerrit-PatchSet: 3
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-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] log: improve mt-log-test benchmark

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

Change subject: log: improve mt-log-test benchmark
......................................................................


Patch Set 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6283/2/src/kudu/consensus/mt-log-test.cc
File src/kudu/consensus/mt-log-test.cc:

PS2, Line 179: if (google::GetCommandLineFlagInfoOrDie("log_segment_size_mb").is_default) {
interesting


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I870dc26e2937e7c92e3f0530e2c2880178507f12
Gerrit-PatchSet: 2
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