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

Reply via email to