You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org> on 2018/04/19 17:40:34 UTC

[Impala-ASF-CR] IMPALA-6821: Push down limits into Kudu 

Thomas Tauber-Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10119


Change subject: IMPALA-6821: Push down limits into Kudu
......................................................................

IMPALA-6821: Push down limits into Kudu

This patch takes advantage of a recent change in Kudu (KUDU-16) that
exposes the ability to set limits on KuduScanners. Since each
KuduScanner corresponds to a scan token, and there will be multiple
scan tokens per query, this is just a performance optimization in
cases where the limit is smaller than the number of rows per token,
and Impala still needs to apply the limit on our side for cases where
the limit is greater than the number of rows per token.

Testing:
- Updated test_limit to run against Kudu by moving the few test cases
  that Kudu cannot support to a different test.
- For the query 'select * from tpch_kudu.lineitem limit 1', a best
  case perf scenario for this change where the limit is highly
  effective, the time spent in the Kudu scan node was reduced from
  6.107ms to 3.498ms (avg over 3 runs).
- For the query 'select count(*) from tpch_kudu.lineitem limit
  1000000', a worst case perf scenario for this change where the limit
  is ineffective, the time spent in the Kudu scan node was essentially
  unchanged, 62.387ms previously vs. 64.168ms (avg over 3 runs).

Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
---
M be/src/exec/kudu-scanner.cc
A testdata/workloads/functional-query/queries/QueryTest/limit-nondeterministic.test
M testdata/workloads/functional-query/queries/QueryTest/limit.test
M tests/query_test/test_queries.py
4 files changed, 87 insertions(+), 73 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/10119/1
-- 
To view, visit http://gerrit.cloudera.org:8080/10119
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
Gerrit-Change-Number: 10119
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-6821: Push down limits into Kudu

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10119 )

Change subject: IMPALA-6821: Push down limits into Kudu
......................................................................


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2377/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
Gerrit-Change-Number: 10119
Gerrit-PatchSet: 6
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Apr 2018 17:58:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6821: Push down limits into Kudu

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

Change subject: IMPALA-6821: Push down limits into Kudu
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10119/3/testdata/workloads/functional-query/queries/QueryTest/kudu_limit.test
File testdata/workloads/functional-query/queries/QueryTest/kudu_limit.test:

http://gerrit.cloudera.org:8080/#/c/10119/3/testdata/workloads/functional-query/queries/QueryTest/kudu_limit.test@11
PS3, Line 11: select * from functional_kudu.alltypes order by id limit 2
Order by limit means there is no limit on the scan node - I don't think that's what we want to test. We want to test limits on the Kudu scan node with and without predicates.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
Gerrit-Change-Number: 10119
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 25 Apr 2018 18:05:52 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6821: Push down limits into Kudu

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10119 )

Change subject: IMPALA-6821: Push down limits into Kudu
......................................................................


Patch Set 6: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
Gerrit-Change-Number: 10119
Gerrit-PatchSet: 6
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Apr 2018 21:55:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6821: Push down limits into Kudu

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/10119 )

Change subject: IMPALA-6821: Push down limits into Kudu
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10119/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10119/1//COMMIT_MSG@24
PS1, Line 24: - For the query 'select count(*) from (select * from
> This query doesn't have a limit on the scan. You need to use an inline view
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
Gerrit-Change-Number: 10119
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Fri, 20 Apr 2018 19:01:22 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6821: Push down limits into Kudu

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/10119 )

Change subject: IMPALA-6821: Push down limits into Kudu
......................................................................


Patch Set 6: Code-Review+2

gvo failed due to:
- clang-format issue that has been fixed (added KUDU_RETURN_IF_ERROR)
- unrelated test failure

carrying forward


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
Gerrit-Change-Number: 10119
Gerrit-PatchSet: 6
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Apr 2018 17:57:52 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6821: Push down limits into Kudu

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/10119 )

Change subject: IMPALA-6821: Push down limits into Kudu
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10119/3/testdata/workloads/functional-query/queries/QueryTest/kudu_limit.test
File testdata/workloads/functional-query/queries/QueryTest/kudu_limit.test:

http://gerrit.cloudera.org:8080/#/c/10119/3/testdata/workloads/functional-query/queries/QueryTest/kudu_limit.test@11
PS3, Line 11: select count(*) from (select * from functional_kudu.alltypes limit 2) v
> Order by limit means there is no limit on the scan node - I don't think tha
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
Gerrit-Change-Number: 10119
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Apr 2018 22:51:33 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6821: Push down limits into Kudu

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

Change subject: IMPALA-6821: Push down limits into Kudu
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10119/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10119/1//COMMIT_MSG@24
PS1, Line 24: - For the query 'select count(*) from tpch_kudu.lineitem limit
This query doesn't have a limit on the scan. You need to use an inline view.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
Gerrit-Change-Number: 10119
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Comment-Date: Thu, 19 Apr 2018 23:49:46 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6821: Push down limits into Kudu

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10119 )

