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 2017/10/17 23:46:38 UTC

[kudu-CR] [webui] Add templates for tserver webui 

Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8307


Change subject: [webui] Add templates for tserver webui
......................................................................

[webui] Add templates for tserver webui

This converts all the tablet server web ui endpoints to use a template,
unless it doesn't make sense to. This includes
- /scans
- /tablets
- /tablet
- /transactions
- /tablet-rowsetlayout-svg
- /tablet-consensus-status
- /log-anchors
- /dashboards

Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
---
M src/kudu/server/webui_util.cc
M src/kudu/tserver/tserver_path_handlers.cc
M src/kudu/tserver/tserver_path_handlers.h
A www/dashboards.mustache
A www/log-anchors.mustache
A www/scans.mustache
A www/tablet-consensus-status.mustache
A www/tablet-rowsetlayout-svg.mustache
A www/tablet.mustache
A www/tablets.mustache
A www/transactions.mustache
11 files changed, 591 insertions(+), 374 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/8307/1
-- 
To view, visit http://gerrit.cloudera.org:8080/8307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
Gerrit-Change-Number: 8307
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>

[kudu-CR] [webui] Add templates for tserver webui

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/8307

to look at the new patch set (#6).

Change subject: [webui] Add templates for tserver webui
......................................................................

[webui] Add templates for tserver webui

This converts all the tablet server web ui endpoints to use a template,
unless it doesn't make sense to. This includes
- /scans
- /tablets
- /tablet
- /transactions
- /tablet-rowsetlayout-svg
- /tablet-consensus-status
- /log-anchors
- /dashboards

Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
---
M src/kudu/server/webserver.cc
M src/kudu/server/webui_util.cc
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/dashboards.mustache
A www/log-anchors.mustache
A www/scans.mustache
M www/table.mustache
A www/tablet-consensus-status.mustache
A www/tablet-rowsetlayout-svg.mustache
A www/tablet.mustache
A www/tablets.mustache
A www/transactions.mustache
14 files changed, 589 insertions(+), 386 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/8307/6
-- 
To view, visit http://gerrit.cloudera.org:8080/8307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
Gerrit-Change-Number: 8307
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [webui] Add templates for tserver webui

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/8307

to look at the new patch set (#2).

Change subject: [webui] Add templates for tserver webui
......................................................................

[webui] Add templates for tserver webui

This converts all the tablet server web ui endpoints to use a template,
unless it doesn't make sense to. This includes
- /scans
- /tablets
- /tablet
- /transactions
- /tablet-rowsetlayout-svg
- /tablet-consensus-status
- /log-anchors
- /dashboards

Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
---
M src/kudu/server/webui_util.cc
M src/kudu/tserver/tserver_path_handlers.cc
M src/kudu/tserver/tserver_path_handlers.h
A www/dashboards.mustache
A www/log-anchors.mustache
A www/scans.mustache
M www/table.mustache
A www/tablet-consensus-status.mustache
A www/tablet-rowsetlayout-svg.mustache
A www/tablet.mustache
A www/tablets.mustache
A www/transactions.mustache
12 files changed, 593 insertions(+), 383 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/8307/2
-- 
To view, visit http://gerrit.cloudera.org:8080/8307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
Gerrit-Change-Number: 8307
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] [webui] Add templates for tserver webui

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/8307 )

Change subject: [webui] Add templates for tserver webui
......................................................................


Patch Set 4:

Not sure what's been going on with these test failures...they've been fine on my machine.


-- 
To view, visit http://gerrit.cloudera.org:8080/8307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
Gerrit-Change-Number: 8307
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 19 Oct 2017 04:18:47 +0000
Gerrit-HasComments: No

[kudu-CR] [webui] Add templates for tserver webui

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8307 )

Change subject: [webui] Add templates for tserver webui
......................................................................


Patch Set 7:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8307/7/src/kudu/tserver/tserver_path_handlers.cc
File src/kudu/tserver/tserver_path_handlers.cc:

http://gerrit.cloudera.org:8080/#/c/8307/7/src/kudu/tserver/tserver_path_handlers.cc@214
PS7, Line 214:   map<TabletStatePB, int> tablet_statuses;
nit: maybe rename this to tablet_states?


