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 2018/04/02 23:28:09 UTC
[kudu-CR] pprof: fix contention output
Hello Andrew Wong,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/9896
to review the following change.
Change subject: pprof: fix contention output
......................................................................
pprof: fix contention output
In 600723e0e954a1b199e6a0bf1dfd63fe7289a6a8 I changed the profile to be
written to an intermediary buffer before writing to the eventual HTTP
output stream, but missed one location where it was still writing
directly. This caused some output that would look like:
<profile samples>
<header>
<more profile samples>
...which confused the pprof tool.
Change-Id: I6b55a47c3e23daa62302b4318580592695ec0f5f
---
M src/kudu/server/pprof_path_handlers.cc
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/9896/1
--
To view, visit http://gerrit.cloudera.org:8080/9896
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6b55a47c3e23daa62302b4318580592695ec0f5f
Gerrit-Change-Number: 9896
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
[kudu-CR] pprof: fix contention output
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9896 )
Change subject: pprof: fix contention output
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9896/1/src/kudu/server/pprof_path_handlers.cc
File src/kudu/server/pprof_path_handlers.cc:
http://gerrit.cloudera.org:8080/#/c/9896/1/src/kudu/server/pprof_path_handlers.cc@169
PS1, Line 169: ostringstream* output = resp->output;
> nit: maybe move this down where it's used. Its current placement is an arti
Oops, `profile` instead of `output`
--
To view, visit http://gerrit.cloudera.org:8080/9896
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b55a47c3e23daa62302b4318580592695ec0f5f
Gerrit-Change-Number: 9896
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 03 Apr 2018 02:57:10 +0000
Gerrit-HasComments: Yes
[kudu-CR] pprof: fix contention output
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9896 )
Change subject: pprof: fix contention output
......................................................................
pprof: fix contention output
In 600723e0e954a1b199e6a0bf1dfd63fe7289a6a8 I changed the profile to be
written to an intermediary buffer before writing to the eventual HTTP
output stream, but missed one location where it was still writing
directly. This caused some output that would look like:
<profile samples>
<header>
<more profile samples>
...which confused the pprof tool.
Change-Id: I6b55a47c3e23daa62302b4318580592695ec0f5f
Reviewed-on: http://gerrit.cloudera.org:8080/9896
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M src/kudu/server/pprof_path_handlers.cc
1 file changed, 3 insertions(+), 3 deletions(-)
Approvals:
Kudu Jenkins: Verified
Andrew Wong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/9896
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6b55a47c3e23daa62302b4318580592695ec0f5f
Gerrit-Change-Number: 9896
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] pprof: fix contention output
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9896 )
Change subject: pprof: fix contention output
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/9896
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b55a47c3e23daa62302b4318580592695ec0f5f
Gerrit-Change-Number: 9896
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 03 Apr 2018 05:21:06 +0000
Gerrit-HasComments: No
[kudu-CR] pprof: fix contention output
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Andrew Wong, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9896
to look at the new patch set (#2).
Change subject: pprof: fix contention output
......................................................................
pprof: fix contention output
In 600723e0e954a1b199e6a0bf1dfd63fe7289a6a8 I changed the profile to be
written to an intermediary buffer before writing to the eventual HTTP
output stream, but missed one location where it was still writing
directly. This caused some output that would look like:
<profile samples>
<header>
<more profile samples>
...which confused the pprof tool.
Change-Id: I6b55a47c3e23daa62302b4318580592695ec0f5f
---
M src/kudu/server/pprof_path_handlers.cc
1 file changed, 3 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/9896/2
--
To view, visit http://gerrit.cloudera.org:8080/9896
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6b55a47c3e23daa62302b4318580592695ec0f5f
Gerrit-Change-Number: 9896
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] pprof: fix contention output
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9896 )
Change subject: pprof: fix contention output
......................................................................
Patch Set 1: Code-Review+2
(2 comments)
Small nit
http://gerrit.cloudera.org:8080/#/c/9896/1/src/kudu/server/pprof_path_handlers.cc
File src/kudu/server/pprof_path_handlers.cc:
http://gerrit.cloudera.org:8080/#/c/9896/1/src/kudu/server/pprof_path_handlers.cc@169
PS1, Line 169: ostringstream* output = resp->output;
nit: maybe move this down where it's used. Its current placement is an artifact of the pre-600723e0e954a1b199e6a0bf1dfd63fe7289a6a8 implementation. Putting it down by L184 might make it more obvious we should be using `output` instead of `profile`.
http://gerrit.cloudera.org:8080/#/c/9896/1/src/kudu/server/pprof_path_handlers.cc@195
PS1, Line 195: Env::Default()
:-/
--
To view, visit http://gerrit.cloudera.org:8080/9896
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b55a47c3e23daa62302b4318580592695ec0f5f
Gerrit-Change-Number: 9896
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 03 Apr 2018 02:12:32 +0000
Gerrit-HasComments: Yes