You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2020/04/23 05:41:55 UTC
[kudu-CR](branch-1.12.x) client: fix extra construction of ColumnSchema
Todd Lipcon has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15788
Change subject: client: fix extra construction of ColumnSchema
......................................................................
client: fix extra construction of ColumnSchema
The VARCHAR patches introduced a line 'auto col = schema.column(idx)'
which ends up making a copy of the ColumnSchema object instead of taking
a reference to it. This is unnecessary and quite slow. This shows up as
a significant CPU consumer when running tpch_real_world insert workload.
Change-Id: Iafae805979495e7e15c5294a317d6e00255654e0
Reviewed-on: http://gerrit.cloudera.org:8080/15787
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Tested-by: Kudu Jenkins
(cherry picked from commit 9c5d8c97915376de4fb6027a3f73d00e160fd1d7)
---
M src/kudu/common/partial_row.cc
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/15788/1
--
To view, visit http://gerrit.cloudera.org:8080/15788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: branch-1.12.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iafae805979495e7e15c5294a317d6e00255654e0
Gerrit-Change-Number: 15788
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
[kudu-CR](branch-1.12.x) client: fix extra construction of ColumnSchema
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15788 )
Change subject: client: fix extra construction of ColumnSchema
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/15788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: branch-1.12.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Iafae805979495e7e15c5294a317d6e00255654e0
Gerrit-Change-Number: 15788
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 23 Apr 2020 16:12:31 +0000
Gerrit-HasComments: No
[kudu-CR](branch-1.12.x) client: fix extra construction of ColumnSchema
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/15788 )
Change subject: client: fix extra construction of ColumnSchema
......................................................................
Patch Set 1:
Hey Hao. If you haven't already cut a new RC for 1.12 I think this one is worth including, since it prevents a perf regression
--
To view, visit http://gerrit.cloudera.org:8080/15788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: branch-1.12.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Iafae805979495e7e15c5294a317d6e00255654e0
Gerrit-Change-Number: 15788
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 23 Apr 2020 05:42:27 +0000
Gerrit-HasComments: No
[kudu-CR](branch-1.12.x) client: fix extra construction of ColumnSchema
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15788 )
Change subject: client: fix extra construction of ColumnSchema
......................................................................
client: fix extra construction of ColumnSchema
The VARCHAR patches introduced a line 'auto col = schema.column(idx)'
which ends up making a copy of the ColumnSchema object instead of taking
a reference to it. This is unnecessary and quite slow. This shows up as
a significant CPU consumer when running tpch_real_world insert workload.
Change-Id: Iafae805979495e7e15c5294a317d6e00255654e0
Reviewed-on: http://gerrit.cloudera.org:8080/15787
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Tested-by: Kudu Jenkins
(cherry picked from commit 9c5d8c97915376de4fb6027a3f73d00e160fd1d7)
Reviewed-on: http://gerrit.cloudera.org:8080/15788
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Hao Hao <ha...@cloudera.com>
---
M src/kudu/common/partial_row.cc
1 file changed, 2 insertions(+), 2 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, but someone else must approve; Verified
Hao Hao: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/15788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: branch-1.12.x
Gerrit-MessageType: merged
Gerrit-Change-Id: Iafae805979495e7e15c5294a317d6e00255654e0
Gerrit-Change-Number: 15788
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR](branch-1.12.x) client: fix extra construction of ColumnSchema
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/15788 )
Change subject: client: fix extra construction of ColumnSchema
......................................................................
Patch Set 1:
I dunno, I think known regressions with trivial fixes should be included. If this were a perf optimization (not a regression) I would completely agree, but I think this could reduce ingest performance by 5-10% for some workloads
--
To view, visit http://gerrit.cloudera.org:8080/15788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: branch-1.12.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Iafae805979495e7e15c5294a317d6e00255654e0
Gerrit-Change-Number: 15788
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 23 Apr 2020 18:02:53 +0000
Gerrit-HasComments: No
[kudu-CR](branch-1.12.x) client: fix extra construction of ColumnSchema
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15788 )
Change subject: client: fix extra construction of ColumnSchema
......................................................................
Patch Set 1:
> Patch Set 1:
>
> Hey Hao. If you haven't already cut a new RC for 1.12 I think this one is worth including, since it prevents a perf regression
I agree with what Grant proposed to only backport essentially break/fix issues only and test issues to 1.12.0. If this is only a perf regression we can backport to 1.12.1 (if we have one)?
--
To view, visit http://gerrit.cloudera.org:8080/15788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: branch-1.12.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Iafae805979495e7e15c5294a317d6e00255654e0
Gerrit-Change-Number: 15788
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 23 Apr 2020 17:49:49 +0000
Gerrit-HasComments: No
[kudu-CR](branch-1.12.x) client: fix extra construction of ColumnSchema
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: client: fix extra construction of ColumnSchema
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/15788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: branch-1.12.x
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Iafae805979495e7e15c5294a317d6e00255654e0
Gerrit-Change-Number: 15788
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR](branch-1.12.x) client: fix extra construction of ColumnSchema
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15788 )
Change subject: client: fix extra construction of ColumnSchema
......................................................................
Patch Set 1: Verified+1
Unrelated test failure in HmsClientTest.TestHmsFaultHandling
--
To view, visit http://gerrit.cloudera.org:8080/15788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: branch-1.12.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Iafae805979495e7e15c5294a317d6e00255654e0
Gerrit-Change-Number: 15788
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 23 Apr 2020 16:13:03 +0000
Gerrit-HasComments: No
[kudu-CR](branch-1.12.x) client: fix extra construction of ColumnSchema
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15788 )
Change subject: client: fix extra construction of ColumnSchema
......................................................................
Patch Set 1: Code-Review+2
> Patch Set 1:
>
> I dunno, I think known regressions with trivial fixes should be included. If this were a perf optimization (not a regression) I would completely agree, but I think this could reduce ingest performance by 5-10% for some workloads
I see, sounds good to me.
--
To view, visit http://gerrit.cloudera.org:8080/15788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: branch-1.12.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Iafae805979495e7e15c5294a317d6e00255654e0
Gerrit-Change-Number: 15788
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 23 Apr 2020 20:52:30 +0000
Gerrit-HasComments: No