Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19584 )
Change subject: [KUDU-3452] A tool to show on creating tables ...................................................................... Patch Set 8: (8 comments) http://gerrit.cloudera.org:8080/#/c/19584/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19584/7//COMMIT_MSG@20 PS7, Line 20: creating tables and the t > nit: creating tablets and the table's state. Done http://gerrit.cloudera.org:8080/#/c/19584/7/src/kudu/master/catalog_manager.h File src/kudu/master/catalog_manager.h: http://gerrit.cloudera.org:8080/#/c/19584/7/src/kudu/master/catalog_manager.h@147 PS7, Line 147: return !is_running() && !is_deleted(); > Using PREPARING, CREATING directly may be better I think this form is more simple. http://gerrit.cloudera.org:8080/#/c/19584/7/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/19584/7/src/kudu/master/catalog_manager.cc@3951 PS7, Line 3951: req->has_list_on_creating_tabl > nit: if (req->has_list_on_creating_tables() && req->list_on_creating_tables Done http://gerrit.cloudera.org:8080/#/c/19584/7/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: http://gerrit.cloudera.org:8080/#/c/19584/7/src/kudu/tools/kudu-admin-test.cc@a1838 PS7, Line 1838: > Because --list_tablets is not contained in new flag. Sorry, I made a mistake. http://gerrit.cloudera.org:8080/#/c/19584/7/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/19584/7/src/kudu/tools/kudu-tool-test.cc@9383 PS7, Line 9383: kDeleteTableNa > nit: kDeleteTableName ? Done http://gerrit.cloudera.org:8080/#/c/19584/7/src/kudu/tools/kudu-tool-test.cc@9384 PS7, Line 9384: kAddNewPartitionTableNa > nit: kAddNewPartitionTableName Done http://gerrit.cloudera.org:8080/#/c/19584/7/src/kudu/tools/kudu-tool-test.cc@9385 PS7, Line 9385: kNotEnoughTServersTableNa > nit: kNotEnoughTServersTableName Done http://gerrit.cloudera.org:8080/#/c/19584/7/src/kudu/tools/kudu-tool-test.cc@9520 PS7, Line 9520: > I think adding a case --list_tables_with_creating_tablets=false (default) a I think no need to test the case --list_tables_with_creating_tablets=false, this case is equal to the case "kudu table list $master_addr" which has already been recovered. The second advice has been done. Thanks. -- To view, visit http://gerrit.cloudera.org:8080/19584 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I348b69f48e6ce36ed869097f9f798c5946136de5 Gerrit-Change-Number: 19584 Gerrit-PatchSet: 8 Gerrit-Owner: Wang Xixu <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <[email protected]> Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Reviewer: Yuqi Du <[email protected]> Gerrit-Comment-Date: Fri, 19 May 2023 02:56:31 +0000 Gerrit-HasComments: Yes
