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/05/16 01:20:04 UTC
[kudu-CR] [webui] Convert /tablets page to mustache
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10422
Change subject: [webui] Convert /tablets page to mustache
......................................................................
[webui] Convert /tablets page to mustache
Originally I wanted to add a message to the /tablets page if there were
no tablet replicas on the server, just so it wouldn't be blank, but here
we are.
Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
---
M src/kudu/tserver/tserver_path_handlers.cc
M src/kudu/tserver/tserver_path_handlers.h
A www/tablets.mustache
3 files changed, 160 insertions(+), 68 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/10422/1
--
To view, visit http://gerrit.cloudera.org:8080/10422
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
Gerrit-Change-Number: 10422
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
[kudu-CR] [webui] Convert /tablets page to mustache
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10422 )
Change subject: [webui] Convert /tablets page to mustache
......................................................................
Patch Set 4: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10422/4/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/10422/4/src/kudu/tserver/tablet_server-test.cc@403
PS4, Line 403: ASSERT_STR_CONTAINS(buf.ToString(), "RANGE (key) PARTITION UNBOUNDED");
:
: // Tablet page should include the schema.
: ASSERT_OK(c.FetchURL(Substitute("http://$0/tablet?id=$1", addr, kTabletId),
: &buf));
: ASSERT_STR_CONTAINS(buf.ToString(), "key");
: ASSERT_STR_CONTAINS(buf.ToString(), "string NULLABLE");
> The original asserts had html bits in them. Without a template to fill in,
Ah I see, thanks for the explanation!
--
To view, visit http://gerrit.cloudera.org:8080/10422
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
Gerrit-Change-Number: 10422
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Sun, 20 May 2018 18:58:53 +0000
Gerrit-HasComments: Yes
[kudu-CR] [webui] Convert /tablets page to mustache
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10422 )
Change subject: [webui] Convert /tablets page to mustache
......................................................................
[webui] Convert /tablets page to mustache
Originally I wanted to add a message to the /tablets page if there were
no tablet replicas on the server, just so it wouldn't be blank, but here
we are.
Also, since our pre-commit test machines don't have templates, I had to
adjust some tests to search for substrings of the JSON rather than the
template-generated HTML.
Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
Reviewed-on: http://gerrit.cloudera.org:8080/10422
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Will Berkeley <wd...@gmail.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tserver_path_handlers.cc
M src/kudu/tserver/tserver_path_handlers.h
A www/tablets.mustache
4 files changed, 255 insertions(+), 179 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved
Will Berkeley: Verified
Andrew Wong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/10422
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
Gerrit-Change-Number: 10422
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [webui] Convert /tablets page to mustache
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10422 )
Change subject: [webui] Convert /tablets page to mustache
......................................................................
Patch Set 2:
> Hm.. To Alexey's point, maybe we should meet in the middle with
> "tablet replicas" or something?
"replicas" or "tablet replicas" are correct..."tablets" is an abuse of language. Developers know from context whether "tablet" means "tablet" or "replica", but not random users, so it's worth changing everything to be accurate.
--
To view, visit http://gerrit.cloudera.org:8080/10422
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
Gerrit-Change-Number: 10422
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 18 May 2018 21:50:46 +0000
Gerrit-HasComments: No
[kudu-CR] [webui] Convert /tablets page to mustache
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10422
to look at the new patch set (#4).
Change subject: [webui] Convert /tablets page to mustache
......................................................................
[webui] Convert /tablets page to mustache
Originally I wanted to add a message to the /tablets page if there were
no tablet replicas on the server, just so it wouldn't be blank, but here
we are.
Also, since our pre-commit test machines don't have templates, I had to
adjust some tests to search for substrings of the JSON rather than the
template-generated HTML.
Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
---
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tserver_path_handlers.cc
M src/kudu/tserver/tserver_path_handlers.h
A www/tablets.mustache
4 files changed, 255 insertions(+), 179 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/10422/4
--
To view, visit http://gerrit.cloudera.org:8080/10422
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
Gerrit-Change-Number: 10422
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [webui] Convert /tablets page to mustache
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10422 )
Change subject: [webui] Convert /tablets page to mustache
......................................................................
Patch Set 1:
(11 comments)
Looks pretty good, just some nits. Also, it seems we have some mix of 'replica' and 'tablet' while naming entities. Maybe, use this opportunity to clean that up?
http://gerrit.cloudera.org:8080/#/c/10422/1/src/kudu/tserver/tserver_path_handlers.cc
File src/kudu/tserver/tserver_path_handlers.cc:
http://gerrit.cloudera.org:8080/#/c/10422/1/src/kudu/tserver/tserver_path_handlers.cc@274
PS1, Line 274: string n_bytes = "";
Is it used at all?
http://gerrit.cloudera.org:8080/#/c/10422/1/src/kudu/tserver/tserver_path_handlers.cc@303
PS1, Line 303: live_tablets
live_replicas?
http://gerrit.cloudera.org:8080/#/c/10422/1/src/kudu/tserver/tserver_path_handlers.cc@307
PS1, Line 307: tombstoned_tablets
tombstoned_replicas?
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache
File www/tablets.mustache:
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache@22
PS1, Line 22: live_tablets
live_replicas?
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache@23
PS1, Line 23: Tablets
Replicas?
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache@71
PS1, Line 71: Tombstone
Tombstoned
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache@71
PS1, Line 71: tablets
replicas?
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache@72
PS1, Line 72: tablets
replicas?
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache@80
PS1, Line 80: tablet
replica ?
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache@82
PS1, Line 82: tablet
replica ?
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache@118
PS1, Line 118: tablets
replicas ?
--
To view, visit http://gerrit.cloudera.org:8080/10422
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
Gerrit-Change-Number: 10422
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 17 May 2018 17:23:32 +0000
Gerrit-HasComments: Yes
[kudu-CR] [webui] Convert /tablets page to mustache
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10422
to look at the new patch set (#2).
Change subject: [webui] Convert /tablets page to mustache
......................................................................
[webui] Convert /tablets page to mustache
Originally I wanted to add a message to the /tablets page if there were
no tablet replicas on the server, just so it wouldn't be blank, but here
we are.
Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
---
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tserver_path_handlers.cc
M src/kudu/tserver/tserver_path_handlers.h
A www/tablets.mustache
4 files changed, 244 insertions(+), 161 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/10422/2
--
To view, visit http://gerrit.cloudera.org:8080/10422
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
Gerrit-Change-Number: 10422
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [webui] Convert /tablets page to mustache
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10422 )
Change subject: [webui] Convert /tablets page to mustache
......................................................................
Patch Set 4: Code-Review+2
LGTM.
Maybe it's worth to wait for Andrew's feedback.
--
To view, visit http://gerrit.cloudera.org:8080/10422
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
Gerrit-Change-Number: 10422
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Sat, 19 May 2018 05:59:42 +0000
Gerrit-HasComments: No
[kudu-CR] [webui] Convert /tablets page to mustache
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10422
to look at the new patch set (#3).
Change subject: [webui] Convert /tablets page to mustache
......................................................................
[webui] Convert /tablets page to mustache
Originally I wanted to add a message to the /tablets page if there were
no tablet replicas on the server, just so it wouldn't be blank, but here
we are.
Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
---
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tserver_path_handlers.cc
M src/kudu/tserver/tserver_path_handlers.h
A www/tablets.mustache
4 files changed, 244 insertions(+), 166 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/10422/3
--
To view, visit http://gerrit.cloudera.org:8080/10422
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
Gerrit-Change-Number: 10422
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [webui] Convert /tablets page to mustache
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has removed a vote on this change.
Change subject: [webui] Convert /tablets page to mustache
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/10422
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
Gerrit-Change-Number: 10422
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [webui] Convert /tablets page to mustache
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10422 )
Change subject: [webui] Convert /tablets page to mustache
......................................................................
Patch Set 2:
The test failures indicate the templates in www/ were not available, not a problem with the patch. I will retry the tests when I update the patch to fix the IWYU failure.
--
To view, visit http://gerrit.cloudera.org:8080/10422
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
Gerrit-Change-Number: 10422
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 18 May 2018 21:48:57 +0000
Gerrit-HasComments: No
[kudu-CR] [webui] Convert /tablets page to mustache
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10422 )
Change subject: [webui] Convert /tablets page to mustache
......................................................................
Patch Set 1:
(3 comments)
Hm.. To Alexey's point, maybe we should meet in the middle with "tablet replicas" or something? I don't have a strong opinion either way.
http://gerrit.cloudera.org:8080/#/c/10422/1/src/kudu/tserver/tserver_path_handlers.cc
File src/kudu/tserver/tserver_path_handlers.cc:
http://gerrit.cloudera.org:8080/#/c/10422/1/src/kudu/tserver/tserver_path_handlers.cc@237
PS1, Line 237: auto make_replicas_json = [this
Might be missing something, but why does `this` need to be here? Also can this be const auto?
http://gerrit.cloudera.org:8080/#/c/10422/1/src/kudu/tserver/tserver_path_handlers.cc@259
PS1, Line 259: replica->tablet() != nullptr) {
: replica_json["id_or_link"] = TabletLink(status.tablet_id());
: } else {
: replica_json["id_or_link"] = status.tablet_id();
: }
: replica_json["partition"] =
: replica->tablet_metadata()
: ->partition_schema()
: .PartitionDebugString(replica->tablet_metadata()->partition(),
: replica->tablet_metadata()->schema());
: replica_json["state"] = replica->HumanReadableState();
: if (replica->tablet() != nullptr) {
: replica_json["mem_bytes"] = HumanReadableNumBytes::ToString(
: replica->tablet()->mem_tracker()->consumption());
nit: maybe pull out `replica->tablet()` and `replica->tablet_metadata()` into their own local variables? Also we should be able to merge L270 and L259, no?
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache
File www/tablets.mustache:
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache@96
PS1, Line 96: <th>Write buffer memory usage</th>
: <th>On-disk size</th>
Are these expected for tombstones? Everything else I can see being useful, if anything for tombstoned voting, but IIUC these should both be 0, right?
--
To view, visit http://gerrit.cloudera.org:8080/10422
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
Gerrit-Change-Number: 10422
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 17 May 2018 17:57:41 +0000
Gerrit-HasComments: Yes
[kudu-CR] [webui] Convert /tablets page to mustache
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10422 )
Change subject: [webui] Convert /tablets page to mustache
......................................................................
Patch Set 4: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10422/4/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/10422/4/src/kudu/tserver/tablet_server-test.cc@403
PS4, Line 403: ASSERT_STR_CONTAINS(buf.ToString(), "RANGE (key) PARTITION UNBOUNDED");
:
: // Tablet page should include the schema.
: ASSERT_OK(c.FetchURL(Substitute("http://$0/tablet?id=$1", addr, kTabletId),
: &buf));
: ASSERT_STR_CONTAINS(buf.ToString(), "key");
: ASSERT_STR_CONTAINS(buf.ToString(), "string NULLABLE");
I'm surprised these had to be changed. Was it caused by some spacing issue or something? From the mustache it seems like the output should have been more ore less the same.
--
To view, visit http://gerrit.cloudera.org:8080/10422
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
Gerrit-Change-Number: 10422
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Sun, 20 May 2018 17:34:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] [webui] Convert /tablets page to mustache
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10422 )
Change subject: [webui] Convert /tablets page to mustache
......................................................................
Patch Set 1:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/10422/1/src/kudu/tserver/tserver_path_handlers.cc
File src/kudu/tserver/tserver_path_handlers.cc:
http://gerrit.cloudera.org:8080/#/c/10422/1/src/kudu/tserver/tserver_path_handlers.cc@237
PS1, Line 237: auto make_replicas_json = [this
> Might be missing something, but why does `this` need to be here? Also can t
For the use of ConsensusStatePBToHtml. That function looks like it could be static, so I'll make it static and remove the this-capture.
http://gerrit.cloudera.org:8080/#/c/10422/1/src/kudu/tserver/tserver_path_handlers.cc@259
PS1, Line 259: replica->tablet() != nullptr) {
: replica_json["id_or_link"] = TabletLink(status.tablet_id());
: } else {
: replica_json["id_or_link"] = status.tablet_id();
: }
: replica_json["partition"] =
: replica->tablet_metadata()
: ->partition_schema()
: .PartitionDebugString(replica->tablet_metadata()->partition(),
: replica->tablet_metadata()->schema());
: replica_json["state"] = replica->HumanReadableState();
: if (replica->tablet() != nullptr) {
: replica_json["mem_bytes"] = HumanReadableNumBytes::ToString(
: replica->tablet()->mem_tracker()->consumption());
> nit: maybe pull out `replica->tablet()` and `replica->tablet_metadata()` in
Done
http://gerrit.cloudera.org:8080/#/c/10422/1/src/kudu/tserver/tserver_path_handlers.cc@274
PS1, Line 274: string n_bytes = "";
> Is it used at all?
Done
http://gerrit.cloudera.org:8080/#/c/10422/1/src/kudu/tserver/tserver_path_handlers.cc@303
PS1, Line 303: live_tablets
> live_replicas?
Done
http://gerrit.cloudera.org:8080/#/c/10422/1/src/kudu/tserver/tserver_path_handlers.cc@307
PS1, Line 307: tombstoned_tablets
> tombstoned_replicas?
Done
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache
File www/tablets.mustache:
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache@22
PS1, Line 22: live_tablets
> live_replicas?
Done
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache@23
PS1, Line 23: Tablets
> Replicas?
Live Tablet Replicas
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache@71
PS1, Line 71: tablets
> replicas?
tablet replicas
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache@71
PS1, Line 71: Tombstone
> Tombstoned
Done
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache@72
PS1, Line 72: tablets
> replicas?
tablet replicas
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache@80
PS1, Line 80: tablet
> replica ?
Done
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache@82
PS1, Line 82: tablet
> replica ?
Done
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache@96
PS1, Line 96: <th>Write buffer memory usage</th>
: <th>On-disk size</th>
> Are these expected for tombstones? Everything else I can see being useful,
On-disk size won't be zero because of cmeta and tablet meta. Write buffer is zero, and we don't show the config because, even though it exists, it's likely an old one for the tablet and may mislead people. It is available to more sophisticated users via ksck or other CLI tools.
I'll just get rid of the write buffer and config columns altogether for tombstones.
http://gerrit.cloudera.org:8080/#/c/10422/1/www/tablets.mustache@118
PS1, Line 118: tablets
> replicas ?
Done
--
To view, visit http://gerrit.cloudera.org:8080/10422
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
Gerrit-Change-Number: 10422
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 17 May 2018 21:58:24 +0000
Gerrit-HasComments: Yes
[kudu-CR] [webui] Convert /tablets page to mustache
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10422 )
Change subject: [webui] Convert /tablets page to mustache
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10422/4/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/10422/4/src/kudu/tserver/tablet_server-test.cc@403
PS4, Line 403: ASSERT_STR_CONTAINS(buf.ToString(), "RANGE (key) PARTITION UNBOUNDED");
:
: // Tablet page should include the schema.
: ASSERT_OK(c.FetchURL(Substitute("http://$0/tablet?id=$1", addr, kTabletId),
: &buf));
: ASSERT_STR_CONTAINS(buf.ToString(), "key");
: ASSERT_STR_CONTAINS(buf.ToString(), "string NULLABLE");
> I'm surprised these had to be changed. Was it caused by some spacing issue
The original asserts had html bits in them. Without a template to fill in, the jenkins Kudu processes just return the json body, so there were no html tags.
--
To view, visit http://gerrit.cloudera.org:8080/10422
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
Gerrit-Change-Number: 10422
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Sun, 20 May 2018 18:01:23 +0000
Gerrit-HasComments: Yes
[kudu-CR] [webui] Convert /tablets page to mustache
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10422 )
Change subject: [webui] Convert /tablets page to mustache
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/10422
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifddd74b23add52ae181b98a226b39e39df9b51ce
Gerrit-Change-Number: 10422
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Sat, 19 May 2018 06:14:20 +0000
Gerrit-HasComments: No