Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19594 )
Change subject: [KUDU-3452] Make validate table creating task not affected ...................................................................... Patch Set 14: (7 comments) http://gerrit.cloudera.org:8080/#/c/19594/13/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/19594/13/src/kudu/master/catalog_manager.cc@5971 PS13, Line 5971: , > nit: should be ", " Done http://gerrit.cloudera.org:8080/#/c/19594/13/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/19594/13/src/kudu/tools/kudu-tool-test.cc@9411 PS13, Line 9411: Create a table without enough h > nit: Also check the Status type is Timeout? Done http://gerrit.cloudera.org:8080/#/c/19594/13/src/kudu/tools/kudu-tool-test.cc@9402 PS13, Line 9402: client->NewTableCreator()); : return table_creator->table_name(table_name) : .schema(&schema) : .set_range_partition_columns({ "key" }) : .num_replicas(replica_num) : .timeout(MonoDelta::FromMilliseconds(kTimeout)) : .Create(); : }; : : // Create a table without enough healthy tablet servers. : / > There are some duplicate code, how about encapsulate a function? Done http://gerrit.cloudera.org:8080/#/c/19594/13/src/kudu/tools/kudu-tool-test.cc@9442 PS13, Line 9442: > Better to check the 2 tables are exactly kTwoReplicasTableId and kOneReplic Done http://gerrit.cloudera.org:8080/#/c/19594/13/src/kudu/tools/kudu-tool-test.cc@9445 PS13, Line 9445: Restart the tablet server. : ASSERT_OK(cluster_->tablet_server(0)->Restart()); : ClusterVerifier cv(cluster_.get()); : NO_FATALS(cv.CheckCluster()); : } : } > nit: how about? Done http://gerrit.cloudera.org:8080/#/c/19594/13/src/kudu/tools/kudu-tool-test.cc@9457 PS13, Line 9457: : : : : : > I'm not sure why check the tserver and the cluster state? It seems not mean Done http://gerrit.cloudera.org:8080/#/c/19594/13/src/kudu/tools/kudu-tool-test.cc@9463 PS13, Line 9463: > Also to check the status of table kNotEnoughTServersTableId is health now? Here if KSCK test passed, kNotEnoughTServersTableId should be healthy. I think no need to check table kNotEnoughTServersTableId again. -- To view, visit http://gerrit.cloudera.org:8080/19594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I64668651d0e8f58b92cfb841bdb20617de6776f9 Gerrit-Change-Number: 19594 Gerrit-PatchSet: 14 Gerrit-Owner: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Alexey Serbin <ale...@apache.org> Gerrit-Reviewer: KeDeng <kdeng...@gmail.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang <chinazhangyi...@163.com> Gerrit-Reviewer: Yingchun Lai <laiyingc...@apache.org> Gerrit-Reviewer: Yuqi Du <shenxingwuy...@gmail.com> Gerrit-Comment-Date: Mon, 08 May 2023 02:20:08 +0000 Gerrit-HasComments: Yes
