You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2018/11/05 20:21:25 UTC
[Impala-ASF-CR] IMPALA-7477: Batch-oriented query set construction
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11879
Change subject: IMPALA-7477: Batch-oriented query set construction
......................................................................
IMPALA-7477: Batch-oriented query set construction
Rework the row-by-row construction of query result sets in PlanRootSink
so that it materialises an output column at a time. Make some minor
optimisations like preallocating output vectors and initialising
strings more efficiently.
My intent is both to make this faster and to make the QueryResultSet
interface better before IMPALA-4268 does a bunch of surgery on this
part of the code.
Testing:
Ran core tests.
Perf:
Downloaded tpch_parquet.orders via JDBC driver.
Before: 3.01s, After: 2.57s.
Downloaded l_orderkey from tpch_parquet.lineitem.
Before: 1.21s, After: 1.08s.
Change-Id: I764fa302842438902cd5db2551ec6e3cb77b6874
---
M be/src/exec/plan-root-sink.cc
M be/src/exec/plan-root-sink.h
M be/src/service/hs2-util.cc
M be/src/service/hs2-util.h
M be/src/service/query-result-set.cc
M be/src/service/query-result-set.h
6 files changed, 331 insertions(+), 161 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/79/11879/1
--
To view, visit http://gerrit.cloudera.org:8080/11879
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I764fa302842438902cd5db2551ec6e3cb77b6874
Gerrit-Change-Number: 11879
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7477: Batch-oriented query set construction
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11879 )
Change subject: IMPALA-7477: Batch-oriented query set construction
......................................................................
Patch Set 1:
(1 comment)
This is a clean cherry-pick of https://gerrit.cloudera.org/#/c/11297/ with the required fix that I commented on in hs2-util.cc. IMPALA-7647 addressed the test gap by running more queries for a variety of data types through HS2.
http://gerrit.cloudera.org:8080/#/c/11879/1/be/src/service/hs2-util.cc
File be/src/service/hs2-util.cc:
http://gerrit.cloudera.org:8080/#/c/11879/1/be/src/service/hs2-util.cc@149
PS1, Line 149: ++output_row_idx;
++output_row_idx here and below was the required fix.
--
To view, visit http://gerrit.cloudera.org:8080/11879
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I764fa302842438902cd5db2551ec6e3cb77b6874
Gerrit-Change-Number: 11879
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Nov 2018 20:22:55 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7477: Batch-oriented query set construction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11879 )
Change subject: IMPALA-7477: Batch-oriented query set construction
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11879
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I764fa302842438902cd5db2551ec6e3cb77b6874
Gerrit-Change-Number: 11879
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Nov 2018 17:26:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7477: Batch-oriented query set construction
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11879 )
Change subject: IMPALA-7477: Batch-oriented query set construction
......................................................................
Patch Set 1:
Yeah that's a good idea for next time :)
--
To view, visit http://gerrit.cloudera.org:8080/11879
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I764fa302842438902cd5db2551ec6e3cb77b6874
Gerrit-Change-Number: 11879
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Nov 2018 17:26:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7477: Batch-oriented query set construction
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11879 )
Change subject: IMPALA-7477: Batch-oriented query set construction
......................................................................
Patch Set 1: Code-Review+2
Thanks for the comment that points to the fix. What we found useful in such cases is to have PS1 contain the original commit that was reverted and PS2 contain the fix.
--
To view, visit http://gerrit.cloudera.org:8080/11879
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I764fa302842438902cd5db2551ec6e3cb77b6874
Gerrit-Change-Number: 11879
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Nov 2018 10:25:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7477: Batch-oriented query set construction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11879 )
Change subject: IMPALA-7477: Batch-oriented query set construction
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3431/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/11879
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I764fa302842438902cd5db2551ec6e3cb77b6874
Gerrit-Change-Number: 11879
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Nov 2018 17:26:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7477: Batch-oriented query set construction
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/11879 )
Change subject: IMPALA-7477: Batch-oriented query set construction
......................................................................
IMPALA-7477: Batch-oriented query set construction
Rework the row-by-row construction of query result sets in PlanRootSink
so that it materialises an output column at a time. Make some minor
optimisations like preallocating output vectors and initialising
strings more efficiently.
My intent is both to make this faster and to make the QueryResultSet
interface better before IMPALA-4268 does a bunch of surgery on this
part of the code.
Testing:
Ran core tests.
Perf:
Downloaded tpch_parquet.orders via JDBC driver.
Before: 3.01s, After: 2.57s.
Downloaded l_orderkey from tpch_parquet.lineitem.
Before: 1.21s, After: 1.08s.
Change-Id: I764fa302842438902cd5db2551ec6e3cb77b6874
Reviewed-on: http://gerrit.cloudera.org:8080/11879
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exec/plan-root-sink.cc
M be/src/exec/plan-root-sink.h
M be/src/service/hs2-util.cc
M be/src/service/hs2-util.h
M be/src/service/query-result-set.cc
M be/src/service/query-result-set.h
6 files changed, 331 insertions(+), 161 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/11879
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I764fa302842438902cd5db2551ec6e3cb77b6874
Gerrit-Change-Number: 11879
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-7477: Batch-oriented query set construction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11879 )
Change subject: IMPALA-7477: Batch-oriented query set construction
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1280/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11879
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I764fa302842438902cd5db2551ec6e3cb77b6874
Gerrit-Change-Number: 11879
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Nov 2018 20:53:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7477: Batch-oriented query set construction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11879 )
Change subject: IMPALA-7477: Batch-oriented query set construction
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/11879
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I764fa302842438902cd5db2551ec6e3cb77b6874
Gerrit-Change-Number: 11879
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Nov 2018 21:27:54 +0000
Gerrit-HasComments: No