Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10844 )
Change subject: KUDU-2191: Adjust upgrade tool to tolerate downgraded metadata ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/10844/1/src/kudu/tools/tool_action_hms.cc File src/kudu/tools/tool_action_hms.cc: http://gerrit.cloudera.org:8080/#/c/10844/1/src/kudu/tools/tool_action_hms.cc@163 PS1, Line 163: if (!exist) { : // TODO(Hao): Use notification listener to avoid race conditions. : s = AlterKuduTableOnly(kudu_client, table_name, new_table_name).AndThen([&] { : return hms_catalog->UpgradeLegacyImpalaTable(kudu_table->id(), : hms_table->dbName, hms_table->tableName, : client::SchemaFromKuduSchema(kudu_table->schema())); : }); : } else { : // Only upgrade legacy impala table if the Kudu table with hive-compatible : // name already exists. : RETURN_NOT_OK(hms_catalog->UpgradeLegacyImpalaTable( : kudu_table->id(), : hms_table->dbName, : hms_table->tableName, : client::SchemaFromKuduSchema(kudu_table->schema()))); : } Can this be rewritten? Looks like the calls to UpgradeLegacyImpalaTable are exactly the same in both branches. Also, why is UpgradeLegacyImpalaTable non-fatal (i.e. just stores the result into 's') in the !exist case, but fatal (via RETURN_NOT_OK) in the exist case? -- To view, visit http://gerrit.cloudera.org:8080/10844 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie9a3b154e77db4eda72b9c4b53861f9fe06f09a3 Gerrit-Change-Number: 10844 Gerrit-PatchSet: 1 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: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Fri, 29 Jun 2018 21:43:51 +0000 Gerrit-HasComments: Yes
