You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Hao Hao (Code Review)" <ge...@cloudera.org> on 2018/06/02 00:05:55 UTC
[kudu-CR] KUDU-2191: use 'kudu only' in metadata upgrade tool
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10582
Change subject: KUDU-2191: use 'kudu_only' in metadata upgrade tool
......................................................................
KUDU-2191: use 'kudu_only' in metadata upgrade tool
Previously the metadata upgrade tool requires the HMS integration
feature being disabled. This commit updates the tool to use 'kudu_only'
flag when altering Kudu tables, so that the restriction no longer
applies.
Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
2 files changed, 7 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/10582/1
--
To view, visit http://gerrit.cloudera.org:8080/10582
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
Gerrit-Change-Number: 10582
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
[kudu-CR] KUDU-2191: use 'kudu only' in metadata upgrade tool
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10582 )
Change subject: KUDU-2191: use 'kudu_only' in metadata upgrade tool
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10582/3/src/kudu/client/client-internal.h
File src/kudu/client/client-internal.h:
http://gerrit.cloudera.org:8080/#/c/10582/3/src/kudu/client/client-internal.h@295
PS3, Line 295: // Helper function. When HMS integration is enabled, alter the table in Kudu but not
I'm confused; why is this function needed at all? Can't tool_action_hms call KuduTableAlterer::Alter(boolean) directly, provided you set up friendship appropriately?
--
To view, visit http://gerrit.cloudera.org:8080/10582
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
Gerrit-Change-Number: 10582
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 07 Jun 2018 19:41:13 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2191: allow metadata upgrade tool to run with HMS integration
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/10582 )
Change subject: KUDU-2191: allow metadata upgrade tool to run with HMS integration
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10582
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
Gerrit-Change-Number: 10582
Gerrit-PatchSet: 7
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 15 Jun 2018 19:21:17 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2191: use 'kudu only' in metadata upgrade tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10582
to look at the new patch set (#3).
Change subject: KUDU-2191: use 'kudu_only' in metadata upgrade tool
......................................................................
KUDU-2191: use 'kudu_only' in metadata upgrade tool
Previously the metadata upgrade tool requires the HMS integration
feature being disabled. This commit updates the tool to use 'kudu_only'
flag when altering Kudu tables, so that the restriction no longer
applies.
Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
5 files changed, 19 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/10582/3
--
To view, visit http://gerrit.cloudera.org:8080/10582
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
Gerrit-Change-Number: 10582
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2191: use 'kudu only' in metadata upgrade tool
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10582 )
Change subject: KUDU-2191: use 'kudu_only' in metadata upgrade tool
......................................................................
Patch Set 2:
> > The LSAN failure looks legit. Also, why does tool_action_hms
> > include a test header (client-test-util.h)? Why is this new
> > function defined in a test header?
>
> tool_action_hms includes client-test-util.h because it uses a util method SchemaFromKuduSchema defined in client-test-util, but maybe it would make more sense to move them all to sth like client-util.h?
Sure. Or maybe in schema-internal.h, which is currently used for the internal definition of a client schema (i.e. KuduSchema::Data). It's not a perfect fit but you won't need to create a new header.
--
To view, visit http://gerrit.cloudera.org:8080/10582
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
Gerrit-Change-Number: 10582
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 07 Jun 2018 18:46:16 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2191: allow metadata upgrade tool to run with HMS integration
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10582
to look at the new patch set (#5).
Change subject: KUDU-2191: allow metadata upgrade tool to run with HMS integration
......................................................................
KUDU-2191: allow metadata upgrade tool to run with HMS integration
Previously the metadata upgrade tool requires the HMS integration
feature being disabled. This commit updates the tool to only alter
tables in Kudu but not in the HMS during upgrade, so that the
restriction no longer applies.
Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
---
M src/kudu/client/client.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
3 files changed, 32 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/10582/5
--
To view, visit http://gerrit.cloudera.org:8080/10582
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
Gerrit-Change-Number: 10582
Gerrit-PatchSet: 5
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2191: use 'kudu only' in metadata upgrade tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/10582 )
Change subject: KUDU-2191: use 'kudu_only' in metadata upgrade tool
......................................................................
Patch Set 2:
> The LSAN failure looks legit. Also, why does tool_action_hms
> include a test header (client-test-util.h)? Why is this new
> function defined in a test header?
> The LSAN failure looks legit. Also, why does tool_action_hms
> include a test header (client-test-util.h)? Why is this new
> function defined in a test header?
tool_action_hms includes client-test-util.h because it uses a util method SchemaFromKuduSchema defined in client-test-util, but maybe it would make more sense to move them all to sth like client-util.h?
--
To view, visit http://gerrit.cloudera.org:8080/10582
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
Gerrit-Change-Number: 10582
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 07 Jun 2018 18:35:09 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2191: use 'kudu only' in metadata upgrade tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/10582 )
Change subject: KUDU-2191: use 'kudu_only' in metadata upgrade tool
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10582/3/src/kudu/client/client-internal.h
File src/kudu/client/client-internal.h:
http://gerrit.cloudera.org:8080/#/c/10582/3/src/kudu/client/client-internal.h@295
PS3, Line 295: // Helper function. When HMS integration is enabled, alter the table in Kudu but not
> I'm confused; why is this function needed at all? Can't tool_action_hms cal
You're right, updated the patch. Thanks!
--
To view, visit http://gerrit.cloudera.org:8080/10582
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
Gerrit-Change-Number: 10582
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 11 Jun 2018 19:42:19 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2191: use 'kudu only' in metadata upgrade tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10582
to look at the new patch set (#2).
Change subject: KUDU-2191: use 'kudu_only' in metadata upgrade tool
......................................................................
KUDU-2191: use 'kudu_only' in metadata upgrade tool
Previously the metadata upgrade tool requires the HMS integration
feature being disabled. This commit updates the tool to use 'kudu_only'
flag when altering Kudu tables, so that the restriction no longer
applies.
Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
---
M src/kudu/client/client-test-util.cc
M src/kudu/client/client-test-util.h
M src/kudu/client/client.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
5 files changed, 18 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/10582/2
--
To view, visit http://gerrit.cloudera.org:8080/10582
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
Gerrit-Change-Number: 10582
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2191: use 'kudu only' in metadata upgrade tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10582
to look at the new patch set (#4).
Change subject: KUDU-2191: use 'kudu_only' in metadata upgrade tool
......................................................................
KUDU-2191: use 'kudu_only' in metadata upgrade tool
Previously the metadata upgrade tool requires the HMS integration
feature being disabled. This commit updates the tool to use 'kudu_only'
flag when altering Kudu tables, so that the restriction no longer
applies.
Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
---
M src/kudu/client/client.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
3 files changed, 32 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/10582/4
--
To view, visit http://gerrit.cloudera.org:8080/10582
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
Gerrit-Change-Number: 10582
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2191: allow metadata upgrade tool to run with HMS integration
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10582 )
Change subject: KUDU-2191: allow metadata upgrade tool to run with HMS integration
......................................................................
KUDU-2191: allow metadata upgrade tool to run with HMS integration
Previously the metadata upgrade tool requires the HMS integration
feature being disabled. This commit updates the tool to only alter
tables in Kudu but not in the HMS during upgrade, so that the
restriction no longer applies.
Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
Reviewed-on: http://gerrit.cloudera.org:8080/10582
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <da...@apache.org>
---
M src/kudu/client/client.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
3 files changed, 32 insertions(+), 10 deletions(-)
Approvals:
Kudu Jenkins: Verified
Dan Burkert: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/10582
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
Gerrit-Change-Number: 10582
Gerrit-PatchSet: 8
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2191: use 'kudu only' in metadata upgrade tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/10582 )
Change subject: KUDU-2191: use 'kudu_only' in metadata upgrade tool
......................................................................
Patch Set 3:
> > > The LSAN failure looks legit. Also, why does tool_action_hms
> > > include a test header (client-test-util.h)? Why is this new
> > > function defined in a test header?
> >
> > tool_action_hms includes client-test-util.h because it uses a
> util method SchemaFromKuduSchema defined in client-test-util, but
> maybe it would make more sense to move them all to sth like
> client-util.h?
>
> Sure. Or maybe in schema-internal.h, which is currently used for
> the internal definition of a client schema (i.e. KuduSchema::Data).
> It's not a perfect fit but you won't need to create a new header.
Thanks for the suggestion! I updated this patch to define the new function
in client-internal.h. I will do another patch for the other method.
--
To view, visit http://gerrit.cloudera.org:8080/10582
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
Gerrit-Change-Number: 10582
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 07 Jun 2018 19:30:56 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2191: allow metadata upgrade tool to run with HMS integration
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10582 )
Change subject: KUDU-2191: allow metadata upgrade tool to run with HMS integration
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10582
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
Gerrit-Change-Number: 10582
Gerrit-PatchSet: 5
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 11 Jun 2018 23:21:51 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2191: use 'kudu only' in metadata upgrade tool
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10582 )
Change subject: KUDU-2191: use 'kudu_only' in metadata upgrade tool
......................................................................
Patch Set 2:
The LSAN failure looks legit. Also, why does tool_action_hms include a test header (client-test-util.h)? Why is this new function defined in a test header?
--
To view, visit http://gerrit.cloudera.org:8080/10582
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
Gerrit-Change-Number: 10582
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 07 Jun 2018 17:15:25 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2191: use 'kudu only' in metadata upgrade tool
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10582 )
Change subject: KUDU-2191: use 'kudu_only' in metadata upgrade tool
......................................................................
Patch Set 4: Code-Review+2
But I'd like Dan to take a look too.
--
To view, visit http://gerrit.cloudera.org:8080/10582
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
Gerrit-Change-Number: 10582
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 11 Jun 2018 19:52:40 +0000
Gerrit-HasComments: No