Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/10075 )
Change subject: KUDU-2191: Metadata Upgrade Tool ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/10075/1/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/10075/1/src/kudu/tools/kudu-tool-test.cc@1854 PS1, Line 1854: table.tableName = table_name; Isn't that metadata checking going to be problematic in practice? If it's checking that legacy tables don't have the kudu.master_addresses property and failing to roll them forward if so, then we aren't going to be able to upgrade legacy tables, right? > I do not feel it is crucial to add 'kudu.master_addresses' here for testing. I think it's important to test against entries which are as similar to what we'll be encountering in real clusters as possible. http://gerrit.cloudera.org:8080/#/c/10075/1/src/kudu/tools/tool_action_hms.cc File src/kudu/tools/tool_action_hms.cc: http://gerrit.cloudera.org:8080/#/c/10075/1/src/kudu/tools/tool_action_hms.cc@94 PS1, Line 94: // TODO(Hao): Do we need to check if Impala-managed table is not present in HMS? > Good point. Maybe we should instead ask the users to input which database h Yeah, I think in any case where we're going to rename a table it would be better to confirm first, and if we can, we should fill in a default based on either the existing table name or kudu table name table param. http://gerrit.cloudera.org:8080/#/c/10075/1/src/kudu/tools/tool_action_hms.cc@155 PS1, Line 155: ReplicaController::SetVisibility(&builder, ReplicaController::Visibility::ALL); > Right, experimental flags are checked and have to unlock them with FLAGS_un So would this work if the user passes --unlock_experimental_flags? I think that will be preferred in the short-term, rather than having two different flag names. In the long term we'll be making these flags stable. -- To view, visit http://gerrit.cloudera.org:8080/10075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1c8488e1c7b04b8bedc31c83b1496b53bae49cb3 Gerrit-Change-Number: 10075 Gerrit-PatchSet: 2 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: Tue, 17 Apr 2018 17:44:14 +0000 Gerrit-HasComments: Yes
