You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Will Berkeley (Code Review)" <ge...@cloudera.org> on 2018/11/20 22:58:08 UTC
[kudu-CR] [webui] Fancy table for /mem-trackers and sortable tables
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11968
Change subject: [webui] Fancy table for /mem-trackers and sortable tables
......................................................................
[webui] Fancy table for /mem-trackers and sortable tables
This fancifies the table of trackers on /mem-trackers in the style of
6ae9ecbe2595090c78e7afd271aae9d04dd4d0b5. It also adds the ability to
sort some tables by some numeric columns. Namely:
- the /mem-trackers trackers table is sortable by current consumption
and peak consumption
- the /tablets page tablets tables are sortable by on-disk size
- the /maintenance-manager "non-running op" table is sortable by RAM
anchored, logs retained, and perf.
I tested this change out manually, verifying that ascending and
descending sort looked good. Unfortunately, the bootstrap table
library doesn't seem to use a stable sort.
Change-Id: Ibdf8e7bd82fe2b95e699b8bb238a9cf0e5a7e727
---
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/webserver.cc
A www/kudu.js
M www/maintenance-manager.mustache
M www/tablets.mustache
5 files changed, 91 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/68/11968/1
--
To view, visit http://gerrit.cloudera.org:8080/11968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibdf8e7bd82fe2b95e699b8bb238a9cf0e5a7e727
Gerrit-Change-Number: 11968
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
[kudu-CR] [webui] Fancy table for /mem-trackers and sortable tables
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11968 )
Change subject: [webui] Fancy table for /mem-trackers and sortable tables
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11968/1/www/kudu.js
File www/kudu.js:
http://gerrit.cloudera.org:8080/#/c/11968/1/www/kudu.js@18
PS1, Line 18: // Converts a human-readable bytes value like '1.23B' or '985.32M' to a number
: // of bytes. The suffix must be present: '1.23' is not valid but '1.23B' is.
Seems like we could relax the suffix restriction if we parsed the last byte of the string first. If it's a letter, we parse the remaining string into a float and transform the value based on the unit (erroring if it's a letter we don't recognize). If it's a digit, we parse the entire string as a float and don't transform it.
http://gerrit.cloudera.org:8080/#/c/11968/1/www/kudu.js@41
PS1, Line 41: case 'k': val *= 1024.0;
Curious why we do special treatment for lower-case k but not for any of the other units?
http://gerrit.cloudera.org:8080/#/c/11968/1/www/maintenance-manager.mustache
File www/maintenance-manager.mustache:
http://gerrit.cloudera.org:8080/#/c/11968/1/www/maintenance-manager.mustache@66
PS1, Line 66: Ram
Nit: isn't 'RAM' a more precise term though?
--
To view, visit http://gerrit.cloudera.org:8080/11968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibdf8e7bd82fe2b95e699b8bb238a9cf0e5a7e727
Gerrit-Change-Number: 11968
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Comment-Date: Wed, 21 Nov 2018 00:21:47 +0000
Gerrit-HasComments: Yes
[kudu-CR] [webui] Fancy table for /mem-trackers and sortable tables
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Mitch Barnett, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11968
to look at the new patch set (#3).
Change subject: [webui] Fancy table for /mem-trackers and sortable tables
......................................................................
[webui] Fancy table for /mem-trackers and sortable tables
This fancifies the table of trackers on /mem-trackers in the style of
6ae9ecbe2595090c78e7afd271aae9d04dd4d0b5. It also adds the ability to
sort some tables by some numeric columns. Namely:
- the /mem-trackers trackers table is sortable by current consumption
and peak consumption
- the /tablets page tablets tables are sortable by on-disk size
- the /maintenance-manager "non-running op" table is sortable by RAM
anchored, logs retained, and perf.
I tested this change out manually, verifying that ascending and
descending sort looked good. Unfortunately, the bootstrap table
library doesn't seem to use a stable sort.
Change-Id: Ibdf8e7bd82fe2b95e699b8bb238a9cf0e5a7e727
---
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/webserver.cc
A www/kudu.js
M www/maintenance-manager.mustache
M www/tablets.mustache
5 files changed, 90 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/68/11968/3
--
To view, visit http://gerrit.cloudera.org:8080/11968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibdf8e7bd82fe2b95e699b8bb238a9cf0e5a7e727
Gerrit-Change-Number: 11968
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [webui] Fancy table for /mem-trackers and sortable tables
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Mitch Barnett, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11968
to look at the new patch set (#2).
Change subject: [webui] Fancy table for /mem-trackers and sortable tables
......................................................................
[webui] Fancy table for /mem-trackers and sortable tables
This fancifies the table of trackers on /mem-trackers in the style of
6ae9ecbe2595090c78e7afd271aae9d04dd4d0b5. It also adds the ability to
sort some tables by some numeric columns. Namely:
- the /mem-trackers trackers table is sortable by current consumption
and peak consumption
- the /tablets page tablets tables are sortable by on-disk size
- the /maintenance-manager "non-running op" table is sortable by RAM
anchored, logs retained, and perf.
I tested this change out manually, verifying that ascending and
descending sort looked good. Unfortunately, the bootstrap table
library doesn't seem to use a stable sort.
Change-Id: Ibdf8e7bd82fe2b95e699b8bb238a9cf0e5a7e727
---
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/webserver.cc
A www/kudu.js
M www/maintenance-manager.mustache
M www/tablets.mustache
5 files changed, 90 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/68/11968/2
--
To view, visit http://gerrit.cloudera.org:8080/11968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibdf8e7bd82fe2b95e699b8bb238a9cf0e5a7e727
Gerrit-Change-Number: 11968
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [webui] Fancy table for /mem-trackers and sortable tables
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11968 )
Change subject: [webui] Fancy table for /mem-trackers and sortable tables
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11968/1/www/kudu.js
File www/kudu.js:
http://gerrit.cloudera.org:8080/#/c/11968/1/www/kudu.js@18
PS1, Line 18: // Converts a human-readable bytes value like '1.23B' or '985.32M' to a number
: // of bytes. The suffix must be present: '1.23' is not valid but '1.23B' is.
> Seems like we could relax the suffix restriction if we parsed the last byte
Yeah, but I think it's not necessary and it makes the code more complicated. There's also the technical point that 1.23 != 1.23B, just like 1 != 1 foot.
Also, this function is meant to invert HumanReadableNumBytes::ToString, which always includes the suffix.
http://gerrit.cloudera.org:8080/#/c/11968/1/www/kudu.js@41
PS1, Line 41: case 'k': val *= 1024.0;
> Curious why we do special treatment for lower-case k but not for any of the
Well, frankly, it's because I copied the body of the switch statement from HumanReadableBytes::ToDouble (except for a really terrible joke comment good riddance) and it handles 'K' and 'k'. HumanReadableNumBytes::ToString emits only 'K' so I can remove handling for 'k'.
http://gerrit.cloudera.org:8080/#/c/11968/1/www/maintenance-manager.mustache
File www/maintenance-manager.mustache:
http://gerrit.cloudera.org:8080/#/c/11968/1/www/maintenance-manager.mustache@66
PS1, Line 66: Ram
> Nit: isn't 'RAM' a more precise term though?
Whoops. Messed up when I rewrote it.
--
To view, visit http://gerrit.cloudera.org:8080/11968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibdf8e7bd82fe2b95e699b8bb238a9cf0e5a7e727
Gerrit-Change-Number: 11968
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 21 Nov 2018 00:54:47 +0000
Gerrit-HasComments: Yes
[kudu-CR] [webui] Fancy table for /mem-trackers and sortable tables
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11968 )
Change subject: [webui] Fancy table for /mem-trackers and sortable tables
......................................................................
[webui] Fancy table for /mem-trackers and sortable tables
This fancifies the table of trackers on /mem-trackers in the style of
6ae9ecbe2595090c78e7afd271aae9d04dd4d0b5. It also adds the ability to
sort some tables by some numeric columns. Namely:
- the /mem-trackers trackers table is sortable by current consumption
and peak consumption
- the /tablets page tablets tables are sortable by on-disk size
- the /maintenance-manager "non-running op" table is sortable by RAM
anchored, logs retained, and perf.
I tested this change out manually, verifying that ascending and
descending sort looked good. Unfortunately, the bootstrap table
library doesn't seem to use a stable sort.
Change-Id: Ibdf8e7bd82fe2b95e699b8bb238a9cf0e5a7e727
Reviewed-on: http://gerrit.cloudera.org:8080/11968
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/webserver.cc
A www/kudu.js
M www/maintenance-manager.mustache
M www/tablets.mustache
5 files changed, 90 insertions(+), 13 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Andrew Wong: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/11968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibdf8e7bd82fe2b95e699b8bb238a9cf0e5a7e727
Gerrit-Change-Number: 11968
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [webui] Fancy table for /mem-trackers and sortable tables
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11968 )
Change subject: [webui] Fancy table for /mem-trackers and sortable tables
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11968/1/www/kudu.js
File www/kudu.js:
http://gerrit.cloudera.org:8080/#/c/11968/1/www/kudu.js@41
PS1, Line 41: case 'B': break;
> Well, frankly, it's because I copied the body of the switch statement from
From HumanReadableNumBytes::ToDouble:
case 'Y': d *= 1024.0; // That's a yotta bytes!
It's not _that_ bad...
--
To view, visit http://gerrit.cloudera.org:8080/11968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibdf8e7bd82fe2b95e699b8bb238a9cf0e5a7e727
Gerrit-Change-Number: 11968
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 21 Nov 2018 01:02:03 +0000
Gerrit-HasComments: Yes
[kudu-CR] [webui] Fancy table for /mem-trackers and sortable tables
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11968 )
Change subject: [webui] Fancy table for /mem-trackers and sortable tables
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibdf8e7bd82fe2b95e699b8bb238a9cf0e5a7e727
Gerrit-Change-Number: 11968
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 21 Nov 2018 01:18:34 +0000
Gerrit-HasComments: No