[kudu-CR] Add Maintenance Manager visualizer

[Dan Burkert (Code Review)]

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 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add Maintenance Manager visualizer

[Todd Lipcon (Code Review)]

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($("").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 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add Maintenance Manager visualizer

[Dan Burkert (Code Review)]

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 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add Maintenance Manager visualizer

[Sam Okrent (Code Review)]

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 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add Maintenance Manager visualizer

[Jean-Daniel Cryans (Code Review)]

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 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add Maintenance Manager visualizer

[Sam Okrent (Code Review)]

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 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add Maintenance Manager visualizer

[Andrew Wong (Code Review)]

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 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add Maintenance Manager visualizer

[Sam Okrent (Code Review)]

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 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add Maintenance Manager visualizer

[Sam Okrent (Code Review)]

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