Hao Hao 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 2:

(3 comments)

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@171
PS1, Line 171: ), hms_table->dbName, hms_table->tableNa
> uppercase Impala and Hive
Done


http://gerrit.cloudera.org:8080/#/c/10844/1/src/kudu/tools/tool_action_hms.cc@171
PS1, Line 171:                   kudu_table->id(), hms_table->dbName, 
hms_table->tableName,
> This comment is confusing, because the legacy impala table is upgraded on l
Done


http://gerrit.cloudera.org:8080/#/c/10844/1/src/kudu/tools/tool_action_hms.cc@163
PS1, Line 163:         // then downgraded. In this case, we only upgrade the 
legacy Impala table.
             :         if (s.IsAlreadyPresent() && table_name != 
new_table_name) {
             :           LOG(WARNING) << Substitute("Failed to upgrade legacy 
Impala table '$0.$1' "
             :                                      "(Kudu table name: $2), 
because a Kudu table with "
             :                                      "name '$0.$1' already 
exists'.", hms_table->dbName,
             :                                      hms_table->tableName, 
table_name);
             :         } else if (s.ok() || table_name == new_table_name) {
             :           s = hms_catalog->UpgradeLegacyImpalaTable(
             :                   kudu_table->id(), hms_table->dbName, 
hms_table->tableName,
             :                   
client::SchemaFromKuduSchema(kudu_table->schema()));
             :         }
             :       }
             :     } else {
             :       // Create the table in the HMS.
             :       string new_table_name = Substitute("$0.$1", 
default_database, table_name);
             :       Sch
> Can this be rewritten? Looks like the calls to UpgradeLegacyImpalaTable are
Realized the logic is not quite right in this patch as there might be another 
different tables with same name already exist. So updated it, And you are 
right, this should also be stored.



--
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: 2
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: Hao Hao <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 29 Jun 2018 23:47:21 +0000
Gerrit-HasComments: Yes

Reply via email to