http://gerrit.cloudera.org:8080/#/c/8307/7/src/kudu/tserver/tserver_path_handlers.cc@215
PS7, Line 215:   for (const scoped_refptr<TabletReplica>& replica : replicas) {
Can this loop be merged with the other loop over `replicas`? Seems like there could be ample time in the second two loops for `tablet_statuses` to become stale.


http://gerrit.cloudera.org:8080/#/c/8307/7/src/kudu/tserver/tserver_path_handlers.cc@240
PS7, Line 240:       tablet_detail_json["target"] = Substitute("/tablet?id=$0", status.tablet_id());
Does the /tablet page exist if replica exists but the tablet doesn't exist? Or will they be reported in the 'tombstoned tablets' section or something?



-- 
To view, visit http://gerrit.cloudera.org:8080/8307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
Gerrit-Change-Number: 8307
Gerrit-PatchSet: 7
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2017 20:35:24 +0000
Gerrit-HasComments: Yes

[kudu-CR] [webui] Add templates for tserver webui

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/8307

to look at the new patch set (#3).

Change subject: [webui] Add templates for tserver webui
......................................................................

[webui] Add templates for tserver webui

This converts all the tablet server web ui endpoints to use a template,
unless it doesn't make sense to. This includes
- /scans
- /tablets
- /tablet
- /transactions
- /tablet-rowsetlayout-svg
- /tablet-consensus-status
- /log-anchors
- /dashboards

Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
---
M src/kudu/server/webui_util.cc
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/dashboards.mustache
A www/log-anchors.mustache
A www/scans.mustache
M www/table.mustache
A www/tablet-consensus-status.mustache
A www/tablet-rowsetlayout-svg.mustache
A www/tablet.mustache
A www/tablets.mustache
A www/transactions.mustache
13 files changed, 594 insertions(+), 384 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/8307/3
-- 
To view, visit http://gerrit.cloudera.org:8080/8307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
Gerrit-Change-Number: 8307
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [webui] Add templates for tserver webui

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8307 )

Change subject: [webui] Add templates for tserver webui
......................................................................


Patch Set 6:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc
File src/kudu/tserver/tserver_path_handlers.cc:

http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@a156
PS6, Line 156: 
so did we lose the ?raw ability here? I actually use this pretty often with 'curl' to get output in a readable format over an ssh connection. Not sure if JSON would be as readable.


http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@169
PS6, Line 169:         transaction_json["trace"] = inflight_tx.trace_buffer();
Perhaps we should be std::move()ing the large strings out of the PB into the JSON to avoid copy?


http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@210
PS6, Line 210: Subst
std::to_string


http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@220
PS6, Line 220: percentage
do you want to be using StringPrintf here to get a nice printout? it seems like in the template you're just including this directly rather than something like %%%.1f or whatever


http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@225
PS6, Line 225: kArray
I'm surprised this isn't kObject.. how does this work?


http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@235
PS6, Line 235:  if (replica->tablet() != nullptr) {
             :       tablet_detail_json["target"] = Substitute("/tablet?id=$0", status.tablet_id());
             :       tablet_detail_json["mem_bytes"] = HumanReadableNumBytes::ToString(
             :           replica->tablet()
isn't there a race here where tablet() can become null in between, if it's shutting down?


http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@413
PS6, Line 413: Substitute("$0", 
std::to_string() here and a few other spots

(though I thought easyjson could hold ints too)



-- 
To view, visit http://gerrit.cloudera.org:8080/8307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
Gerrit-Change-Number: 8307
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 23 Oct 2017 23:53:13 +0000
Gerrit-HasComments: Yes

[kudu-CR] [webui] Add templates for tserver webui

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Todd Lipcon, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/8307

to look at the new patch set (#7).

Change subject: [webui] Add templates for tserver webui
......................................................................

[webui] Add templates for tserver webui

This converts all the tablet server web ui endpoints to use a template,
unless it doesn't make sense to. This includes
- /scans
- /tablets
- /tablet
- /transactions
- /tablet-rowsetlayout-svg
- /tablet-consensus-status
- /log-anchors
- /dashboards

Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
---
M src/kudu/server/webserver.cc
M src/kudu/server/webui_util.cc
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/dashboards.mustache
A www/log-anchors.mustache
A www/scans.mustache
M www/table.mustache
A www/tablet-consensus-status.mustache
A www/tablet-rowsetlayout-svg.mustache
A www/tablet.mustache
A www/tablets.mustache
A www/transactions.mustache
14 files changed, 602 insertions(+), 384 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/8307/7
-- 
To view, visit http://gerrit.cloudera.org:8080/8307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
Gerrit-Change-Number: 8307
Gerrit-PatchSet: 7
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [webui] Add templates for tserver webui

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/8307 )

Change subject: [webui] Add templates for tserver webui
......................................................................


Patch Set 7:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/8307/7/src/kudu/tserver/tserver_path_handlers.h
File src/kudu/tserver/tserver_path_handlers.h:

http://gerrit.cloudera.org:8080/#/c/8307/7/src/kudu/tserver/tserver_path_handlers.h@27
PS7, Line 27: template <class T> class scoped_refptr;
> warning: invalid case style for class 'scoped_refptr' [readability-identifi
:(


http://gerrit.cloudera.org:8080/#/c/8307/7/src/kudu/tserver/tserver_path_handlers.cc
File src/kudu/tserver/tserver_path_handlers.cc:

http://gerrit.cloudera.org:8080/#/c/8307/7/src/kudu/tserver/tserver_path_handlers.cc@170
PS7, Line 170:       transaction_json["desc"] = std::move(inflight_tx.description());
> warning: std::move of the const expression has no effect; remove std::move(
Done


http://gerrit.cloudera.org:8080/#/c/8307/7/src/kudu/tserver/tserver_path_handlers.cc@172
PS7, Line 172:         transaction_json["trace"] = std::move(inflight_tx.trace_buffer());
> warning: std::move of the const expression has no effect; remove std::move(
Done


http://gerrit.cloudera.org:8080/#/c/8307/7/src/kudu/tserver/tserver_path_handlers.cc@214
PS7, Line 214:   map<TabletStatePB, int> tablet_statuses;
> nit: maybe rename this to tablet_states?
Done


http://gerrit.cloudera.org:8080/#/c/8307/7/src/kudu/tserver/tserver_path_handlers.cc@215
PS7, Line 215:   for (const scoped_refptr<TabletReplica>& replica : replicas) {
> Can this loop be merged with the other loop over `replicas`? Seems like the
Done


http://gerrit.cloudera.org:8080/#/c/8307/7/src/kudu/tserver/tserver_path_handlers.cc@240
PS7, Line 240:       tablet_detail_json["target"] = Substitute("/tablet?id=$0", status.tablet_id());
> Does the /tablet page exist if replica exists but the tablet doesn't exist?
Technically the /tablet page does exist if the replica exists but the tablet doesn't, but there's not going to be anything useful about it anymore.



-- 
To view, visit http://gerrit.cloudera.org:8080/8307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
Gerrit-Change-Number: 8307
Gerrit-PatchSet: 7
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 30 Oct 2017 22:10:54 +0000
Gerrit-HasComments: Yes

[kudu-CR] [webui] Add templates for tserver webui

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/8307

to look at the new patch set (#4).

Change subject: [webui] Add templates for tserver webui
......................................................................

[webui] Add templates for tserver webui

This converts all the tablet server web ui endpoints to use a template,
unless it doesn't make sense to. This includes
- /scans
- /tablets
- /tablet
- /transactions
- /tablet-rowsetlayout-svg
- /tablet-consensus-status
- /log-anchors
- /dashboards

Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
---
M src/kudu/server/webui_util.cc
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/dashboards.mustache
A www/log-anchors.mustache
A www/scans.mustache
M www/table.mustache
A www/tablet-consensus-status.mustache
A www/tablet-rowsetlayout-svg.mustache
A www/tablet.mustache
A www/tablets.mustache
A www/transactions.mustache
13 files changed, 594 insertions(+), 383 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/8307/4
-- 
To view, visit http://gerrit.cloudera.org:8080/8307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
Gerrit-Change-Number: 8307
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [webui] Add templates for tserver webui

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/8307 )

Change subject: [webui] Add templates for tserver webui
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8307/1/src/kudu/tserver/tserver_path_handlers.h
File src/kudu/tserver/tserver_path_handlers.h:

http://gerrit.cloudera.org:8080/#/c/8307/1/src/kudu/tserver/tserver_path_handlers.h@79
PS1, Line 79:   void IteratorStatsToJson(const Schema& projection,
> warning: function 'kudu::tserver::TabletServerPathHandlers::TabletsToJson' 
Done


http://gerrit.cloudera.org:8080/#/c/8307/1/src/kudu/tserver/tserver_path_handlers.cc
File src/kudu/tserver/tserver_path_handlers.cc:

http://gerrit.cloudera.org:8080/#/c/8307/1/src/kudu/tserver/tserver_path_handlers.cc@221
PS1, Line 221:   }
> warning: the 'empty' method should be used to check for emptiness instead o
Done


http://gerrit.cloudera.org:8080/#/c/8307/1/src/kudu/tserver/tserver_path_handlers.cc@477
PS1, Line 477:                                                     Webserver::WebResponse* resp) {
> warning: parameter 'req' is unused [misc-unused-parameters]
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/8307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
Gerrit-Change-Number: 8307
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 18 Oct 2017 15:45:17 +0000
Gerrit-HasComments: Yes

[kudu-CR] [webui] Add templates for tserver webui

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/8307

to look at the new patch set (#5).

Change subject: [webui] Add templates for tserver webui
......................................................................

[webui] Add templates for tserver webui

This converts all the tablet server web ui endpoints to use a template,
unless it doesn't make sense to. This includes
- /scans
- /tablets
- /tablet
- /transactions
- /tablet-rowsetlayout-svg
- /tablet-consensus-status
- /log-anchors
- /dashboards

Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
---
M src/kudu/server/webui_util.cc
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/dashboards.mustache
A www/log-anchors.mustache
A www/scans.mustache
M www/table.mustache
A www/tablet-consensus-status.mustache
A www/tablet-rowsetlayout-svg.mustache
A www/tablet.mustache
A www/tablets.mustache
A www/transactions.mustache
13 files changed, 596 insertions(+), 385 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/8307/5
-- 
To view, visit http://gerrit.cloudera.org:8080/8307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
Gerrit-Change-Number: 8307
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [webui] Add templates for tserver webui

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has abandoned this change. ( http://gerrit.cloudera.org:8080/8307 )

Change subject: [webui] Add templates for tserver webui
......................................................................


Abandoned

I'm doing this as it comes up for specific endpoints.
-- 
To view, visit http://gerrit.cloudera.org:8080/8307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
Gerrit-Change-Number: 8307
Gerrit-PatchSet: 8
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [webui] Add templates for tserver webui

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Todd Lipcon, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/8307

to look at the new patch set (#8).

Change subject: [webui] Add templates for tserver webui
......................................................................

[webui] Add templates for tserver webui

This converts all the tablet server web ui endpoints to use a template,
unless it doesn't make sense to. This includes
- /scans
- /tablets
- /tablet
- /transactions
- /tablet-rowsetlayout-svg
- /tablet-consensus-status
- /log-anchors
- /dashboards

Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
---
M src/kudu/server/webserver.cc
M src/kudu/server/webui_util.cc
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/dashboards.mustache
A www/log-anchors.mustache
A www/scans.mustache
M www/table.mustache
A www/tablet-consensus-status.mustache
A www/tablet-rowsetlayout-svg.mustache
A www/tablet.mustache
A www/tablets.mustache
A www/transactions.mustache
14 files changed, 604 insertions(+), 384 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/8307/8
-- 
To view, visit http://gerrit.cloudera.org:8080/8307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
Gerrit-Change-Number: 8307
Gerrit-PatchSet: 8
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [webui] Add templates for tserver webui

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/8307 )

Change subject: [webui] Add templates for tserver webui
......................................................................


Patch Set 6:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.h
File src/kudu/tserver/tserver_path_handlers.h:

http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.h@27
PS6, Line 27: template <class T> class scoped_refptr;
> warning: invalid case style for class 'scoped_refptr' [readability-identifi
Not my fault, tidy bot.


http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc
File src/kudu/tserver/tserver_path_handlers.cc:

http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@a156
PS6, Line 156: 
> so did we lose the ?raw ability here? I actually use this pretty often with
We did, because I wasn't sure anyone used it. Since someone does, I'll keep it.


http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@169
PS6, Line 169:         transaction_json["trace"] = inflight_tx.trace_buffer();
> Perhaps we should be std::move()ing the large strings out of the PB into th
Done


http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@210
PS6, Line 210: Subst
> std::to_string
Done


http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@220
PS6, Line 220: percentage
> do you want to be using StringPrintf here to get a nice printout? it seems 
Done


http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@225
PS6, Line 225: kArray
> I'm surprised this isn't kObject.. how does this work?
Yeah, that's a mistake. I think it works because EasyJson replaces the array value with an object value because I'm doing object-y things to it.


http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@235
PS6, Line 235:  if (replica->tablet() != nullptr) {
             :       tablet_detail_json["target"] = Substitute("/tablet?id=$0", status.tablet_id());
             :       tablet_detail_json["mem_bytes"] = HumanReadableNumBytes::ToString(
             :           replica->tablet()
> isn't there a race here where tablet() can become null in between, if it's 
Done


http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@413
PS6, Line 413: Substitute("$0", 
> std::to_string() here and a few other spots
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/8307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
Gerrit-Change-Number: 8307
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2017 17:56:32 +0000
Gerrit-HasComments: Yes