You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Sam Okrent (Code Review)" <ge...@cloudera.org> on 2017/08/03 00:47:34 UTC
[kudu-CR] Add Maintenance Manager visualizer
Sam Okrent has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7570
Change subject: Add Maintenance Manager visualizer
......................................................................
Add Maintenance Manager visualizer
Adds a timeline/swimlane chart to display maintenance
manager operations to the /maintenance-manager page
of the tserver web UI. Previously, completed and
running ops were displayed in two tables.
Also switches the /maintenance-manager page over
to a mustache template.
Change-Id: If2e7f18ac4834791a94d935b540930b11ea14532
---
M src/kudu/tserver/tserver-path-handlers.cc
M src/kudu/tserver/tserver-path-handlers.h
A www/maintenance-manager.mustache
3 files changed, 393 insertions(+), 45 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/7570/1
--
To view, visit http://gerrit.cloudera.org:8080/7570
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If2e7f18ac4834791a94d935b540930b11ea14532
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
[kudu-CR] Add Maintenance Manager visualizer
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: Add Maintenance Manager visualizer
......................................................................
Patch Set 7:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/7570/7/www/maintenance-manager.mustache
File www/maintenance-manager.mustache:
PS7, Line 106: CompactRowSetsOp {
: fill: #095;
: }
:
: .FlushDeltaMemStoresOp {
: fill: #ec1;
: }
:
: .FlushMRSOp {
: fill: #16a;
: }
:
: .LogGCOp {
: fill: #2ae;
: }
:
: .MajorDeltaCompactionOp {
: fill: #c23;
: }
:
: .MinorDeltaCompactionOp {
: fill: #92c;
: }
:
: .UndoDeltaBlockGCOp {
maybe we can fill these in from the template?
Line 173: } else if (millis < 1000 * 60) {
I think a better threshold here might be 300 or something... otherwise anywhere from 61 to 119 seconds will be reported the same, which doesn't seem that great
PS7, Line 180: // Translates a thread id hex string into a counting-number identifier.
: function threadString(thread_id) {
: return "Thread " + (threads.indexOf(thread_id) + 1);
: }
would rather figure a way to show pids here so you can correlate with trace results, pstacks, etc
PS7, Line 192: html
shoudl be .text() instead of .html()
Line 192: .append($("<h5/>").html(op.name))
why not use jQuery(popup).css(...) to set the above styles instead of the native DOM manipulation?
also could just use jQuery("#popup") to find the element
PS7, Line 201: var popup = document.getElementById("popup");
: popup.innerHTML = "";
: popup.style.display = "none";
jQuery("#popup").empty().hide();
PS7, Line 217: (i
var i
PS7, Line 219: [];
an empty array here seems unexpected since it's used as an array index below
PS7, Line 246: .
nit: inconsistent indentation
Line 328: // and a corresponding css class (as above) should be added.
can this be passed in from the code?
--
To view, visit http://gerrit.cloudera.org:8080/7570
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If2e7f18ac4834791a94d935b540930b11ea14532
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Add Maintenance Manager visualizer
Posted by "Sam Okrent (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7570
to look at the new patch set (#5).
Change subject: Add Maintenance Manager visualizer
......................................................................
Add Maintenance Manager visualizer
Adds a timeline/swimlane chart to display maintenance
manager operations to the /maintenance-manager page
of the tserver web UI. Previously, completed and
running ops were displayed in two tables. Now, both
running and completed ops are displayed as color-coded
bars in parallel lanes representing threads.
A screenshot of the new maintenance-manager page
can be found here: http://imgur.com/gallery/BVJAV.
Also switches the /maintenance-manager page over
to a mustache template.
Change-Id: If2e7f18ac4834791a94d935b540930b11ea14532
---
M src/kudu/tserver/tserver-path-handlers.cc
M src/kudu/tserver/tserver-path-handlers.h
A www/maintenance-manager.mustache
3 files changed, 409 insertions(+), 45 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/7570/5
--
To view, visit http://gerrit.cloudera.org:8080/7570
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If2e7f18ac4834791a94d935b540930b11ea14532
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Add Maintenance Manager visualizer
Posted by "Sam Okrent (Code Review)" <ge...@cloudera.org>.
Sam Okrent has posted comments on this change.
Change subject: Add Maintenance Manager visualizer
......................................................................
Patch Set 1:
The last two builds failed with different tests, and both seem unrelated.
--
To view, visit http://gerrit.cloudera.org:8080/7570
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If2e7f18ac4834791a94d935b540930b11ea14532
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] Add Maintenance Manager visualizer
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: Add Maintenance Manager visualizer
......................................................................
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/7570/7/www/maintenance-manager.mustache
File www/maintenance-manager.mustache:
Line 173: } else if (millis < 1000 * 60) {
> I think a better threshold here might be 300 or something... otherwise anyw
My personal preference is to always make it the largest possible unit, but keep around 3 significant digits of precision.
PS7, Line 180: // Translates a thread id hex string into a counting-number identifier.
: function threadString(thread_id) {
: return "Thread " + (threads.indexOf(thread_id) + 1);
: }
> would rather figure a way to show pids here so you can correlate with trace
https://gerrit.cloudera.org/#/c/7621/ switches the MM to use the system TID
--
To view, visit http://gerrit.cloudera.org:8080/7570
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If2e7f18ac4834791a94d935b540930b11ea14532
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Add Maintenance Manager visualizer
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: Add Maintenance Manager visualizer
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7570/5/www/maintenance-manager.mustache
File www/maintenance-manager.mustache:
Line 326: // If new operation is added, its name should be added to this list,
Mmm do we have a comment where those classes are defined about not forgetting to modify this file?
--
To view, visit http://gerrit.cloudera.org:8080/7570
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If2e7f18ac4834791a94d935b540930b11ea14532
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Add Maintenance Manager visualizer
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change.
Change subject: Add Maintenance Manager visualizer
......................................................................
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/7570/3//COMMIT_MSG
Commit Message:
PS3, Line 7: Add Maintenance Manager visualizer
Could you add a link to some publicly-hosted screenshot? E.g. upload the image from your slides to your own local github branch and link to that, or somesuch.
PS3, Line 11: Previously, completed and
: running ops were displayed in two tables.
Are running ops displayed differently from completed ops? In addition to an image, a description of what is newly shown would be nice.
http://gerrit.cloudera.org:8080/#/c/7570/3/www/maintenance-manager.mustache
File www/maintenance-manager.mustache:
PS3, Line 69: }
nit: Got some whitespaces throughout this file.
PS3, Line 181:
: function inspectOp(op, event) {
While some of these functions are fairly straightforward, it'd still be nice to have some docs explaining what these do.
PS3, Line 202: "";
Do we expect these?
Might be worth documenting what an expected op.name/output are.
--
To view, visit http://gerrit.cloudera.org:8080/7570
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If2e7f18ac4834791a94d935b540930b11ea14532
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Add Maintenance Manager visualizer
Posted by "Sam Okrent (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7570
to look at the new patch set (#6).
Change subject: Add Maintenance Manager visualizer
......................................................................
Add Maintenance Manager visualizer
Adds a timeline/swimlane chart to display maintenance
manager operations to the /maintenance-manager page
of the tserver web UI. Previously, completed and
running ops were displayed in two tables. Now, both
running and completed ops are displayed as color-coded
bars in parallel lanes representing threads.
A screenshot of the new maintenance-manager page
can be found here: http://imgur.com/gallery/qxiHa.
Also switches the /maintenance-manager page over
to a mustache template.
Change-Id: If2e7f18ac4834791a94d935b540930b11ea14532
---
M src/kudu/tserver/tserver-path-handlers.cc
M src/kudu/tserver/tserver-path-handlers.h
M src/kudu/util/maintenance_manager.h
A www/maintenance-manager.mustache
4 files changed, 413 insertions(+), 45 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/7570/6
--
To view, visit http://gerrit.cloudera.org:8080/7570
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If2e7f18ac4834791a94d935b540930b11ea14532
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Add Maintenance Manager visualizer
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7570
to look at the new patch set (#7).
Change subject: Add Maintenance Manager visualizer
......................................................................
Add Maintenance Manager visualizer
Adds a timeline/swimlane chart to display maintenance
manager operations to the /maintenance-manager page
of the tserver web UI. Previously, completed and
running ops were displayed in two tables. Now, both
running and completed ops are displayed as color-coded
bars in parallel lanes representing threads.
A screenshot of the new maintenance-manager page
can be found here: http://imgur.com/gallery/qxiHa.
Change-Id: If2e7f18ac4834791a94d935b540930b11ea14532
---
M src/kudu/tserver/tserver-path-handlers.cc
M src/kudu/util/maintenance_manager.h
M www/maintenance-manager.mustache
3 files changed, 348 insertions(+), 47 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/7570/7
--
To view, visit http://gerrit.cloudera.org:8080/7570
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If2e7f18ac4834791a94d935b540930b11ea14532
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>