You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2019/03/07 19:35:23 UTC
[Impala-ASF-CR] IMPALA-8284. KuduTableSink spends too much CPU in KuduSchema::Column()
Hello Thomas Marshall,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/12692
to review the following change.
Change subject: IMPALA-8284. KuduTableSink spends too much CPU in KuduSchema::Column()
......................................................................
IMPALA-8284. KuduTableSink spends too much CPU in KuduSchema::Column()
The KuduSchema::Column() accessor actually returns a copy of the
KuduColumnSchema object, which is not lightweight. We were inadvertently
calling this function once for every null cell seen during an insertion.
This caused a performance bottleneck for datasets with large numbers of
NULL cells.
This improves the situation by caching the nullability of the Kudu
columns in our own vector. The vector lookups should be inlined and much
faster than copying a KuduColumnSchema.
No new tests included as this is a perf fix.
Change-Id: I1b4d14d20252bdb190f50ebaaf6179a46eafb932
---
M be/src/exec/kudu-table-sink.cc
M be/src/exec/kudu-table-sink.h
2 files changed, 16 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/92/12692/1
--
To view, visit http://gerrit.cloudera.org:8080/12692
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1b4d14d20252bdb190f50ebaaf6179a46eafb932
Gerrit-Change-Number: 12692
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-8284. KuduTableSink spends too much CPU in KuduSchema::Column()
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12692 )
Change subject: IMPALA-8284. KuduTableSink spends too much CPU in KuduSchema::Column()
......................................................................
Patch Set 1: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/12692
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b4d14d20252bdb190f50ebaaf6179a46eafb932
Gerrit-Change-Number: 12692
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 08 Mar 2019 10:03:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8284. KuduTableSink spends too much CPU in KuduSchema::Column()
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/12692 )
Change subject: IMPALA-8284. KuduTableSink spends too much CPU in KuduSchema::Column()
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/12692
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b4d14d20252bdb190f50ebaaf6179a46eafb932
Gerrit-Change-Number: 12692
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 07 Mar 2019 19:51:01 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8284. KuduTableSink spends too much CPU in KuduSchema::Column()
Posted by "Thomas Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/12692 )
Change subject: IMPALA-8284. KuduTableSink spends too much CPU in KuduSchema::Column()
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12692
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b4d14d20252bdb190f50ebaaf6179a46eafb932
Gerrit-Change-Number: 12692
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 07 Mar 2019 21:51:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8284. KuduTableSink spends too much CPU in KuduSchema::Column()
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12692 )
Change subject: IMPALA-8284. KuduTableSink spends too much CPU in KuduSchema::Column()
......................................................................
Patch Set 1:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3893/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/12692
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b4d14d20252bdb190f50ebaaf6179a46eafb932
Gerrit-Change-Number: 12692
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 08 Mar 2019 05:55:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8284. KuduTableSink spends too much CPU in KuduSchema::Column()
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12692 )
Change subject: IMPALA-8284. KuduTableSink spends too much CPU in KuduSchema::Column()
......................................................................
IMPALA-8284. KuduTableSink spends too much CPU in KuduSchema::Column()
The KuduSchema::Column() accessor actually returns a copy of the
KuduColumnSchema object, which is not lightweight. We were inadvertently
calling this function once for every null cell seen during an insertion.
This caused a performance bottleneck for datasets with large numbers of
NULL cells.
This improves the situation by caching the nullability of the Kudu
columns in our own vector. The vector lookups should be inlined and much
faster than copying a KuduColumnSchema.
No new tests included as this is a perf fix.
Change-Id: I1b4d14d20252bdb190f50ebaaf6179a46eafb932
Reviewed-on: http://gerrit.cloudera.org:8080/12692
Reviewed-by: Will Berkeley <wd...@gmail.com>
Reviewed-by: Thomas Marshall <tm...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exec/kudu-table-sink.cc
M be/src/exec/kudu-table-sink.h
2 files changed, 16 insertions(+), 4 deletions(-)
Approvals:
Will Berkeley: Looks good to me, but someone else must approve
Thomas Marshall: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/12692
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1b4d14d20252bdb190f50ebaaf6179a46eafb932
Gerrit-Change-Number: 12692
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[Impala-ASF-CR] IMPALA-8284. KuduTableSink spends too much CPU in KuduSchema::Column()
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12692 )
Change subject: IMPALA-8284. KuduTableSink spends too much CPU in KuduSchema::Column()
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2389/ : 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/12692
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b4d14d20252bdb190f50ebaaf6179a46eafb932
Gerrit-Change-Number: 12692
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 07 Mar 2019 20:15:55 +0000
Gerrit-HasComments: No