Yingchun Lai 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 6: (8 comments) http://gerrit.cloudera.org:8080/#/c/19594/4/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/19594/4/src/kudu/master/catalog_manager.cc@5940 PS4, Line 5940: { > In the origin logic, if the state of a tablet is CREATING(persisted by lock Thanks for your explain. So even if the tablet's state is not change, there is still a write operation for it? http://gerrit.cloudera.org:8080/#/c/19594/6/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/19594/6/src/kudu/master/catalog_manager.cc@5926 PS6, Line 5926: vector<scoped_refptr<TabletInfo>>::iterator nit: simplify by auto& it = ... http://gerrit.cloudera.org:8080/#/c/19594/6/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: http://gerrit.cloudera.org:8080/#/c/19594/6/src/kudu/tools/kudu-admin-test.cc@1962 PS6, Line 1962: add_range_partition Is it necessary to create a table with range partitions? I guess it's not related to the test. http://gerrit.cloudera.org:8080/#/c/19594/6/src/kudu/tools/kudu-admin-test.cc@1978 PS6, Line 1978: kNotEnoughTServersTableId It would be great to check 2 of the 3 replicas have been created successfully. http://gerrit.cloudera.org:8080/#/c/19594/6/src/kudu/tools/kudu-admin-test.cc@1979 PS6, Line 1979: ASSERT_EQ(std::strcmp("UNDER_REPLICATED", items[i]["health"].GetString()), 0); nit: ASSERT_EQ(string("UNDER_REPLICATED"), items[i]["health"].GetString()); http://gerrit.cloudera.org:8080/#/c/19594/6/src/kudu/tools/kudu-admin-test.cc@1982 PS6, Line 1982: ASSERT_EQ(std::strcmp("HEALTHY", items[i]["health"].GetString()), 0); Same. http://gerrit.cloudera.org:8080/#/c/19594/6/src/kudu/tools/kudu-admin-test.cc@1990 PS6, Line 1990: SleepFor(MonoDelta::FromMilliseconds(2 * kTimeout)); Call WaitStarted() to ensure it has been started. http://gerrit.cloudera.org:8080/#/c/19594/6/src/kudu/tools/kudu-admin-test.cc@1992 PS6, Line 1992: master_address How about cache the variable at first? -- 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: 6 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, 03 Apr 2023 16:42:03 +0000 Gerrit-HasComments: Yes