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 <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Thu, 06 Sep 2018 01:17:20 +0000 Gerrit-HasComments: Yes
