[kudu-CR] [webui] Convert /tablets page to mustache

[Will Berkeley (Code Review)]

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 
Tested-by: Will Berkeley 
Reviewed-by: Andrew Wong 
---
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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] [webui] Convert /tablets page to mustache

[Andrew Wong (Code Review)]

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),
 :));
 :   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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Sun, 20 May 2018 18:58:53 +
Gerrit-HasComments: Yes

[kudu-CR] [webui] Convert /tablets page to mustache

[Will Berkeley (Code Review)]

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),
 :));
 :   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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Sun, 20 May 2018 18:01:23 +
Gerrit-HasComments: Yes

[kudu-CR] [webui] Convert /tablets page to mustache

[Andrew Wong (Code Review)]

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),
 :));
 :   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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Sun, 20 May 2018 17:34:04 +
Gerrit-HasComments: Yes

[kudu-CR] [webui] Convert /tablets page to mustache

[Will Berkeley (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] [webui] Convert /tablets page to mustache

[Will Berkeley (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Sat, 19 May 2018 06:14:20 +
Gerrit-HasComments: No

[kudu-CR] [webui] Convert /tablets page to mustache

[Alexey Serbin (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Sat, 19 May 2018 05:59:42 +
Gerrit-HasComments: No

[kudu-CR] [webui] Convert /tablets page to mustache

[Will Berkeley (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] [webui] Convert /tablets page to mustache

[Will Berkeley (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] [webui] Convert /tablets page to mustache

[Will Berkeley (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 18 May 2018 21:50:46 +
Gerrit-HasComments: No

[kudu-CR] [webui] Convert /tablets page to mustache

[Will Berkeley (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 18 May 2018 21:48:57 +
Gerrit-HasComments: No

[kudu-CR] [webui] Convert /tablets page to mustache

[Will Berkeley (Code Review)]

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:   Write buffer memory usage
:   On-disk size
> 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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 17 May 2018 21:58:24 +
Gerrit-HasComments: Yes

[kudu-CR] [webui] Convert /tablets page to mustache

[Will Berkeley (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [webui] Convert /tablets page to mustache

[Andrew Wong (Code Review)]

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:   Write buffer memory usage
:   On-disk size
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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 17 May 2018 17:57:41 +
Gerrit-HasComments: Yes

[kudu-CR] [webui] Convert /tablets page to mustache

[Alexey Serbin (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 17 May 2018 17:23:32 +
Gerrit-HasComments: Yes

[kudu-CR] [webui] Convert /tablets page to mustache

[Will Berkeley (Code Review)]

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