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/08/13 04:43:29 UTC
[kudu-CR] Add delete external catalogs flag to table delete tool
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11197
Change subject: Add delete_external_catalogs flag to table delete tool
......................................................................
Add delete_external_catalogs flag to table delete tool
Similar to alter_external_catalogs flag, it might be necessary to
introduce a flag to control the table deletion in the Kudu catalog
independently of the HMS. So we can repair tables if needed when the
HMS integration is enabled.
Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/tools/tool_action_table.cc
8 files changed, 50 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/11197/1
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
[kudu-CR] Add modify external catalogs flag to table delete tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11197
to look at the new patch set (#7).
Change subject: Add modify_external_catalogs flag to table delete tool
......................................................................
Add modify_external_catalogs flag to table delete tool
The 'hms fix' tool helps recover from metadata inconsistencies between
Kudu and the HMS. However, there might be some unforeseen edge cases
that the tool doesn't cover. To that end, this patch introduces a flag
to control the deletion of tables in the Kudu catalog independently of
HMS' catalog.
Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/table_alterer-internal.cc
M src/kudu/client/table_alterer-internal.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_hms.cc
M src/kudu/tools/tool_action_table.cc
14 files changed, 80 insertions(+), 50 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/11197/7
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 7
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Add delete external catalogs flag to table delete tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11197
to look at the new patch set (#3).
Change subject: Add delete_external_catalogs flag to table delete tool
......................................................................
Add delete_external_catalogs flag to table delete tool
Despite of 'hms fix' tool which intends to recover from metadata
inconsistency between Kudu and the HMS, it is a good idea to introduce
a flag to control the table deletion in the Kudu catalog independently
of the HMS, to repair tables when HMS integration is enabled. Since
there might be some edge cases we do not foresee and hence not covered
in 'hms fix' tool.
Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/table_alterer-internal.cc
M src/kudu/client/table_alterer-internal.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_hms.cc
M src/kudu/tools/tool_action_table.cc
14 files changed, 76 insertions(+), 39 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/11197/3
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Add delete external catalogs flag to table delete tool
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11197 )
Change subject: Add delete_external_catalogs flag to table delete tool
......................................................................
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/client/client.h
File src/kudu/client/client.h:
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/client/client.h@547
PS1, Line 547: Status KUDU_NO_EXPORT DeleteTableWithOption(const std::string& table_name,
Could this just be called DeleteTable? 'WithOption' implies there would be an options struct or something similar.
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/client/client.cc
File src/kudu/client/client.cc:
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/client/client.cc@400
PS1, Line 400: return data_->DeleteTable(this, table_name, deadline);
Could simplify this by forwarding to the new API.
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/master/master.proto
File src/kudu/master/master.proto:
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/master/master.proto@449
PS1, Line 449: optional bool delete_external_catalogs = 2 [default = true];
What do you think about unifying the names of 'delete_external_catalogs' and 'alter_external_catalogs', perhaps to 'modify_external_catalogs'? I think it may be easier to understand that way, since they affect the DDL ops in the same way. It would also mean only one CLI flag is needed.
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/tools/tool_action_table.cc@129
PS1, Line 129: return client->DeleteTableWithOption(table_name, FLAGS_delete_external_catalogs);
Add a CLI test.
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 14 Aug 2018 20:53:50 +0000
Gerrit-HasComments: Yes
[kudu-CR] Add delete external catalogs flag to table delete tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11197
to look at the new patch set (#2).
Change subject: Add delete_external_catalogs flag to table delete tool
......................................................................
Add delete_external_catalogs flag to table delete tool
Even with 'hms fix' tool which intendd to recover from
metadata inconsistency between Kudu and the HMS, it is a good idea to
introduce a flag to control the table deletion in the Kudu catalog
independently of the HMS to repair tables when HMS integration is enabled.
Since there might be some edge cases wo do not foresee and hence not covered in 'hms fix' tool
Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/table_alterer-internal.cc
M src/kudu/client/table_alterer-internal.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_hms.cc
M src/kudu/tools/tool_action_table.cc
14 files changed, 76 insertions(+), 39 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/11197/2
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Add delete external catalogs flag to table delete tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11197
to look at the new patch set (#4).
Change subject: Add delete_external_catalogs flag to table delete tool
......................................................................
Add delete_external_catalogs flag to table delete tool
Despite of 'hms fix' tool which intends to recover from metadata
inconsistency between Kudu and the HMS, it is a good idea to introduce
a flag to control the table deletion in the Kudu catalog independently
of the HMS, to repair tables when HMS integration is enabled. Since
there might be some edge cases we do not foresee and hence not covered
in 'hms fix' tool.
Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/table_alterer-internal.cc
M src/kudu/client/table_alterer-internal.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_hms.cc
M src/kudu/tools/tool_action_table.cc
14 files changed, 79 insertions(+), 42 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/11197/4
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Add modify external catalogs flag to table delete tool
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11197 )
Change subject: Add modify_external_catalogs flag to table delete tool
......................................................................
Patch Set 9: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 9
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 07 Sep 2018 16:42:54 +0000
Gerrit-HasComments: No
[kudu-CR] Add modify external catalogs flag to table delete tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11197 )
Change subject: Add modify_external_catalogs flag to table delete tool
......................................................................
Add modify_external_catalogs flag to table delete tool
The 'hms fix' tool helps recover from metadata inconsistencies between
Kudu and the HMS. However, there might be some unforeseen edge cases
that the tool doesn't cover. To that end, this patch introduces a flag
to control the deletion of tables in the Kudu catalog independently of
HMS' catalog.
Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Reviewed-on: http://gerrit.cloudera.org:8080/11197
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <da...@apache.org>
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/table_alterer-internal.cc
M src/kudu/client/table_alterer-internal.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_hms.cc
M src/kudu/tools/tool_action_table.cc
14 files changed, 79 insertions(+), 51 deletions(-)
Approvals:
Kudu Jenkins: Verified
Dan Burkert: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 12
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Add delete external catalogs flag to table delete tool
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11197 )
Change subject: Add delete_external_catalogs flag to table delete tool
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 5
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 06 Sep 2018 22:19:44 +0000
Gerrit-HasComments: No
[kudu-CR] Add modify external catalogs flag to table delete tool
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11197 )
Change subject: Add modify_external_catalogs flag to table delete tool
......................................................................
Patch Set 12:
I think you missed my comments on PS6 regarding removing (now unnecessary) friendship.
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 12
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 10 Sep 2018 20:01:27 +0000
Gerrit-HasComments: No
[kudu-CR] Add modify external catalogs flag to table delete tool
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11197 )
Change subject: Add modify_external_catalogs flag to table delete tool
......................................................................
Patch Set 11: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 11
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 07 Sep 2018 19:53:12 +0000
Gerrit-HasComments: No
[kudu-CR] Add modify external catalogs flag to table delete tool
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11197 )
Change subject: Add modify_external_catalogs flag to table delete tool
......................................................................
Patch Set 6:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11197/6/src/kudu/client/client.h
File src/kudu/client/client.h:
http://gerrit.cloudera.org:8080/#/c/11197/6/src/kudu/client/client.h@54
PS6, Line 54: class MasterHmsTest_TestAlterTable_Test;
: class MasterHmsUpgradeTest_TestRenameExistingTables_Test;
And remove these?
http://gerrit.cloudera.org:8080/#/c/11197/6/src/kudu/client/client.h@65
PS6, Line 65: namespace tools {
: class LeaderMasterProxy;
: std::string GetMasterAddresses(const client::KuduClient&);
: void SetModifyExternalCatalogs(client::KuduTableAlterer*, bool);
: } // namespace tools
Do we still need this friendship?
http://gerrit.cloudera.org:8080/#/c/11197/6/src/kudu/client/client.h@1230
PS6, Line 1230:
: friend void tools::SetModifyExternalCatalogs(KuduTableAlterer*, bool);
: FRIEND_TEST(kudu::MasterHmsTest, TestAlterTable);
: FRIEND_TEST(kudu::MasterHmsUpgradeTest, TestRenameExistingTables);
Should be able to remove these now that modify_external_catalogs() is public?
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 6
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 07 Sep 2018 04:29:34 +0000
Gerrit-HasComments: Yes
[kudu-CR] Add modify external catalogs flag to table delete tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has removed a vote on this change.
Change subject: Add modify_external_catalogs flag to table delete tool
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 9
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Add modify external catalogs flag to table delete tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11197
to look at the new patch set (#11).
Change subject: Add modify_external_catalogs flag to table delete tool
......................................................................
Add modify_external_catalogs flag to table delete tool
The 'hms fix' tool helps recover from metadata inconsistencies between
Kudu and the HMS. However, there might be some unforeseen edge cases
that the tool doesn't cover. To that end, this patch introduces a flag
to control the deletion of tables in the Kudu catalog independently of
HMS' catalog.
Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/table_alterer-internal.cc
M src/kudu/client/table_alterer-internal.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_hms.cc
M src/kudu/tools/tool_action_table.cc
14 files changed, 79 insertions(+), 51 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/11197/11
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 11
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Add modify external catalogs flag to table delete tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11197 )
Change subject: Add modify_external_catalogs flag to table delete tool
......................................................................
Patch Set 11:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11197/10/src/kudu/client/client.cc
File src/kudu/client/client.cc:
http://gerrit.cloudera.org:8080/#/c/11197/10/src/kudu/client/client.cc@404
PS10, Line 404: ing&
> & with the type
Done
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 11
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 07 Sep 2018 18:43:49 +0000
Gerrit-HasComments: Yes
[kudu-CR] Add modify external catalogs flag to table delete tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11197 )
Change subject: Add modify_external_catalogs flag to table delete tool
......................................................................
Patch Set 9: Verified+1
The failure due to flaky tests.
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 9
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 07 Sep 2018 17:13:58 +0000
Gerrit-HasComments: No
[kudu-CR] Add modify external catalogs flag to table delete tool
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11197 )
Change subject: Add modify_external_catalogs flag to table delete tool
......................................................................
Patch Set 10:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11197/10/src/kudu/client/client.cc
File src/kudu/client/client.cc:
http://gerrit.cloudera.org:8080/#/c/11197/10/src/kudu/client/client.cc@404
PS10, Line 404: ing &
& with the type
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 07 Sep 2018 17:34:01 +0000
Gerrit-HasComments: Yes
[kudu-CR] Add modify external catalogs flag to table delete tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11197 )
Change subject: Add modify_external_catalogs flag to table delete tool
......................................................................
Patch Set 6:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11197/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11197/5//COMMIT_MSG@7
PS5, Line 7: modify_external_catalogs
> modify_external_catalogs
Done
http://gerrit.cloudera.org:8080/#/c/11197/4/src/kudu/client/client.h
File src/kudu/client/client.h:
http://gerrit.cloudera.org:8080/#/c/11197/4/src/kudu/client/client.h@1233
PS4, Line 1233: FRIEND_TEST(kudu::MasterHmsUpgradeTest, TestRenameExistingTables);
> I think this should be done as part of this patch, since the same lines of
Done
http://gerrit.cloudera.org:8080/#/c/11197/5/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/11197/5/src/kudu/tools/tool_action_table.cc@43
PS5, Line 43: "when renaming or dropping a table.");
> 'renaming' -> 'renaming or dropping'
Done
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 6
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 07 Sep 2018 04:14:07 +0000
Gerrit-HasComments: Yes
[kudu-CR] Add delete external catalogs flag to table delete tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11197 )
Change subject: Add delete_external_catalogs flag to table delete tool
......................................................................
Patch Set 5:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11197/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11197/4//COMMIT_MSG@9
PS4, Line 9: The 'hms fix' tool helps recover from metadata inconsistencies between
> Maybe rewrite this description as:
Done
http://gerrit.cloudera.org:8080/#/c/11197/4/src/kudu/client/client.h
File src/kudu/client/client.h:
http://gerrit.cloudera.org:8080/#/c/11197/4/src/kudu/client/client.h@354
PS4, Line 354: Status KUDU_NO_EXPORT DeleteTableInCatalogs(const std::string& table_name,
> According to the google style guide, we should only use function overloads
Done
http://gerrit.cloudera.org:8080/#/c/11197/4/src/kudu/client/client.h@1233
PS4, Line 1233: // Whether to apply the alteration to external catalogs, such as the Hive
> I mentioned this to Dan in an earlier review, but I'd like to see this func
Thanks a lot for bringing this up and I will take a note.
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 5
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 06 Sep 2018 22:18:02 +0000
Gerrit-HasComments: Yes
[kudu-CR] Add modify external catalogs flag to table delete tool
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11197 )
Change subject: Add modify_external_catalogs flag to table delete tool
......................................................................
Patch Set 9: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11197/9/src/kudu/client/client.cc
File src/kudu/client/client.cc:
http://gerrit.cloudera.org:8080/#/c/11197/9/src/kudu/client/client.cc@404
PS9, Line 404: const std::string &
nit: const string& table_name
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 9
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 07 Sep 2018 17:14:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] Add modify external catalogs flag to table delete tool
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11197 )
Change subject: Add modify_external_catalogs flag to table delete tool
......................................................................
Patch Set 9: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 9
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 07 Sep 2018 17:14:35 +0000
Gerrit-HasComments: No
[kudu-CR] Add delete external catalogs flag to table delete tool
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11197 )
Change subject: Add delete_external_catalogs flag to table delete tool
......................................................................
Patch Set 5:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11197/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11197/5//COMMIT_MSG@7
PS5, Line 7: delete_external_catalogs
modify_external_catalogs
http://gerrit.cloudera.org:8080/#/c/11197/4/src/kudu/client/client.h
File src/kudu/client/client.h:
http://gerrit.cloudera.org:8080/#/c/11197/4/src/kudu/client/client.h@1233
PS4, Line 1233: // Whether to apply the alteration to external catalogs, such as the Hive
> Thanks a lot for bringing this up and I will take a note.
I think this should be done as part of this patch, since the same lines of code are having to be churned anyway.
http://gerrit.cloudera.org:8080/#/c/11197/5/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/11197/5/src/kudu/tools/tool_action_table.cc@43
PS5, Line 43: "when renaming a table.");
'renaming' -> 'renaming or dropping'
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 5
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 06 Sep 2018 22:58:56 +0000
Gerrit-HasComments: Yes
[kudu-CR] Add modify external catalogs flag to table delete tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11197 )
Change subject: Add modify_external_catalogs flag to table delete tool
......................................................................
Patch Set 10:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11197/9/src/kudu/client/client.cc
File src/kudu/client/client.cc:
http://gerrit.cloudera.org:8080/#/c/11197/9/src/kudu/client/client.cc@404
PS9, Line 404: const string &table
> nit: const string& table_name
Done
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 07 Sep 2018 17:32:49 +0000
Gerrit-HasComments: Yes
[kudu-CR] Add modify external catalogs flag to table delete tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11197
to look at the new patch set (#6).
Change subject: Add modify_external_catalogs flag to table delete tool
......................................................................
Add modify_external_catalogs flag to table delete tool
The 'hms fix' tool helps recover from metadata inconsistencies between
Kudu and the HMS. However, there might be some unforeseen edge cases
that the tool doesn't cover. To that end, this patch introduces a flag
to control the deletion of tables in the Kudu catalog independently of
HMS' catalog.
Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/table_alterer-internal.cc
M src/kudu/client/table_alterer-internal.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_hms.cc
M src/kudu/tools/tool_action_table.cc
14 files changed, 84 insertions(+), 47 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/11197/6
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 6
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Add modify external catalogs flag to table delete tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11197
to look at the new patch set (#8).
Change subject: Add modify_external_catalogs flag to table delete tool
......................................................................
Add modify_external_catalogs flag to table delete tool
The 'hms fix' tool helps recover from metadata inconsistencies between
Kudu and the HMS. However, there might be some unforeseen edge cases
that the tool doesn't cover. To that end, this patch introduces a flag
to control the deletion of tables in the Kudu catalog independently of
HMS' catalog.
Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/table_alterer-internal.cc
M src/kudu/client/table_alterer-internal.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_hms.cc
M src/kudu/tools/tool_action_table.cc
14 files changed, 79 insertions(+), 50 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/11197/8
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 8
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Add delete external catalogs flag to table delete tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11197 )
Change subject: Add delete_external_catalogs flag to table delete tool
......................................................................
Patch Set 3:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/11197/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11197/1//COMMIT_MSG@9
PS1, Line 9: o recover from metadata
: inconsistency between Kudu and the HMS, it is a good idea to introduce
: a flag to control the tab
> Not really understanding this: this commit introduces this flag, so how cou
Sorry for the confusion. We have 'hms fix' tool which hopefully can cover all the cases when metadata between Kudu and the HMS become unsynchronized. But there might be some corner cases we do not foresee, that is why I introduce this commit. I will revise the commit message to better capture this.
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/client/client.h
File src/kudu/client/client.h:
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/client/client.h@539
PS1, Line 539: /// the necessary credentials to authenticate to the cluster, as well as to
> Can you move this so it's next to DeleteTable?
Done
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/client/client.h@547
PS1, Line 547: /// The resulting binary authentication credentials.
> Could this just be called DeleteTable? 'WithOption' implies there would be
Done
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/client/client.cc
File src/kudu/client/client.cc:
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/client/client.cc@400
PS1, Line 400: Status KuduClient::DeleteTable(const string& table_name) {
> Could simplify this by forwarding to the new API.
Done
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/master/master.proto
File src/kudu/master/master.proto:
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/master/master.proto@449
PS1, Line 449: optional bool modify_external_catalogs = 2 [default = true];
> What do you think about unifying the names of 'delete_external_catalogs' an
Done
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/tools/tool_action_table.cc@39
PS1, Line 39:
: DECLARE_string(tables);
: DEFINE_bool(modify_external_catalogs, true,
: "Whether to modify external catalogs, such as the Hive Metastore, "
: "when renaming a table.");
: DEFINE_bool(list_tablets, false,
> These only exist to facilitate "upgrade" workflows that take you from no HM
Not really, these can be used in normal workflows by admin to recover a cluster when somehow metadata in Kudu and the HMS go out of sync.
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/tools/tool_action_table.cc@129
PS1, Line 129: RETURN_NOT_OK(CreateKuduClient(context, &client));
> Add a CLI test.
Done
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 06 Sep 2018 01:17:20 +0000
Gerrit-HasComments: Yes
[kudu-CR] Add delete external catalogs flag to table delete tool
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11197 )
Change subject: Add delete_external_catalogs flag to table delete tool
......................................................................
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11197/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11197/4//COMMIT_MSG@9
PS4, Line 9: Despite of 'hms fix' tool which intends to recover from metadata
Maybe rewrite this description as:
The 'hms fix' tool helps recover from metadata inconsistencies between Kudu and the HMS. However, there might be some unforeseen edge cases that the tool doesn't cover. To that end, this patch introduces a flag to control the deletion of tables in the Kudu catalog independently of HMS' catalog.
http://gerrit.cloudera.org:8080/#/c/11197/4/src/kudu/client/client.h
File src/kudu/client/client.h:
http://gerrit.cloudera.org:8080/#/c/11197/4/src/kudu/client/client.h@354
PS4, Line 354: Status KUDU_NO_EXPORT DeleteTable(const std::string& table_name,
According to the google style guide, we should only use function overloads if the various overloads are semantically equivalent. I don't think this meets those guidelines: it's not like the argument set represent an alternative to const std::string&. Moreover, using the same name means it's easy to accidentally call the private API and get a somewhat inscrutable link error.
https://google.github.io/styleguide/cppguide.html#Function_Overloading
http://gerrit.cloudera.org:8080/#/c/11197/4/src/kudu/client/client.h@1233
PS4, Line 1233: // Whether to apply the alteration to external catalogs, such as the Hive
I mentioned this to Dan in an earlier review, but I'd like to see this function made public and KUDU_NO_EXPORT. I think that's the cleanest way for us to provide private client APIs.
Shouldn't happen in this patch, but something to keep in mind.
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 06 Sep 2018 17:30:11 +0000
Gerrit-HasComments: Yes
[kudu-CR] Add delete external catalogs flag to table delete tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11197
to look at the new patch set (#5).
Change subject: Add delete_external_catalogs flag to table delete tool
......................................................................
Add delete_external_catalogs flag to table delete tool
The 'hms fix' tool helps recover from metadata inconsistencies between
Kudu and the HMS. However, there might be some unforeseen edge cases
that the tool doesn't cover. To that end, this patch introduces a flag
to control the deletion of tables in the Kudu catalog independently of
HMS' catalog.
Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/table_alterer-internal.cc
M src/kudu/client/table_alterer-internal.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_hms.cc
M src/kudu/tools/tool_action_table.cc
14 files changed, 79 insertions(+), 42 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/11197/5
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 5
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Add delete external catalogs flag to table delete tool
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11197 )
Change subject: Add delete_external_catalogs flag to table delete tool
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11197/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11197/1//COMMIT_MSG@9
PS1, Line 9: it might be necessary to
: introduce a flag to control the table deletion in the Kudu catalog
: independently of the HMS.
Not really understanding this: this commit introduces this flag, so how could it "might be necessary" to introduce it in the future?
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/client/client.h
File src/kudu/client/client.h:
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/client/client.h@539
PS1, Line 539: /// Delete/drop a table with option.
Can you move this so it's next to DeleteTable?
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/11197/1/src/kudu/tools/tool_action_table.cc@39
PS1, Line 39: DEFINE_bool(alter_external_catalogs, true,
: "Whether to alter external catalogs, such as the Hive Metastore, "
: "when renaming a table.");
: DEFINE_bool(delete_external_catalogs, true,
: "Whether to delete external catalogs, such as the Hive Metastore, "
: "when deleting a table.");
These only exist to facilitate "upgrade" workflows that take you from no HMS integration to HMS integration, right? If so, perhaps we should warn users against changing the values of these parameters?
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 13 Aug 2018 22:33:46 +0000
Gerrit-HasComments: Yes
[kudu-CR] Add modify external catalogs flag to table delete tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11197
to look at the new patch set (#10).
Change subject: Add modify_external_catalogs flag to table delete tool
......................................................................
Add modify_external_catalogs flag to table delete tool
The 'hms fix' tool helps recover from metadata inconsistencies between
Kudu and the HMS. However, there might be some unforeseen edge cases
that the tool doesn't cover. To that end, this patch introduces a flag
to control the deletion of tables in the Kudu catalog independently of
HMS' catalog.
Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/table_alterer-internal.cc
M src/kudu/client/table_alterer-internal.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_hms.cc
M src/kudu/tools/tool_action_table.cc
14 files changed, 79 insertions(+), 51 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/11197/10
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Add modify external catalogs flag to table delete tool
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11197
to look at the new patch set (#9).
Change subject: Add modify_external_catalogs flag to table delete tool
......................................................................
Add modify_external_catalogs flag to table delete tool
The 'hms fix' tool helps recover from metadata inconsistencies between
Kudu and the HMS. However, there might be some unforeseen edge cases
that the tool doesn't cover. To that end, this patch introduces a flag
to control the deletion of tables in the Kudu catalog independently of
HMS' catalog.
Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/table_alterer-internal.cc
M src/kudu/client/table_alterer-internal.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_hms.cc
M src/kudu/tools/tool_action_table.cc
14 files changed, 79 insertions(+), 51 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/11197/9
--
To view, visit http://gerrit.cloudera.org:8080/11197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0a128fb53c974a5c839786204d56408681b434e8
Gerrit-Change-Number: 11197
Gerrit-PatchSet: 9
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins