You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2017/12/20 03:12:14 UTC
[kudu-CR] Improve scans dashboard
Hello Will Berkeley, Andrew Wong, Todd Lipcon,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/8891
to review the following change.
Change subject: Improve scans dashboard
......................................................................
Improve scans dashboard
* Adds a ring buffer to the scanners manager which holds historical scan
details, so that the scan dashboard can show recently completed scans.
The size of the buffer is configurable with a new experimental flag,
--scan-history-count which defaults to 20. A new 'state' column has
been added to indicate the state of the scan (active, expired, failed,
or complete).
* Mustache-ifies the scans dashboard.
* The dashboard previously showed raw values, with pretty-printed values
as a tooltip. In order to match the maintenance manager dashboard,
these have been swapped so that pretty-printed values are more
prominent.
* Adds a new pseudo-SQL query description column, replacing the
predicates columns. This allows the table, projected columns, and
optimized predicates to be shown in a very concise way. Previously the
table name and projected columns weren't exposed at all.
* Removes 'from disk' from statistics names, since it was misleading, or
downright wrong. The tooltip for the cells, bytes, and cblocks read
columns now indicates that the values could have been read from cache or
from disk. It would probably be nice to add a cache hit rate to the
stats, but I didn't want to include it in this commit as it's already
big.
* Adds aggregated totals for scan stats.
* A bunch of other polish, like tooltip descriptions of the
non-completely-obvious column headers, and adding a link from the
tablet ID to the replica's page.
Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
example: https://i.imgur.com/WLMK263.png
---
M src/kudu/cfile/cfile_reader.cc
M src/kudu/common/column_predicate-test.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/encoded_key.cc
M src/kudu/common/iterator_stats.cc
M src/kudu/common/iterator_stats.h
M src/kudu/common/scan_spec-test.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver_path_handlers.cc
M src/kudu/tserver/tserver_path_handlers.h
A www/scans.mustache
15 files changed, 448 insertions(+), 219 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/8891/1
--
To view, visit http://gerrit.cloudera.org:8080/8891
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
Gerrit-Change-Number: 8891
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] Improve scans dashboard
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8891 )
Change subject: Improve scans dashboard
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8891/2/src/kudu/tablet/cfile_set-test.cc
File src/kudu/tablet/cfile_set-test.cc:
http://gerrit.cloudera.org:8080/#/c/8891/2/src/kudu/tablet/cfile_set-test.cc@319
PS2, Line 319: EXPECT_EQ(stats[0].cblocks_read, 1);
: EXPECT_EQ(stats[1].cblocks_read, 1);
: EXPECT_EQ(stats[2].cblocks_read, 1);
> nit (since the change list touches these lines): the expected value comes f
Done
--
To view, visit http://gerrit.cloudera.org:8080/8891
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
Gerrit-Change-Number: 8891
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
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: Wed, 20 Dec 2017 20:19:44 +0000
Gerrit-HasComments: Yes
[kudu-CR] Improve scans dashboard
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Tidy Bot, Andrew Wong, Kudu Jenkins, Andrew Wong, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8891
to look at the new patch set (#2).
Change subject: Improve scans dashboard
......................................................................
Improve scans dashboard
* Adds a ring buffer to the scanners manager which holds historical scan
details, so that the scan dashboard can show recently completed scans.
The size of the buffer is configurable with a new experimental flag,
--scan-history-count which defaults to 20. A new 'state' column has
been added to indicate the state of the scan (active, expired, failed,
or complete).
* Mustache-ifies the scans dashboard.
* The dashboard previously showed raw values, with pretty-printed values
as a tooltip. In order to match the maintenance manager dashboard,
these have been swapped so that pretty-printed values are more
prominent.
* Adds a new pseudo-SQL query description column, replacing the
predicates columns. This allows the table, projected columns, and
optimized predicates to be shown in a very concise way. Previously the
table name and projected columns weren't exposed at all.
* Removes 'from disk' from statistics names, since it was misleading, or
downright wrong. The tooltip for the cells, bytes, and cblocks read
columns now indicates that the values could have been read from cache or
from disk. It would probably be nice to add a cache hit rate to the
stats, but I didn't want to include it in this commit as it's already
big.
* Adds aggregated totals for scan stats.
* A bunch of other polish, like tooltip descriptions of the
non-completely-obvious column headers, and adding a link from the
tablet ID to the replica's page.
example: https://i.imgur.com/WLMK263.png
Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
---
M src/kudu/cfile/cfile_reader.cc
M src/kudu/common/column_predicate-test.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/encoded_key.cc
M src/kudu/common/iterator_stats.cc
M src/kudu/common/iterator_stats.h
M src/kudu/common/scan_spec-test.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver_path_handlers.cc
M src/kudu/tserver/tserver_path_handlers.h
A www/scans.mustache
16 files changed, 464 insertions(+), 237 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/8891/2
--
To view, visit http://gerrit.cloudera.org:8080/8891
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
Gerrit-Change-Number: 8891
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.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] Improve scans dashboard
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Tidy Bot, Alexey Serbin, Andrew Wong, Kudu Jenkins, Andrew Wong, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8891
to look at the new patch set (#4).
Change subject: Improve scans dashboard
......................................................................
Improve scans dashboard
* Adds a ring buffer to the scanners manager which holds historical scan
details, so that the scan dashboard can show recently completed scans.
The size of the buffer is configurable with a new experimental flag,
--scan-history-count which defaults to 20. A new 'state' column has
been added to indicate the state of the scan (active, expired, failed,
or complete).
* Mustache-ifies the scans dashboard.
* The dashboard previously showed raw values, with pretty-printed values
as a tooltip. In order to match the maintenance manager dashboard,
these have been swapped so that pretty-printed values are more
prominent.
* Adds a new pseudo-SQL query description column, replacing the
predicates columns. This allows the table, projected columns, and
optimized predicates to be shown in a very concise way. Previously the
table name and projected columns weren't exposed at all.
* Removes 'from disk' from statistics names, since it was misleading, or
downright wrong. The tooltip for the cells, bytes, and cblocks read
columns now indicates that the values could have been read from cache or
from disk. It would probably be nice to add a cache hit rate to the
stats, but I didn't want to include it in this commit as it's already
big.
* Adds aggregated totals for scan stats.
* A bunch of other polish, like tooltip descriptions of the
non-completely-obvious column headers, and adding a link from the
tablet ID to the replica's page.
example: https://i.imgur.com/WLMK263.png
Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
---
M src/kudu/cfile/cfile_reader.cc
M src/kudu/common/column_predicate-test.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/encoded_key.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/iterator_stats.cc
M src/kudu/common/iterator_stats.h
M src/kudu/common/scan_spec-test.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver_path_handlers.cc
M src/kudu/tserver/tserver_path_handlers.h
A www/scans.mustache
17 files changed, 505 insertions(+), 251 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/8891/4
--
To view, visit http://gerrit.cloudera.org:8080/8891
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
Gerrit-Change-Number: 8891
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] Improve scans dashboard
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Tidy Bot, Alexey Serbin, Andrew Wong, Kudu Jenkins, Andrew Wong, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8891
to look at the new patch set (#5).
Change subject: Improve scans dashboard
......................................................................
Improve scans dashboard
* Adds a ring buffer to the scanners manager which holds historical scan
details, so that the scan dashboard can show recently completed scans.
The size of the buffer is configurable with a new experimental flag,
--scan-history-count which defaults to 20. A new 'state' column has
been added to indicate the state of the scan (active, expired, failed,
or complete).
* Mustache-ifies the scans dashboard.
* The dashboard previously showed raw values, with pretty-printed values
as a tooltip. In order to match the maintenance manager dashboard,
these have been swapped so that pretty-printed values are more
prominent.
* Adds a new pseudo-SQL query description column, replacing the
predicates columns. This allows the table, projected columns, and
optimized predicates to be shown in a very concise way. Previously the
table name and projected columns weren't exposed at all.
* Removes 'from disk' from statistics names, since it was misleading, or
downright wrong. The tooltip for the cells, bytes, and cblocks read
columns now indicates that the values could have been read from cache or
from disk. It would probably be nice to add a cache hit rate to the
stats, but I didn't want to include it in this commit as it's already
big.
* Adds aggregated totals for scan stats.
* A bunch of other polish, like tooltip descriptions of the
non-completely-obvious column headers, and adding a link from the
tablet ID to the replica's page.
example: https://i.imgur.com/WLMK263.png
Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
---
M src/kudu/cfile/cfile_reader.cc
M src/kudu/common/column_predicate-test.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/encoded_key.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/iterator_stats.cc
M src/kudu/common/iterator_stats.h
M src/kudu/common/scan_spec-test.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver_path_handlers.cc
M src/kudu/tserver/tserver_path_handlers.h
A www/scans.mustache
17 files changed, 506 insertions(+), 251 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/8891/5
--
To view, visit http://gerrit.cloudera.org:8080/8891
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
Gerrit-Change-Number: 8891
Gerrit-PatchSet: 5
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] Improve scans dashboard
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has removed Andrew Wong from this change. ( http://gerrit.cloudera.org:8080/8891 )
Change subject: Improve scans dashboard
......................................................................
Removed reviewer Andrew Wong.
--
To view, visit http://gerrit.cloudera.org:8080/8891
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
Gerrit-Change-Number: 8891
Gerrit-PatchSet: 5
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] Improve scans dashboard
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8891 )
Change subject: Improve scans dashboard
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/8891/1/src/kudu/tserver/scanners.cc
File src/kudu/tserver/scanners.cc:
http://gerrit.cloudera.org:8080/#/c/8891/1/src/kudu/tserver/scanners.cc@164
PS1, Line 164: anDe
> const auto&?
The type here is an iterator, so it's a local.
http://gerrit.cloudera.org:8080/#/c/8891/2/src/kudu/tserver/scanners.cc
File src/kudu/tserver/scanners.cc:
http://gerrit.cloudera.org:8080/#/c/8891/2/src/kudu/tserver/scanners.cc@282
PS2, Line 282: %
> Mod is division is expensive and this is under lock, and we'd be modding ev
I was hoping the optimizer would figure this out, since the addition is by 1, but apparently not: https://godbolt.org/g/Er2Xhz. Fixed!
--
To view, visit http://gerrit.cloudera.org:8080/8891
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
Gerrit-Change-Number: 8891
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
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: Wed, 20 Dec 2017 19:13:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] Improve scans dashboard
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8891
to look at the new patch set (#6).
Change subject: Improve scans dashboard
......................................................................
Improve scans dashboard
* Adds a ring buffer to the scanners manager which holds historical scan
details, so that the scan dashboard can show recently completed scans.
The size of the buffer is configurable with a new experimental flag,
--scan-history-count which defaults to 20. A new 'state' column has
been added to indicate the state of the scan (active, expired, failed,
or complete).
* Mustache-ifies the scans dashboard.
* The dashboard previously showed raw values, with pretty-printed values
as a tooltip. In order to match the maintenance manager dashboard,
these have been swapped so that pretty-printed values are more
prominent.
* Adds a new pseudo-SQL query description column, replacing the
predicates columns. This allows the table, projected columns, and
optimized predicates to be shown in a very concise way. Previously the
table name and projected columns weren't exposed at all.
* Removes 'from disk' from statistics names, since it was misleading, or
downright wrong. The tooltip for the cells, bytes, and cblocks read
columns now indicates that the values could have been read from cache or
from disk. It would probably be nice to add a cache hit rate to the
stats, but I didn't want to include it in this commit as it's already
big.
* Adds aggregated totals for scan stats.
* A bunch of other polish, like tooltip descriptions of the
non-completely-obvious column headers, and adding a link from the
tablet ID to the replica's page.
example: https://i.imgur.com/WLMK263.png
Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
---
M src/kudu/cfile/cfile_reader.cc
M src/kudu/common/column_predicate-test.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/encoded_key.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/iterator_stats.cc
M src/kudu/common/iterator_stats.h
M src/kudu/common/scan_spec-test.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver_path_handlers.cc
M src/kudu/tserver/tserver_path_handlers.h
A www/scans.mustache
17 files changed, 508 insertions(+), 252 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/8891/6
--
To view, visit http://gerrit.cloudera.org:8080/8891
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
Gerrit-Change-Number: 8891
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] Improve scans dashboard
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Tidy Bot, Andrew Wong, Kudu Jenkins, Andrew Wong, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8891
to look at the new patch set (#3).
Change subject: Improve scans dashboard
......................................................................
Improve scans dashboard
* Adds a ring buffer to the scanners manager which holds historical scan
details, so that the scan dashboard can show recently completed scans.
The size of the buffer is configurable with a new experimental flag,
--scan-history-count which defaults to 20. A new 'state' column has
been added to indicate the state of the scan (active, expired, failed,
or complete).
* Mustache-ifies the scans dashboard.
* The dashboard previously showed raw values, with pretty-printed values
as a tooltip. In order to match the maintenance manager dashboard,
these have been swapped so that pretty-printed values are more
prominent.
* Adds a new pseudo-SQL query description column, replacing the
predicates columns. This allows the table, projected columns, and
optimized predicates to be shown in a very concise way. Previously the
table name and projected columns weren't exposed at all.
* Removes 'from disk' from statistics names, since it was misleading, or
downright wrong. The tooltip for the cells, bytes, and cblocks read
columns now indicates that the values could have been read from cache or
from disk. It would probably be nice to add a cache hit rate to the
stats, but I didn't want to include it in this commit as it's already
big.
* Adds aggregated totals for scan stats.
* A bunch of other polish, like tooltip descriptions of the
non-completely-obvious column headers, and adding a link from the
tablet ID to the replica's page.
example: https://i.imgur.com/WLMK263.png
Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
---
M src/kudu/cfile/cfile_reader.cc
M src/kudu/common/column_predicate-test.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/encoded_key.cc
M src/kudu/common/iterator_stats.cc
M src/kudu/common/iterator_stats.h
M src/kudu/common/scan_spec-test.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver_path_handlers.cc
M src/kudu/tserver/tserver_path_handlers.h
A www/scans.mustache
16 files changed, 476 insertions(+), 237 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/8891/3
--
To view, visit http://gerrit.cloudera.org:8080/8891
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
Gerrit-Change-Number: 8891
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.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] Improve scans dashboard
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8891 )
Change subject: Improve scans dashboard
......................................................................
Patch Set 3:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/8891/2/src/kudu/common/iterator_stats.cc
File src/kudu/common/iterator_stats.cc:
http://gerrit.cloudera.org:8080/#/c/8891/2/src/kudu/common/iterator_stats.cc@40
PS2, Line 40: void IteratorStats::AddStats(const IteratorStats& other) {
: cells_read += other.cells_read;
: bytes_read += other.bytes_read;
: cblocks_read += other.cblocks_read;
: DCheckNonNegative();
: }
:
: void IteratorStats::SubtractStats(const IteratorStats& other) {
: cells_read -= other.cells_read;
: bytes_read -= other.bytes_read;
: cblocks_read -= other.cblocks_read;
: DCheckNonNegative();
: }
syntactic sugar nit: these might be operator -= and operator +=; members of the IteratorStats class or friend functions.
http://gerrit.cloudera.org:8080/#/c/8891/2/src/kudu/tablet/cfile_set-test.cc
File src/kudu/tablet/cfile_set-test.cc:
http://gerrit.cloudera.org:8080/#/c/8891/2/src/kudu/tablet/cfile_set-test.cc@319
PS2, Line 319: EXPECT_EQ(stats[0].cblocks_read, 1);
: EXPECT_EQ(stats[1].cblocks_read, 1);
: EXPECT_EQ(stats[2].cblocks_read, 1);
nit (since the change list touches these lines): the expected value comes first.
Also, while you are at it, maybe add ASSERT_GE(stats.size(), 3) check prior to accessing 'stats' elements.
http://gerrit.cloudera.org:8080/#/c/8891/2/src/kudu/tserver/scanners.h
File src/kudu/tserver/scanners.h:
http://gerrit.cloudera.org:8080/#/c/8891/2/src/kudu/tserver/scanners.h@104
PS2, Line 104: std::vector<ScanDescriptor> ListScans();
nit: const?
http://gerrit.cloudera.org:8080/#/c/8891/2/src/kudu/tserver/tserver_path_handlers.cc
File src/kudu/tserver/tserver_path_handlers.cc:
http://gerrit.cloudera.org:8080/#/c/8891/2/src/kudu/tserver/tserver_path_handlers.cc@571
PS2, Line 571:
nit: re-use 'now' from line 563 ?
--
To view, visit http://gerrit.cloudera.org:8080/8891
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
Gerrit-Change-Number: 8891
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
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: Wed, 20 Dec 2017 19:27:47 +0000
Gerrit-HasComments: Yes
[kudu-CR] Improve scans dashboard
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8891 )
Change subject: Improve scans dashboard
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/8891
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
Gerrit-Change-Number: 8891
Gerrit-PatchSet: 5
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
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: Wed, 20 Dec 2017 21:02:12 +0000
Gerrit-HasComments: No
[kudu-CR] Improve scans dashboard
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8891 )
Change subject: Improve scans dashboard
......................................................................
Improve scans dashboard
* Adds a ring buffer to the scanners manager which holds historical scan
details, so that the scan dashboard can show recently completed scans.
The size of the buffer is configurable with a new experimental flag,
--scan-history-count which defaults to 20. A new 'state' column has
been added to indicate the state of the scan (active, expired, failed,
or complete).
* Mustache-ifies the scans dashboard.
* The dashboard previously showed raw values, with pretty-printed values
as a tooltip. In order to match the maintenance manager dashboard,
these have been swapped so that pretty-printed values are more
prominent.
* Adds a new pseudo-SQL query description column, replacing the
predicates columns. This allows the table, projected columns, and
optimized predicates to be shown in a very concise way. Previously the
table name and projected columns weren't exposed at all.
* Removes 'from disk' from statistics names, since it was misleading, or
downright wrong. The tooltip for the cells, bytes, and cblocks read
columns now indicates that the values could have been read from cache or
from disk. It would probably be nice to add a cache hit rate to the
stats, but I didn't want to include it in this commit as it's already
big.
* Adds aggregated totals for scan stats.
* A bunch of other polish, like tooltip descriptions of the
non-completely-obvious column headers, and adding a link from the
tablet ID to the replica's page.
example: https://i.imgur.com/WLMK263.png
Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
Reviewed-on: http://gerrit.cloudera.org:8080/8891
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/cfile/cfile_reader.cc
M src/kudu/common/column_predicate-test.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/encoded_key.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/iterator_stats.cc
M src/kudu/common/iterator_stats.h
M src/kudu/common/scan_spec-test.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver_path_handlers.cc
M src/kudu/tserver/tserver_path_handlers.h
A www/scans.mustache
17 files changed, 513 insertions(+), 256 deletions(-)
Approvals:
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/8891
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
Gerrit-Change-Number: 8891
Gerrit-PatchSet: 8
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] Improve scans dashboard
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8891 )
Change subject: Improve scans dashboard
......................................................................
Patch Set 2:
(4 comments)
The build failure seems to point to a race with descriptors.
http://gerrit.cloudera.org:8080/#/c/8891/1/src/kudu/common/scan_spec-test.cc
File src/kudu/common/scan_spec-test.cc:
PS1:
Good riddance.
http://gerrit.cloudera.org:8080/#/c/8891/1/src/kudu/tserver/scanners.h
File src/kudu/tserver/scanners.h:
http://gerrit.cloudera.org:8080/#/c/8891/1/src/kudu/tserver/scanners.h@146
PS1, Line 146:
nit: scans.
http://gerrit.cloudera.org:8080/#/c/8891/1/src/kudu/tserver/scanners.cc
File src/kudu/tserver/scanners.cc:
http://gerrit.cloudera.org:8080/#/c/8891/1/src/kudu/tserver/scanners.cc@164
PS1, Line 164: anDe
const auto&?
http://gerrit.cloudera.org:8080/#/c/8891/2/src/kudu/tserver/scanners.cc
File src/kudu/tserver/scanners.cc:
http://gerrit.cloudera.org:8080/#/c/8891/2/src/kudu/tserver/scanners.cc@282
PS2, Line 282: %
Mod is division is expensive and this is under lock, and we'd be modding every completed scan after the buffer fills up. Maybe branch instead of mod?
I suppose this isn't really a hotpath and this version is probably the easier on the eyes, so up to you.
--
To view, visit http://gerrit.cloudera.org:8080/8891
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
Gerrit-Change-Number: 8891
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.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: Wed, 20 Dec 2017 06:56:56 +0000
Gerrit-HasComments: Yes
[kudu-CR] Improve scans dashboard
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8891 )
Change subject: Improve scans dashboard
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/8891
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
Gerrit-Change-Number: 8891
Gerrit-PatchSet: 5
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
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: Thu, 21 Dec 2017 19:24:20 +0000
Gerrit-HasComments: No
[kudu-CR] Improve scans dashboard
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8891
to look at the new patch set (#7).
Change subject: Improve scans dashboard
......................................................................
Improve scans dashboard
* Adds a ring buffer to the scanners manager which holds historical scan
details, so that the scan dashboard can show recently completed scans.
The size of the buffer is configurable with a new experimental flag,
--scan-history-count which defaults to 20. A new 'state' column has
been added to indicate the state of the scan (active, expired, failed,
or complete).
* Mustache-ifies the scans dashboard.
* The dashboard previously showed raw values, with pretty-printed values
as a tooltip. In order to match the maintenance manager dashboard,
these have been swapped so that pretty-printed values are more
prominent.
* Adds a new pseudo-SQL query description column, replacing the
predicates columns. This allows the table, projected columns, and
optimized predicates to be shown in a very concise way. Previously the
table name and projected columns weren't exposed at all.
* Removes 'from disk' from statistics names, since it was misleading, or
downright wrong. The tooltip for the cells, bytes, and cblocks read
columns now indicates that the values could have been read from cache or
from disk. It would probably be nice to add a cache hit rate to the
stats, but I didn't want to include it in this commit as it's already
big.
* Adds aggregated totals for scan stats.
* A bunch of other polish, like tooltip descriptions of the
non-completely-obvious column headers, and adding a link from the
tablet ID to the replica's page.
example: https://i.imgur.com/WLMK263.png
Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
---
M src/kudu/cfile/cfile_reader.cc
M src/kudu/common/column_predicate-test.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/encoded_key.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/iterator_stats.cc
M src/kudu/common/iterator_stats.h
M src/kudu/common/scan_spec-test.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver_path_handlers.cc
M src/kudu/tserver/tserver_path_handlers.h
A www/scans.mustache
17 files changed, 513 insertions(+), 256 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/8891/7
--
To view, visit http://gerrit.cloudera.org:8080/8891
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
Gerrit-Change-Number: 8891
Gerrit-PatchSet: 7
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] Improve scans dashboard
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8891 )
Change subject: Improve scans dashboard
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/8891/2/src/kudu/common/iterator_stats.cc
File src/kudu/common/iterator_stats.cc:
http://gerrit.cloudera.org:8080/#/c/8891/2/src/kudu/common/iterator_stats.cc@40
PS2, Line 40: void IteratorStats::AddStats(const IteratorStats& other) {
: cells_read += other.cells_read;
: bytes_read += other.bytes_read;
: cblocks_read += other.cblocks_read;
: DCheckNonNegative();
: }
:
: void IteratorStats::SubtractStats(const IteratorStats& other) {
: cells_read -= other.cells_read;
: bytes_read -= other.bytes_read;
: cblocks_read -= other.cblocks_read;
: DCheckNonNegative();
: }
> syntactic sugar nit: these might be operator -= and operator +=; members of
Done
http://gerrit.cloudera.org:8080/#/c/8891/2/src/kudu/tserver/scanners.h
File src/kudu/tserver/scanners.h:
http://gerrit.cloudera.org:8080/#/c/8891/2/src/kudu/tserver/scanners.h@104
PS2, Line 104: std::vector<ScanDescriptor> ListScans();
> nit: const?
Done
http://gerrit.cloudera.org:8080/#/c/8891/2/src/kudu/tserver/tserver_path_handlers.cc
File src/kudu/tserver/tserver_path_handlers.cc:
http://gerrit.cloudera.org:8080/#/c/8891/2/src/kudu/tserver/tserver_path_handlers.cc@571
PS2, Line 571:
> nit: re-use 'now' from line 563 ?
good catch.
--
To view, visit http://gerrit.cloudera.org:8080/8891
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
Gerrit-Change-Number: 8891
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
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: Wed, 20 Dec 2017 20:17:47 +0000
Gerrit-HasComments: Yes
[kudu-CR] Improve scans dashboard
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8891 )
Change subject: Improve scans dashboard
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/8891
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a
Gerrit-Change-Number: 8891
Gerrit-PatchSet: 7
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
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: Fri, 22 Dec 2017 19:22:24 +0000
Gerrit-HasComments: No