You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2018/07/24 22:14:28 UTC
[kudu-CR] hms precheck tool
Hello Adar Dembo, Hao Hao,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/11040
to review the following change.
Change subject: hms precheck tool
......................................................................
hms precheck tool
This commit adds a 'kudu hms precheck' tool that is meant to be used
before enabling the HMS integration on an existing cluster. The tool
checks for issues that cause the master to fail to startup after
enabling the HMS flags.
Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
2 files changed, 133 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/11040/1
--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
[kudu-CR] hms precheck tool
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11040 )
Change subject: hms precheck tool
......................................................................
Patch Set 6: Verified+1
unrelated flake
--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Burkert <da...@apache.org>
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, 30 Jul 2018 19:46:33 +0000
Gerrit-HasComments: No
[kudu-CR] hms precheck tool
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11040 )
Change subject: hms precheck tool
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/tool_action_hms.cc@695
PS3, Line 695:
> I see your point, the reason I was a bit concern about this is I am not sur
Yeah I thought about that too, but I don't think listing the normalized table name will necessarily help in all cases, for instance the table names 'FOO' and 'FoO' the shared normalized name will be distinct from both.
--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert <da...@apache.org>
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, 27 Jul 2018 20:55:50 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms precheck tool
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11040
to look at the new patch set (#9).
Change subject: hms precheck tool
......................................................................
hms precheck tool
This commit adds a 'kudu hms precheck' tool that is meant to be used
before enabling the HMS integration on an existing cluster. The tool
checks for tables whose names are not unique when compared with Hive's
case-insensitive identifier rules. These conflicting tables would cause
the master to fail to startup after enabling the Hive Metastore
integration.
Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
2 files changed, 143 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/11040/9
--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 9
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] hms precheck tool
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11040
to look at the new patch set (#4).
Change subject: hms precheck tool
......................................................................
hms precheck tool
This commit adds a 'kudu hms precheck' tool that is meant to be used
before enabling the HMS integration on an existing cluster. The tool
checks for tables whose names are not unique when compared with Hive's
case-insensitive identifier rules. These conflicting tables would cause
the master to fail to startup after enabling the Hive Metastore
integration.
Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
2 files changed, 143 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/11040/4
--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] hms precheck tool
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has removed a vote on this change.
Change subject: hms precheck tool
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Burkert <da...@apache.org>
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] hms precheck tool
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11040 )
Change subject: hms precheck tool
......................................................................
Patch Set 9: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11040/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11040/7//COMMIT_MSG@11
PS7, Line 11: are not unique when compared with Hive's
: case-insensitive identifier rules
> Not sure I fully understand your question. It's definitely possible to hav
Ah, so the 'hms fix' will take care of the sync between Kudu and HMS then, I see. So, that's the tool that will take care of possible conflict if default.MegaTable would exist in HMS already. SGMT.
--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 9
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 31 Jul 2018 20:49:37 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms precheck tool
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11040 )
Change subject: hms precheck tool
......................................................................
Patch Set 9:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/11040/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11040/7//COMMIT_MSG@11
PS7, Line 11: are not unique when compared with Hive's
: case-insensitive identifier rules
> Are there any other funny cases like already existing Kuud tables named lik
Not sure I fully understand your question. It's definitely possible to have a table named 'default.MegaTable' in a cluster and then go through the workflow for enabling the HMS integration. In that case the precheck tool would not flag the table (unless a second table conflicted with it by case such as 'default.MEGATABLE'). The 'hms fix' tool will automatically create an HMS table for it when run.
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2605
PS7, Line 2605: for (const string& table_name : {
: "a.b",
: "foo.bar",
: "FOO.bar",
> I don't think this is relevant if using CreateKuduTable()
Done
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2626
PS7, Line 2626: ASSERT_STR_CONTAINS(o
> Maybe, it's worth asserting on specific type of error (RunTimeError, etc.)?
That depends on the internals of Subprocess::Call, which isn't really relevant here, and doesn't appear to be documented (not sure if that's intentional or not).
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2642
PS7, Line 2642: NO_FATALS(RunAction
> wrap into NO_FATALS()
Done
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2643
PS7, Line 2643:
> ditto
Done
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2646
PS7, Line 2646: cluster_->EnableMet
> ditto
Done
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2657
PS7, Line 2657:
: "foo.bar",
: "foo.bar2",
: "foo.bar3",
: "fuzz",
: }), tables);
: }
:
: // This test is parameterized on the serialization mode and Kerberos.
: cla
> nit: the expected value comes first -- that way it's easier to read the err
Done
--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 9
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 31 Jul 2018 00:10:14 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms precheck tool
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11040 )
Change subject: hms precheck tool
......................................................................
Patch Set 7:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/11040/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11040/7//COMMIT_MSG@11
PS7, Line 11: are not unique when compared with Hive's
: case-insensitive identifier rules
Are there any other funny cases like already existing Kuud tables named like 'default.MegaTable' when HMS would have 'MegaTable' in its default database?
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2605
PS7, Line 2605: KuduSchemaBuilder schema_builder;
: schema_builder.AddColumn("key")->Type(client::KuduColumnSchema::INT32)->NotNull()->PrimaryKey();
: KuduSchema schema;
: ASSERT_OK(schema_builder.Build(&schema));
I don't think this is relevant if using CreateKuduTable()
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2626
PS7, Line 2626: ASSERT_FALSE(s.ok());
Maybe, it's worth asserting on specific type of error (RunTimeError, etc.)?
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2642
PS7, Line 2642: RunActionStdoutNone
wrap into NO_FATALS()
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2643
PS7, Line 2643: RunActionStdoutNone
ditto
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2646
PS7, Line 2646: RunActionStdoutNone
ditto
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2657
PS7, Line 2657: vector<string>({
: "A.B!",
: "FUZZ",
: "a.b",
: "a.b!",
: "foo.bar",
: "foo.bar2",
: "foo.bar3",
: "fuzz",
: }
nit: the expected value comes first -- that way it's easier to read the error message (if any).
--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 7
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 30 Jul 2018 23:45:01 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms precheck tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11040 )
Change subject: hms precheck tool
......................................................................
Patch Set 4: Code-Review+2
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/tool_action_hms.cc@685
PS3, Line 685: s.ok()) {
: // This is not a Hive-comp
> This has to do with how the CatalogManager normalizes table names. It effe
Thanks for your explanation. Makes sense to me.
http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/tool_action_hms.cc@695
PS3, Line 695:
> I went back and forth on this, and I eventually settled on just including t
I see your point, the reason I was a bit concern about this is I am not sure if users are aware that they do not need to rename all the listed tables. e.g. for "foo.bar foo.BAR Foo.bar", they only need to rename two out of three. But maybe I'm over-concerned.
--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert <da...@apache.org>
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, 26 Jul 2018 21:24:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms precheck tool
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11040 )
Change subject: hms precheck tool
......................................................................
Patch Set 12: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 12
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 01 Aug 2018 02:17:31 +0000
Gerrit-HasComments: No
[kudu-CR] hms precheck tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11040 )
Change subject: hms precheck tool
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/tool_action_hms.cc@695
PS3, Line 695:
> Yeah I thought about that too, but I don't think listing the normalized tab
Maybe anther idea way is to log all conflicting names in one row? e.g:
FOo FOO
Bar BAR
But the format does not look like a nature fit for DataTable. So I am good with keeping it as the way it is.
--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert <da...@apache.org>
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, 27 Jul 2018 21:11:47 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms precheck tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11040 )
Change subject: hms precheck tool
......................................................................
Patch Set 12: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 12
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 01 Aug 2018 18:40:35 +0000
Gerrit-HasComments: No
[kudu-CR] hms precheck tool
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11040
to look at the new patch set (#10).
Change subject: hms precheck tool
......................................................................
hms precheck tool
This commit adds a 'kudu hms precheck' tool that is meant to be used
before enabling the HMS integration on an existing cluster. The tool
checks for tables whose names are not unique when compared with Hive's
case-insensitive identifier rules. These conflicting tables would cause
the master to fail to startup after enabling the Hive Metastore
integration.
Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
2 files changed, 145 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/11040/10
--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 10
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] hms precheck tool
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11040 )
Change subject: hms precheck tool
......................................................................
Patch Set 4:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/11040/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11040/2//COMMIT_MSG@11
PS2, Line 11: tables
> Can you add a bit explanation of what kind of issue they are?
Done
http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/kudu-tool-test.cc@2639
PS3, Line 2639: A.B
> 'A.B!' ?
Done
http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/tool_action_hms.cc@685
PS3, Line 685: s.ok()) {
: // This is not a Hive-comp
> Sorry that I am not sure if I follow why conflicting names with hive-incomp
This has to do with how the CatalogManager normalizes table names. It effectively skips tables whose names don't match the Hive identifier rules. I've added a reference in this comment to this other comment: https://github.com/apache/kudu/blob/master/src/kudu/master/catalog_manager.cc#L4789-L4801
http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/tool_action_hms.cc@695
PS3, Line 695:
> It might be more clear if print out in the following format:
I went back and forth on this, and I eventually settled on just including the conflicting table names because I think the case conflicts will be clear from the non-normalized names, and I think it may be somewhat confusing to include the normalized name if there aren't any tables that actually have that name. I've also tried to keep the word 'normalized' out of user-facing messages, since it's pretty jargony. If you feel strongly it's easy to add, though.
http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/tool_action_hms.cc@754
PS3, Line 754: return ModeBuilder("hms").Description("Operate on remote Hive Metastores")
> nit: Is this alphabetical ordered?
Nope, didn't realize we followed that convention. Fixed.
--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert <da...@apache.org>
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, 26 Jul 2018 18:28:40 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms precheck tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11040 )
Change subject: hms precheck tool
......................................................................
Patch Set 3:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/11040/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11040/2//COMMIT_MSG@11
PS2, Line 11: issues
Can you add a bit explanation of what kind of issue they are?
http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/kudu-tool-test.cc@2639
PS3, Line 2639: A.B
'A.B!' ?
http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/tool_action_hms.cc@685
PS3, Line 685: This is not a Hive-compatible table name, so there can't be a conflict
: // among normalized names.
Sorry that I am not sure if I follow why conflicting names with hive-incompatible is not a concern?
http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/tool_action_hms.cc@695
PS3, Line 695: conflicting table names
It might be more clear if print out in the following format:
normalized_table_name conflicting_table_name
http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/tool_action_hms.cc@754
PS3, Line 754: .AddAction(std::move(hms_downgrade))
nit: Is this alphabetical ordered?
--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 26 Jul 2018 00:49:03 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms precheck tool
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11040 )
Change subject: hms precheck tool
......................................................................
hms precheck tool
This commit adds a 'kudu hms precheck' tool that is meant to be used
before enabling the HMS integration on an existing cluster. The tool
checks for tables whose names are not unique when compared with Hive's
case-insensitive identifier rules. These conflicting tables would cause
the master to fail to startup after enabling the Hive Metastore
integration.
Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Reviewed-on: http://gerrit.cloudera.org:8080/11040
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-by: Hao Hao <ha...@cloudera.com>
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
2 files changed, 144 insertions(+), 15 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved
Kudu Jenkins: Verified
Hao Hao: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 13
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] hms precheck tool
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11040
to look at the new patch set (#12).
Change subject: hms precheck tool
......................................................................
hms precheck tool
This commit adds a 'kudu hms precheck' tool that is meant to be used
before enabling the HMS integration on an existing cluster. The tool
checks for tables whose names are not unique when compared with Hive's
case-insensitive identifier rules. These conflicting tables would cause
the master to fail to startup after enabling the Hive Metastore
integration.
Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
2 files changed, 144 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/11040/12
--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 12
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins