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/23 19:37:40 UTC
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Hello Adar Dembo, Hao Hao,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/11018
to review the following change.
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
hms-tool: refactor check tool and combine upgrade and fix
This commit combines the 'hms upgrade' and 'hms fix' tools. The tools
previously had overlapping responsibilities for checking some types of
inconsistencies. The 'fix' tool now has flags which can control whether
it attempts certain types of fixes:
--drop_orphan_hms_tables=false
--create_missing_hms_tables=true
--fix_inconsistent_tables=true
--upgrade_hms_tables=true
`drop_orphan_hms_tables` defaults to false since it deletes Hive tables,
and the tool can not rule-out that they are being used. Additionally, a
--dryrun flag is provided in order to print steps that would be taken
without actually making modifications.
The checking logic has been expanded to account for more potential
inconsistencies, and where possible the fix tool now can automatically
repair these inconsistencies.
Finally, the input prompt and default database functionality has been
removed in order to simplify the tool. Instead, the check tool will
issue hints including instructions for how to rename tables without a
database or with a Hive-incompatible name using the
'kudu table rename_table' tool. We can always add this functionality
back in the future if we determine it helps out users.
Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
---
M src/kudu/client/client.h
M src/kudu/gutil/strings/escaping.h
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
7 files changed, 896 insertions(+), 824 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/11018/1
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
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-tool: refactor check tool and combine upgrade and fix
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11018 )
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
Patch Set 8:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/11018/5/src/kudu/hms/hms_catalog.cc
File src/kudu/hms/hms_catalog.cc:
http://gerrit.cloudera.org:8080/#/c/11018/5/src/kudu/hms/hms_catalog.cc@166
PS5, Line 166: RETURN_NOT_OK(ParseTableName(name, &hms_database, &hms_table));
> Is there a way to reduce the duplicated code between DropLegacyTable and Dr
Done
http://gerrit.cloudera.org:8080/#/c/11018/7/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/11018/7/src/kudu/tools/kudu-tool-test.cc@2210
PS7, Line 2210:
> I think it still worth to enable HMS integration after this and see if chec
Done
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc@169
PS6, Line 169: vector<hive::Table>*
> Why not use const ref?
The indexing operations on lines 182-185 require a mutable reference, and our style guide doesn't allow passing by mut ref.
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc@238
PS6, Line 238: d by the fix tool.
> I don't see how duplicate HMS tables are handled in the fix tool. If we exp
Good point. I actually intended to add more suggestion hints in the check output, I'm going to do that. The reason I removed the duplicate table removing is that it's not unambiguously the right thing to do -- Todd pointed out that the admin may want to create a view with the duplicated name pointing to the Kudu table.
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc@410
PS6, Line 410: " and consi
> Another option to fix the orphan HMS tables (other than dropping the table)
It's not possible to recreate a Kudu table from the information in the HMS, for instance primary key and partitioning are not stored in the HMS.
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc@451
PS6, Line 451: og;
> Why not ask user to input the desired name here?
I've reconfigured this somewhat. The tool now classifies tables with Hive-incompatible into their own bucket, and the check tool outputs a suggestion for renaming each of the. Here's the output for the manual fix test:
Found Kudu table(s) with Hive-incompatible names:
Kudu table | Kudu table ID | Kudu master addresses
--------------------------------+----------------------------------+-----------------------
default.hive-incompatible-name | 85108202213a413da70f49a9eb654981 | 127.0.0.1:51579
no_database | 83d7a052e3b44a7f946446211fd39b08 | 127.0.0.1:51579
Suggestion: rename the Kudu table(s) to be Hive-compatible:
$ kudu table rename_table --alter_external_catalogs=false 127.0.0.1:51579 default.hive-incompatible-name <database-name>.<table-name>
$ kudu table rename_table --alter_external_catalogs=false 127.0.0.1:51579 no_database <database-name>.<table-name>
I think this is more or less on par with having the tool take input, and I think it's a simpler implementation. If you feel strongly about input I can add it back, though.
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc@474
PS6, Line 474: table_id
> Should we use normalized table name here?
Done
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc@532
PS6, Line 532: it.
> I think we cannot simply return early if upgrade HMS tables failed. Since t
Good catch. I've inverted the control flow so that it upgrades the table in the HMS first and then renamed the kudu table next. If the rename in Kudu fails, then the next run of 'hms check' will recognize it as a table whose name doesn't match the HMS entry, and it will automatically be renamed in Kudu. I've also added a test case to cover this.
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc@548
PS6, Line 548:
: LOG(INFO) << "[dryrun] Upgrading legacy Impala HMS metadata for table "
: << hms_table_name
> I don't see any metadata change in the HMS from L551 to L568.
Good catch, this was a stale comment.
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 8
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-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 25 Jul 2018 23:29:24 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has removed a vote on this change.
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 11
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-Reviewer: Tidy Bot
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11018 )
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
Patch Set 7:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/11018/5/src/kudu/hms/hms_catalog.cc
File src/kudu/hms/hms_catalog.cc:
http://gerrit.cloudera.org:8080/#/c/11018/5/src/kudu/hms/hms_catalog.cc@166
PS5, Line 166: Status HmsCatalog::DropLegacyTable(const string& name) {
Is there a way to reduce the duplicated code between DropLegacyTable and DropTable?
http://gerrit.cloudera.org:8080/#/c/11018/7/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/11018/7/src/kudu/tools/kudu-tool-test.cc@2210
PS7, Line 2210: }
I think it still worth to enable HMS integration after this and see if check and fix tool works as expected.
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc@169
PS6, Line 169: vector<hive::Table>*
Why not use const ref?
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc@238
PS6, Line 238: duplicate_hms_tables
I don't see how duplicate HMS tables are handled in the fix tool. If we expect this to be manually fixed, we should at least log "manual fix is required" in the fix tool?
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc@410
PS6, Line 410: drop_orphan_hms_tables
Another option to fix the orphan HMS tables (other than dropping the table) is to add the corresponding Kudu tables. Do you think it is worth to add such option in cases that the tables in Kudu were somehow accidentally dropped?
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc@451
PS6, Line 451: rename the Kudu table to a Hive-compatible
Why not ask user to input the desired name here?
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc@474
PS6, Line 474: table_name
Should we use normalized table name here?
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc@532
PS6, Line 532: UpgradeLegacyImpalaTable
I think we cannot simply return early if upgrade HMS tables failed. Since the legacy table name in Kudu has been already renamed. For any kinds of errors we may need to roll back the change in previous step.
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc@548
PS6, Line 548: This will
: // also overwrite any other stale metadata in the HMS since it's not a
: // Kudu-catalog-only alter.
I don't see any metadata change in the HMS from L551 to L568.
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 7
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-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 25 Jul 2018 01:15:46 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11018 )
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
Patch Set 3:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/11018/2/src/kudu/gutil/strings/escaping.h
File src/kudu/gutil/strings/escaping.h:
http://gerrit.cloudera.org:8080/#/c/11018/2/src/kudu/gutil/strings/escaping.h@145
PS2, Line 145: return CUnescape(source, dest, nullptr);
> warning: use nullptr [modernize-use-nullptr]
Done
http://gerrit.cloudera.org:8080/#/c/11018/2/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/11018/2/src/kudu/tools/tool_action_hms.cc@78
PS2, Line 78: using std::pair;
> warning: using decl 'map' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/11018/2/src/kudu/tools/tool_action_hms.cc@118
PS2, Line 118: shared_ptr<KuduClient> kudu_client;
> warning: missing username/bug in TODO [google-readability-todo]
Done
http://gerrit.cloudera.org:8080/#/c/11018/2/src/kudu/tools/tool_action_hms.cc@153
PS2, Line 153: "Kudu table",
> warning: parameter 'out' is unused [misc-unused-parameters]
Done
http://gerrit.cloudera.org:8080/#/c/11018/2/src/kudu/tools/tool_action_hms.cc@170
PS2, Line 170: "HMS database",
> warning: parameter 'out' is unused [misc-unused-parameters]
Done
http://gerrit.cloudera.org:8080/#/c/11018/2/src/kudu/tools/tool_action_hms.cc@194
PS2, Line 194: DataTable table({
> warning: non-const reference parameter 'table_pairs', make it const or use
Done
http://gerrit.cloudera.org:8080/#/c/11018/2/src/kudu/tools/tool_action_hms.cc@484
PS2, Line 484: // This most likely means the database doesn't exist, but it could be ambiguous.
> warning: do not use 'else' after 'continue' [readability-else-after-return]
Done
http://gerrit.cloudera.org:8080/#/c/11018/2/src/kudu/tools/tool_action_hms.cc@586
PS2, Line 586: return Status::RuntimeError("Failed to fix some catalog metadata inconsistencies");
> warning: do not use 'else' after 'return' [readability-else-after-return]
Done
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 3
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-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 24 Jul 2018 00:24:51 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11018
to look at the new patch set (#10).
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
hms-tool: refactor check tool and combine upgrade and fix
This commit combines the 'hms upgrade' and 'hms fix' tools. The tools
previously had overlapping responsibilities for checking some types of
inconsistencies. The 'fix' tool now has flags which can control whether
it attempts certain types of fixes:
--drop_orphan_hms_tables=false
--create_missing_hms_tables=true
--fix_inconsistent_tables=true
--upgrade_hms_tables=true
`drop_orphan_hms_tables` defaults to false since it deletes Hive tables,
and the tool can not rule-out that they are being used. Additionally, a
--dryrun flag is provided in order to print steps that would be taken
without actually making modifications.
The checking logic has been expanded to account for more potential
inconsistencies, and where possible the fix tool now can automatically
repair these inconsistencies.
Finally, the input prompt and default database functionality has been
removed in order to simplify the tool. Instead, the check tool will
issue hints including instructions for how to rename tables without a
database or with a Hive-incompatible name using the
'kudu table rename_table' tool. We can always add this functionality
back in the future if we determine it helps out users.
Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
---
M src/kudu/gutil/strings/escaping.h
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
6 files changed, 886 insertions(+), 818 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/11018/10
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 10
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-Reviewer: Tidy Bot
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11018 )
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
Patch Set 11: Verified+1
unrelated flake
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 11
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-Reviewer: Tidy Bot
Gerrit-Comment-Date: Mon, 30 Jul 2018 18:23:05 +0000
Gerrit-HasComments: No
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11018
to look at the new patch set (#7).
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
hms-tool: refactor check tool and combine upgrade and fix
This commit combines the 'hms upgrade' and 'hms fix' tools. The tools
previously had overlapping responsibilities for checking some types of
inconsistencies. The 'fix' tool now has flags which can control whether
it attempts certain types of fixes:
--drop_orphan_hms_tables=false
--create_missing_hms_tables=true
--fix_inconsistent_tables=true
--upgrade_hms_tables=true
`drop_orphan_hms_tables` defaults to false since it deletes Hive tables,
and the tool can not rule-out that they are being used. Additionally, a
--dryrun flag is provided in order to print steps that would be taken
without actually making modifications.
The checking logic has been expanded to account for more potential
inconsistencies, and where possible the fix tool now can automatically
repair these inconsistencies.
Finally, the input prompt and default database functionality has been
removed in order to simplify the tool. Instead, the check tool will
issue hints including instructions for how to rename tables without a
database or with a Hive-incompatible name using the
'kudu table rename_table' tool. We can always add this functionality
back in the future if we determine it helps out users.
Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
---
M src/kudu/gutil/strings/escaping.h
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
6 files changed, 890 insertions(+), 855 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/11018/7
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 7
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-Reviewer: Tidy Bot
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11018
to look at the new patch set (#8).
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
hms-tool: refactor check tool and combine upgrade and fix
This commit combines the 'hms upgrade' and 'hms fix' tools. The tools
previously had overlapping responsibilities for checking some types of
inconsistencies. The 'fix' tool now has flags which can control whether
it attempts certain types of fixes:
--drop_orphan_hms_tables=false
--create_missing_hms_tables=true
--fix_inconsistent_tables=true
--upgrade_hms_tables=true
`drop_orphan_hms_tables` defaults to false since it deletes Hive tables,
and the tool can not rule-out that they are being used. Additionally, a
--dryrun flag is provided in order to print steps that would be taken
without actually making modifications.
The checking logic has been expanded to account for more potential
inconsistencies, and where possible the fix tool now can automatically
repair these inconsistencies.
Finally, the input prompt and default database functionality has been
removed in order to simplify the tool. Instead, the check tool will
issue hints including instructions for how to rename tables without a
database or with a Hive-incompatible name using the
'kudu table rename_table' tool. We can always add this functionality
back in the future if we determine it helps out users.
Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
---
M src/kudu/gutil/strings/escaping.h
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
6 files changed, 905 insertions(+), 827 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/11018/8
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 8
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-Reviewer: Tidy Bot
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11018 )
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc@451
PS6, Line 451: nt;
> Sure, but they can copy the rename statements that get printed by the check
I do not feel quite strong on this. As you pointed out, if there is a complaint in future, we can add it back.
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 9
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-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 27 Jul 2018 21:17:24 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11018 )
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
Patch Set 11:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11018/10/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/11018/10/src/kudu/tools/tool_action_hms.cc@169
PS10, Line 169: PrintTables
> It looks like PrintKuduTables can also be embedded into this function?
Not entirely sure I follow, but PrintKuduTables can't call into PrintTables without a change in behavior, because PrintTables shows a different set of columns.
http://gerrit.cloudera.org:8080/#/c/11018/10/src/kudu/tools/tool_action_hms.cc@366
PS10, Line 366: Suggestion: rename the Kudu table(s) to be Hive-compatible,
> Should we call out this needs to be run before running the fix tool. I am g
Done
http://gerrit.cloudera.org:8080/#/c/11018/10/src/kudu/tools/tool_action_hms.cc@415
PS10, Line 415: endl
: << "Sug
> nit: this can be put upfront L408 to avoid dup code.
It's somewhat complicated to do that because of the colon in the first case:
cout << endl
<< "Suggestion: use the fix tool to repair the Kudu and Hive Metastore catalog metadata";
if (report.orphan_hms_tables.empty()) {
cout << ":" << endl << "\t$ kudu hms fix " << master_addrs << endl;
} else {
cout << endl
<< "and drop Hive Metastore table(s) which reference non-existent Kudu tables:" << endl
<< "\t$ kudu hms fix --drop_orphan_hms_tables " << master_addrs << endl;
}
I think it reads better as is, but I can switch it if you feel strongly.
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 11
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-Reviewer: Tidy Bot
Gerrit-Comment-Date: Mon, 30 Jul 2018 17:34:21 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11018
to look at the new patch set (#2).
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
hms-tool: refactor check tool and combine upgrade and fix
This commit combines the 'hms upgrade' and 'hms fix' tools. The tools
previously had overlapping responsibilities for checking some types of
inconsistencies. The 'fix' tool now has flags which can control whether
it attempts certain types of fixes:
--drop_orphan_hms_tables=false
--create_missing_hms_tables=true
--fix_inconsistent_tables=true
--upgrade_hms_tables=true
`drop_orphan_hms_tables` defaults to false since it deletes Hive tables,
and the tool can not rule-out that they are being used. Additionally, a
--dryrun flag is provided in order to print steps that would be taken
without actually making modifications.
The checking logic has been expanded to account for more potential
inconsistencies, and where possible the fix tool now can automatically
repair these inconsistencies.
Finally, the input prompt and default database functionality has been
removed in order to simplify the tool. Instead, the check tool will
issue hints including instructions for how to rename tables without a
database or with a Hive-incompatible name using the
'kudu table rename_table' tool. We can always add this functionality
back in the future if we determine it helps out users.
Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
---
M src/kudu/client/client.h
M src/kudu/gutil/strings/escaping.h
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
7 files changed, 891 insertions(+), 852 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/11018/2
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 2
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-Reviewer: Tidy Bot
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11018
to look at the new patch set (#4).
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
hms-tool: refactor check tool and combine upgrade and fix
This commit combines the 'hms upgrade' and 'hms fix' tools. The tools
previously had overlapping responsibilities for checking some types of
inconsistencies. The 'fix' tool now has flags which can control whether
it attempts certain types of fixes:
--drop_orphan_hms_tables=false
--create_missing_hms_tables=true
--fix_inconsistent_tables=true
--upgrade_hms_tables=true
`drop_orphan_hms_tables` defaults to false since it deletes Hive tables,
and the tool can not rule-out that they are being used. Additionally, a
--dryrun flag is provided in order to print steps that would be taken
without actually making modifications.
The checking logic has been expanded to account for more potential
inconsistencies, and where possible the fix tool now can automatically
repair these inconsistencies.
Finally, the input prompt and default database functionality has been
removed in order to simplify the tool. Instead, the check tool will
issue hints including instructions for how to rename tables without a
database or with a Hive-incompatible name using the
'kudu table rename_table' tool. We can always add this functionality
back in the future if we determine it helps out users.
Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
---
M src/kudu/gutil/strings/escaping.h
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
6 files changed, 893 insertions(+), 855 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/11018/4
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
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-Reviewer: Tidy Bot
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11018 )
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc@451
PS6, Line 451: nt;
> One concern I have with this reconfiguration is in the 'upgrade' process. T
Sure, but they can copy the rename statements that get printed by the check tool, fill in the new table names, and run them all at once.
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 9
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-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 27 Jul 2018 20:57:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11018
to look at the new patch set (#5).
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
hms-tool: refactor check tool and combine upgrade and fix
This commit combines the 'hms upgrade' and 'hms fix' tools. The tools
previously had overlapping responsibilities for checking some types of
inconsistencies. The 'fix' tool now has flags which can control whether
it attempts certain types of fixes:
--drop_orphan_hms_tables=false
--create_missing_hms_tables=true
--fix_inconsistent_tables=true
--upgrade_hms_tables=true
`drop_orphan_hms_tables` defaults to false since it deletes Hive tables,
and the tool can not rule-out that they are being used. Additionally, a
--dryrun flag is provided in order to print steps that would be taken
without actually making modifications.
The checking logic has been expanded to account for more potential
inconsistencies, and where possible the fix tool now can automatically
repair these inconsistencies.
Finally, the input prompt and default database functionality has been
removed in order to simplify the tool. Instead, the check tool will
issue hints including instructions for how to rename tables without a
database or with a Hive-incompatible name using the
'kudu table rename_table' tool. We can always add this functionality
back in the future if we determine it helps out users.
Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
---
M src/kudu/gutil/strings/escaping.h
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
6 files changed, 890 insertions(+), 855 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/11018/5
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 5
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-Reviewer: Tidy Bot
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11018
to look at the new patch set (#3).
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
hms-tool: refactor check tool and combine upgrade and fix
This commit combines the 'hms upgrade' and 'hms fix' tools. The tools
previously had overlapping responsibilities for checking some types of
inconsistencies. The 'fix' tool now has flags which can control whether
it attempts certain types of fixes:
--drop_orphan_hms_tables=false
--create_missing_hms_tables=true
--fix_inconsistent_tables=true
--upgrade_hms_tables=true
`drop_orphan_hms_tables` defaults to false since it deletes Hive tables,
and the tool can not rule-out that they are being used. Additionally, a
--dryrun flag is provided in order to print steps that would be taken
without actually making modifications.
The checking logic has been expanded to account for more potential
inconsistencies, and where possible the fix tool now can automatically
repair these inconsistencies.
Finally, the input prompt and default database functionality has been
removed in order to simplify the tool. Instead, the check tool will
issue hints including instructions for how to rename tables without a
database or with a Hive-incompatible name using the
'kudu table rename_table' tool. We can always add this functionality
back in the future if we determine it helps out users.
Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
---
M src/kudu/gutil/strings/escaping.h
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
6 files changed, 893 insertions(+), 856 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/11018/3
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
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-Reviewer: Tidy Bot
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11018 )
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
Patch Set 9:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc@410
PS6, Line 410: " and consi
> It's not possible to recreate a Kudu table from the information in the HMS,
Ah, right. Sorry missed that.
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc@451
PS6, Line 451: nt;
> I've reconfigured this somewhat. The tool now classifies tables with Hive
One concern I have with this reconfiguration is in the 'upgrade' process. The cluster may have many Kudu tables with hive-incompatible names. Then the users need to figure out all of them and rename manually.
http://gerrit.cloudera.org:8080/#/c/11018/6/src/kudu/tools/tool_action_hms.cc@532
PS6, Line 532: able (Kudu and HMS table
> Good catch. I've inverted the control flow so that it upgrades the table i
Cool. One thing to note here is as we discussed offline, it might be more clear to users if we group the fix that requires manual effort together in the analysis report of the 'check' tool.
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 9
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-Reviewer: Tidy Bot
Gerrit-Comment-Date: Thu, 26 Jul 2018 22:35:49 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11018 )
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
Patch Set 10:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11018/10/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/11018/10/src/kudu/tools/tool_action_hms.cc@169
PS10, Line 169: PrintTables
It looks like PrintKuduTables can also be embedded into this function?
http://gerrit.cloudera.org:8080/#/c/11018/10/src/kudu/tools/tool_action_hms.cc@366
PS10, Line 366: Suggestion: rename the Kudu table(s) to be Hive-compatible:
Should we call out this needs to be run before running the fix tool. I am good with not doing it as long as we point out in the user guide doc.
http://gerrit.cloudera.org:8080/#/c/11018/10/src/kudu/tools/tool_action_hms.cc@415
PS10, Line 415: "Suggestion: use the fix tool to repair the Kudu and Hive Metastore catalog metadata"
: << endl
nit: this can be put upfront L408 to avoid dup code.
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 10
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-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 27 Jul 2018 23:08:14 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11018 )
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
Patch Set 11: Code-Review+2
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11018/10/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/11018/10/src/kudu/tools/tool_action_hms.cc@169
PS10, Line 169: PrintTables
> Not entirely sure I follow, but PrintKuduTables can't call into PrintTables
I was thinking when print Kudu tables, the hms table can be left as blank. But after a second thought, having a separate method for printing Kudu table alone probably is more clear. So please ignore my previous comment.
http://gerrit.cloudera.org:8080/#/c/11018/10/src/kudu/tools/tool_action_hms.cc@415
PS10, Line 415: endl
: << "Sug
> It's somewhat complicated to do that because of the colon in the first case
I see, that's OK then.
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 11
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-Reviewer: Tidy Bot
Gerrit-Comment-Date: Mon, 30 Jul 2018 18:43:33 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11018
to look at the new patch set (#9).
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
hms-tool: refactor check tool and combine upgrade and fix
This commit combines the 'hms upgrade' and 'hms fix' tools. The tools
previously had overlapping responsibilities for checking some types of
inconsistencies. The 'fix' tool now has flags which can control whether
it attempts certain types of fixes:
--drop_orphan_hms_tables=false
--create_missing_hms_tables=true
--fix_inconsistent_tables=true
--upgrade_hms_tables=true
`drop_orphan_hms_tables` defaults to false since it deletes Hive tables,
and the tool can not rule-out that they are being used. Additionally, a
--dryrun flag is provided in order to print steps that would be taken
without actually making modifications.
The checking logic has been expanded to account for more potential
inconsistencies, and where possible the fix tool now can automatically
repair these inconsistencies.
Finally, the input prompt and default database functionality has been
removed in order to simplify the tool. Instead, the check tool will
issue hints including instructions for how to rename tables without a
database or with a Hive-incompatible name using the
'kudu table rename_table' tool. We can always add this functionality
back in the future if we determine it helps out users.
Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
---
M src/kudu/gutil/strings/escaping.h
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
6 files changed, 905 insertions(+), 826 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/11018/9
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 9
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-Reviewer: Tidy Bot
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11018 )
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
hms-tool: refactor check tool and combine upgrade and fix
This commit combines the 'hms upgrade' and 'hms fix' tools. The tools
previously had overlapping responsibilities for checking some types of
inconsistencies. The 'fix' tool now has flags which can control whether
it attempts certain types of fixes:
--drop_orphan_hms_tables=false
--create_missing_hms_tables=true
--fix_inconsistent_tables=true
--upgrade_hms_tables=true
`drop_orphan_hms_tables` defaults to false since it deletes Hive tables,
and the tool can not rule-out that they are being used. Additionally, a
--dryrun flag is provided in order to print steps that would be taken
without actually making modifications.
The checking logic has been expanded to account for more potential
inconsistencies, and where possible the fix tool now can automatically
repair these inconsistencies.
Finally, the input prompt and default database functionality has been
removed in order to simplify the tool. Instead, the check tool will
issue hints including instructions for how to rename tables without a
database or with a Hive-incompatible name using the
'kudu table rename_table' tool. We can always add this functionality
back in the future if we determine it helps out users.
Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Reviewed-on: http://gerrit.cloudera.org:8080/11018
Tested-by: Dan Burkert <da...@apache.org>
Reviewed-by: Hao Hao <ha...@cloudera.com>
---
M src/kudu/gutil/strings/escaping.h
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
6 files changed, 887 insertions(+), 818 deletions(-)
Approvals:
Dan Burkert: Verified
Hao Hao: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 12
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-Reviewer: Tidy Bot
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11018
to look at the new patch set (#11).
Change subject: hms-tool: refactor check tool and combine upgrade and fix
......................................................................
hms-tool: refactor check tool and combine upgrade and fix
This commit combines the 'hms upgrade' and 'hms fix' tools. The tools
previously had overlapping responsibilities for checking some types of
inconsistencies. The 'fix' tool now has flags which can control whether
it attempts certain types of fixes:
--drop_orphan_hms_tables=false
--create_missing_hms_tables=true
--fix_inconsistent_tables=true
--upgrade_hms_tables=true
`drop_orphan_hms_tables` defaults to false since it deletes Hive tables,
and the tool can not rule-out that they are being used. Additionally, a
--dryrun flag is provided in order to print steps that would be taken
without actually making modifications.
The checking logic has been expanded to account for more potential
inconsistencies, and where possible the fix tool now can automatically
repair these inconsistencies.
Finally, the input prompt and default database functionality has been
removed in order to simplify the tool. Instead, the check tool will
issue hints including instructions for how to rename tables without a
database or with a Hive-incompatible name using the
'kudu table rename_table' tool. We can always add this functionality
back in the future if we determine it helps out users.
Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
---
M src/kudu/gutil/strings/escaping.h
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
6 files changed, 887 insertions(+), 818 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/11018/11
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 11
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-Reviewer: Tidy Bot