Change subject: IMPALA-6821: Push down limits into Kudu
......................................................................


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2374/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
Gerrit-Change-Number: 10119
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Apr 2018 23:35:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6821: Push down limits into Kudu

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Hello Alex Behm, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-6821: Push down limits into Kudu
......................................................................

IMPALA-6821: Push down limits into Kudu

This patch takes advantage of a recent change in Kudu (KUDU-16) that
exposes the ability to set limits on KuduScanners. Since each
KuduScanner corresponds to a scan token, and there will be multiple
scan tokens per query, this is just a performance optimization in
cases where the limit is smaller than the number of rows per token,
and Impala still needs to apply the limit on our side for cases where
the limit is greater than the number of rows per token.

Testing:
- Added e2e tests for various situations where limits are applied at
  a Kudu scan node.
- For the query 'select * from tpch_kudu.lineitem limit 1', a best
  case perf scenario for this change where the limit is highly
  effective, the time spent in the Kudu scan node was reduced from
  6.107ms to 3.498ms (avg over 3 runs).
- For the query 'select count(*) from (select * from
  tpch_kudu.lineitem limit 1000000) v', a worst case perf scenario for
  this change where the limit is ineffective, the time spent in the
  Kudu scan node was essentially unchanged, 32.815ms previously vs.
  29.532ms (avg over 3 runs).

Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
---
M be/src/exec/kudu-scanner.cc
A testdata/workloads/functional-query/queries/QueryTest/kudu_limit.test
M tests/query_test/test_kudu.py
3 files changed, 120 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/10119/6
-- 
To view, visit http://gerrit.cloudera.org:8080/10119
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
Gerrit-Change-Number: 10119
Gerrit-PatchSet: 6
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-6821: Push down limits into Kudu

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10119 )

Change subject: IMPALA-6821: Push down limits into Kudu
......................................................................


Patch Set 5: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2374/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
Gerrit-Change-Number: 10119
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Apr 2018 03:33:52 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6821: Push down limits into Kudu

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/10119 )

Change subject: IMPALA-6821: Push down limits into Kudu
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10119/2/tests/query_test/test_queries.py
File tests/query_test/test_queries.py:

http://gerrit.cloudera.org:8080/#/c/10119/2/tests/query_test/test_queries.py@60
PS2, Line 60:     self.run_test_case('QueryTest/limit', vector)
> I'd prefer to revert this move and add a new test specifically for Kudu in 
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
Gerrit-Change-Number: 10119
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Apr 2018 21:14:49 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6821: Push down limits into Kudu

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

Change subject: IMPALA-6821: Push down limits into Kudu
......................................................................


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
Gerrit-Change-Number: 10119
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Apr 2018 23:31:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6821: Push down limits into Kudu

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10119 )

Change subject: IMPALA-6821: Push down limits into Kudu
......................................................................

IMPALA-6821: Push down limits into Kudu

This patch takes advantage of a recent change in Kudu (KUDU-16) that
exposes the ability to set limits on KuduScanners. Since each
KuduScanner corresponds to a scan token, and there will be multiple
scan tokens per query, this is just a performance optimization in
cases where the limit is smaller than the number of rows per token,
and Impala still needs to apply the limit on our side for cases where
the limit is greater than the number of rows per token.

Testing:
- Added e2e tests for various situations where limits are applied at
  a Kudu scan node.
- For the query 'select * from tpch_kudu.lineitem limit 1', a best
  case perf scenario for this change where the limit is highly
  effective, the time spent in the Kudu scan node was reduced from
  6.107ms to 3.498ms (avg over 3 runs).
- For the query 'select count(*) from (select * from
  tpch_kudu.lineitem limit 1000000) v', a worst case perf scenario for
  this change where the limit is ineffective, the time spent in the
  Kudu scan node was essentially unchanged, 32.815ms previously vs.
  29.532ms (avg over 3 runs).

Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
Reviewed-on: http://gerrit.cloudera.org:8080/10119
Reviewed-by: Thomas Tauber-Marshall <tm...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exec/kudu-scanner.cc
A testdata/workloads/functional-query/queries/QueryTest/kudu_limit.test
M tests/query_test/test_kudu.py
3 files changed, 120 insertions(+), 0 deletions(-)

Approvals:
  Thomas Tauber-Marshall: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
Gerrit-Change-Number: 10119
Gerrit-PatchSet: 7
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-6821: Push down limits into Kudu

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

Change subject: IMPALA-6821: Push down limits into Kudu
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10119/2/tests/query_test/test_queries.py
File tests/query_test/test_queries.py:

http://gerrit.cloudera.org:8080/#/c/10119/2/tests/query_test/test_queries.py@60
PS2, Line 60:   def test_limit_nondeterminstic(self, vector):
I'd prefer to revert this move and add a new test specifically for Kudu in kudu_test.py. I assume Kudu has tested these scenarios, but it would be good to confirm on our side that the limit works correctly in the following situations:
* No predicates
* PK Kudu predicate only
* non-PK Kudu predicate only
* Kudu predicates and Impala predicates
* Any other interesting cases with predicates you can think of? Anything special about partitioning columns maybe?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
Gerrit-Change-Number: 10119
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Fri, 20 Apr 2018 22:08:17 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6821: Push down limits into Kudu

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Hello Alex Behm, 

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

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

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

Change subject: IMPALA-6821: Push down limits into Kudu
......................................................................

IMPALA-6821: Push down limits into Kudu

This patch takes advantage of a recent change in Kudu (KUDU-16) that
exposes the ability to set limits on KuduScanners. Since each
KuduScanner corresponds to a scan token, and there will be multiple
scan tokens per query, this is just a performance optimization in
cases where the limit is smaller than the number of rows per token,
and Impala still needs to apply the limit on our side for cases where
the limit is greater than the number of rows per token.

Testing:
- Added e2e tests for various situations where limits are applied at
  a Kudu scan node.
- For the query 'select * from tpch_kudu.lineitem limit 1', a best
  case perf scenario for this change where the limit is highly
  effective, the time spent in the Kudu scan node was reduced from
  6.107ms to 3.498ms (avg over 3 runs).
- For the query 'select count(*) from (select * from
  tpch_kudu.lineitem limit 1000000) v', a worst case perf scenario for
  this change where the limit is ineffective, the time spent in the
  Kudu scan node was essentially unchanged, 32.815ms previously vs.
  29.532ms (avg over 3 runs).

Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
---
M be/src/exec/kudu-scanner.cc
A testdata/workloads/functional-query/queries/QueryTest/kudu_limit.test
M tests/query_test/test_kudu.py
3 files changed, 68 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/10119/3
-- 
To view, visit http://gerrit.cloudera.org:8080/10119
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
Gerrit-Change-Number: 10119
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-6821: Push down limits into Kudu

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Hello Alex Behm, 

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

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

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

Change subject: IMPALA-6821: Push down limits into Kudu
......................................................................

IMPALA-6821: Push down limits into Kudu

This patch takes advantage of a recent change in Kudu (KUDU-16) that
exposes the ability to set limits on KuduScanners. Since each
KuduScanner corresponds to a scan token, and there will be multiple
scan tokens per query, this is just a performance optimization in
cases where the limit is smaller than the number of rows per token,
and Impala still needs to apply the limit on our side for cases where
the limit is greater than the number of rows per token.

Testing:
- Updated test_limit to run against Kudu by moving the few test cases
  that Kudu cannot support to a different test.
- For the query 'select * from tpch_kudu.lineitem limit 1', a best
  case perf scenario for this change where the limit is highly
  effective, the time spent in the Kudu scan node was reduced from
  6.107ms to 3.498ms (avg over 3 runs).
- For the query 'select count(*) from (select * from
  tpch_kudu.lineitem limit 1000000) v', a worst case perf scenario for
  this change where the limit is ineffective, the time spent in the
  Kudu scan node was essentially unchanged, 32.815ms previously vs.
  29.532ms (avg over 3 runs).

Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
---
M be/src/exec/kudu-scanner.cc
A testdata/workloads/functional-query/queries/QueryTest/limit-nondeterministic.test
M testdata/workloads/functional-query/queries/QueryTest/limit.test
M tests/query_test/test_queries.py
4 files changed, 87 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/10119/2
-- 
To view, visit http://gerrit.cloudera.org:8080/10119
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
Gerrit-Change-Number: 10119
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>

[Impala-ASF-CR] IMPALA-6821: Push down limits into Kudu

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Hello Alex Behm, 

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

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

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

Change subject: IMPALA-6821: Push down limits into Kudu
......................................................................

IMPALA-6821: Push down limits into Kudu

This patch takes advantage of a recent change in Kudu (KUDU-16) that
exposes the ability to set limits on KuduScanners. Since each
KuduScanner corresponds to a scan token, and there will be multiple
scan tokens per query, this is just a performance optimization in
cases where the limit is smaller than the number of rows per token,
and Impala still needs to apply the limit on our side for cases where
the limit is greater than the number of rows per token.

Testing:
- Added e2e tests for various situations where limits are applied at
  a Kudu scan node.
- For the query 'select * from tpch_kudu.lineitem limit 1', a best
  case perf scenario for this change where the limit is highly
  effective, the time spent in the Kudu scan node was reduced from
  6.107ms to 3.498ms (avg over 3 runs).
- For the query 'select count(*) from (select * from
  tpch_kudu.lineitem limit 1000000) v', a worst case perf scenario for
  this change where the limit is ineffective, the time spent in the
  Kudu scan node was essentially unchanged, 32.815ms previously vs.
  29.532ms (avg over 3 runs).

Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
---
M be/src/exec/kudu-scanner.cc
A testdata/workloads/functional-query/queries/QueryTest/kudu_limit.test
M tests/query_test/test_kudu.py
3 files changed, 119 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/10119/4
-- 
To view, visit http://gerrit.cloudera.org:8080/10119
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibe35e70065d8706b575e24fe20902cd405b49941
Gerrit-Change-Number: 10119
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>