Yuqi Du 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 7: (9 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 state nit: creating tablets and the table's state. 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 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->list_on_creating_tables() nit: if (req->has_list_on_creating_tables() && req->list_on_creating_tables()) { 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. I don't understand the reason that remove these cases. 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: kDeleteTableId nit: kDeleteTableName ? http://gerrit.cloudera.org:8080/#/c/19584/7/src/kudu/tools/kudu-tool-test.cc@9384 PS7, Line 9384: kAddNewPartitionTableId nit: kAddNewPartitionTableName http://gerrit.cloudera.org:8080/#/c/19584/7/src/kudu/tools/kudu-tool-test.cc@9385 PS7, Line 9385: kNotEnoughTServersTableId nit: kNotEnoughTServersTableName 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) at this. And then recover the shutdowned tserver and run this command 'table list ... --list_tables_with_creating_tablets=true' again, check its results. http://gerrit.cloudera.org:8080/#/c/19584/7/src/kudu/tools/tool.proto File src/kudu/tools/tool.proto: http://gerrit.cloudera.org:8080/#/c/19584/7/src/kudu/tools/tool.proto@548 PS7, Line 548: optional bytes id = 6; TableInfoPB has name, its human readable, is table_id necessary? -- 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: 7 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: Thu, 18 May 2023 07:09:04 +0000 Gerrit-HasComments: Yes
