Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/10303 )
Change subject: KUDU-2191: HMS inconsistent metadata fix tool ...................................................................... Patch Set 4: (7 comments) http://gerrit.cloudera.org:8080/#/c/10303/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10303/4//COMMIT_MSG@11 PS4, Line 11: Ant And http://gerrit.cloudera.org:8080/#/c/10303/2/src/kudu/tools/tool_action_hms.cc File src/kudu/tools/tool_action_hms.cc: http://gerrit.cloudera.org:8080/#/c/10303/2/src/kudu/tools/tool_action_hms.cc@237 PS2, Line 237: nit: present http://gerrit.cloudera.org:8080/#/c/10303/4/src/kudu/tools/tool_action_hms.cc File src/kudu/tools/tool_action_hms.cc: http://gerrit.cloudera.org:8080/#/c/10303/4/src/kudu/tools/tool_action_hms.cc@142 PS4, Line 142: Warning 'Warn' http://gerrit.cloudera.org:8080/#/c/10303/4/src/kudu/tools/tool_action_hms.cc@448 PS4, Line 448: // There must be one Kudu table in the entry. This is hard to follow, could the constraint instead be encoded in the type itself? E.g. change tables_map to be an unordered_map<string, pair<shared_ptr<KuduTable>, vector<hive::Table>>> http://gerrit.cloudera.org:8080/#/c/10303/4/src/kudu/tools/tool_action_hms.cc@474 PS4, Line 474: // 3. Correct the table name. Picking an HMS entry to rename arbitrarily makes me pretty nervous, since HMS entries come associated with other metadata like sentry privileges. I'm not sure we should have an automated fix for this situation, since it's not clear what the right course is. It should also be an extraordinary situation, since we try very hard in the master not to allow it to happen. http://gerrit.cloudera.org:8080/#/c/10303/4/src/kudu/tools/tool_action_hms.cc@485 PS4, Line 485: RETURN_NOT_OK(AlterKuduTable(kudu_client, kudu_table_name, new_table_name)); Shouldn't this be a 'kudu only' alter? http://gerrit.cloudera.org:8080/#/c/10303/4/src/kudu/tools/tool_action_hms.cc@502 PS4, Line 502: RETURN_NOT_OK(alterer->RenameTo(kudu_table_name) Why does this do a rename to the same table name? -- To view, visit http://gerrit.cloudera.org:8080/10303 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I63c694b5d9877cfbd218139f2e9ecf05fd69c997 Gerrit-Change-Number: 10303 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Wed, 20 Jun 2018 18:06:09 +0000 Gerrit-HasComments: Yes
