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 2016/09/01 17:21:54 UTC

[kudu-CR] KUDU-1113 pushed predicates not displaying on scans web page

Will Berkeley has uploaded a new change for review.

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

Change subject: KUDU-1113 pushed predicates not displaying on scans web page
......................................................................

KUDU-1113 pushed predicates not displaying on scans web page

This fixes Scanner so it will display predicates in /scanz. A Scanner
is constructed with a reference to its ScanSpec, but the predicates
in the ScanSpec are removed by the Scanner's iterator as they are
pushed down to lower iterators.

The Scanner now holds a copy of the ScanSpec, from just before the
ScanSpec is pass to the iterator and (potentially) modified.

Change-Id: I21821a1f7b5e4d5e5e1bdec17353ae5f3ef8e408
---
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/tablet_service.cc
2 files changed, 6 insertions(+), 2 deletions(-)


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

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

[kudu-CR] KUDU-1113 pushed predicates not displaying on scans web page

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: KUDU-1113 pushed predicates not displaying on scans web page
......................................................................


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I21821a1f7b5e4d5e5e1bdec17353ae5f3ef8e408
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No

[kudu-CR] KUDU-1113 pushed predicates not displaying on scans web page

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.

Change subject: KUDU-1113 pushed predicates not displaying on scans web page
......................................................................


Patch Set 1:

(1 comment)

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

PS1, Line 1488: can report on predicates.
> think it would be better to say something like 'report its predicates on th
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I21821a1f7b5e4d5e5e1bdec17353ae5f3ef8e408
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1113 pushed predicates not displaying on scans web page

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1113 pushed predicates not displaying on scans web page
......................................................................


Patch Set 1:

(1 comment)

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

Line 1489:   gscoped_ptr<ScanSpec> orig_spec(new ScanSpec(*spec));
> This is done after the scan spec is optimized, so some of the predicates ma
IMO it's nice to show the fully-optimized one to know how efficient the scan is.. but I could see the argument the other way.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I21821a1f7b5e4d5e5e1bdec17353ae5f3ef8e408
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1113 pushed predicates not displaying on scans web page

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.

Change subject: KUDU-1113 pushed predicates not displaying on scans web page
......................................................................


Patch Set 1:

(1 comment)

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

Line 1489:   gscoped_ptr<ScanSpec> orig_spec(new ScanSpec(*spec));
> This is done after the scan spec is optimized, so some of the predicates ma
Yes, I chose that because I think it's best to see the predicates that Kudu is actually evaluating.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I21821a1f7b5e4d5e5e1bdec17353ae5f3ef8e408
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1113 pushed predicates not displaying on scans web page

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

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

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

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

Change subject: KUDU-1113 pushed predicates not displaying on scans web page
......................................................................

KUDU-1113 pushed predicates not displaying on scans web page

This fixes Scanner so it will display predicates in /scanz. A Scanner
is constructed with a reference to its ScanSpec, but the predicates
in the ScanSpec are removed by the Scanner's iterator as they are
pushed down to lower iterators.

The Scanner now holds a copy of the ScanSpec, from just before the
ScanSpec is pass to the iterator and (potentially) modified.

Change-Id: I21821a1f7b5e4d5e5e1bdec17353ae5f3ef8e408
---
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/tablet_service.cc
2 files changed, 8 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I21821a1f7b5e4d5e5e1bdec17353ae5f3ef8e408
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] KUDU-1113 pushed predicates not displaying on scans web page

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1113 pushed predicates not displaying on scans web page
......................................................................


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/3187/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I21821a1f7b5e4d5e5e1bdec17353ae5f3ef8e408
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] KUDU-1113 pushed predicates not displaying on scans web page

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged.

Change subject: KUDU-1113 pushed predicates not displaying on scans web page
......................................................................


KUDU-1113 pushed predicates not displaying on scans web page

This fixes Scanner so it will display predicates in /scanz. A Scanner
is constructed with a reference to its ScanSpec, but the predicates
in the ScanSpec are removed by the Scanner's iterator as they are
pushed down to lower iterators.

The Scanner now holds a copy of the ScanSpec, from just before the
ScanSpec is pass to the iterator and (potentially) modified.

Change-Id: I21821a1f7b5e4d5e5e1bdec17353ae5f3ef8e408
Reviewed-on: http://gerrit.cloudera.org:8080/4206
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <da...@cloudera.com>
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/tablet_service.cc
2 files changed, 8 insertions(+), 2 deletions(-)

Approvals:
  Dan Burkert: Looks good to me, but someone else must approve
  Todd Lipcon: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I21821a1f7b5e4d5e5e1bdec17353ae5f3ef8e408
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] KUDU-1113 pushed predicates not displaying on scans web page

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1113 pushed predicates not displaying on scans web page
......................................................................


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/3232/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I21821a1f7b5e4d5e5e1bdec17353ae5f3ef8e408
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No

[kudu-CR] KUDU-1113 pushed predicates not displaying on scans web page

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1113 pushed predicates not displaying on scans web page
......................................................................


Patch Set 1:

(1 comment)

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

PS1, Line 1488: can report on predicates.
think it would be better to say something like 'report its predicates on the /scans page' or something

also, worth noting in this comment that the copy is necessary because iter->Init() ends up removing predicates that got pushed down into underlying iterators or somesuch


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I21821a1f7b5e4d5e5e1bdec17353ae5f3ef8e408
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1113 pushed predicates not displaying on scans web page

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: KUDU-1113 pushed predicates not displaying on scans web page
......................................................................


Patch Set 1:

(1 comment)

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

Line 1489:   gscoped_ptr<ScanSpec> orig_spec(new ScanSpec(*spec));
This is done after the scan spec is optimized, so some of the predicates may be removed (however in that case they will be represented in the PK bounds), is that OK in your opinion?  The other option is to move the copy before the optimize step.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I21821a1f7b5e4d5e5e1bdec17353ae5f3ef8e408
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1113 pushed predicates not displaying on scans web page

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1113 pushed predicates not displaying on scans web page
......................................................................


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I21821a1f7b5e4d5e5e1bdec17353ae5f3ef8e408
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No