Yifan Zhang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18877 )

Change subject: [client] Fix a kudu c++ client bug when using replica_selection 
policy
......................................................................


Patch Set 29:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/18877/29/src/kudu/client/scan_token-test.cc
File src/kudu/client/scan_token-test.cc:

http://gerrit.cloudera.org:8080/#/c/18877/29/src/kudu/client/scan_token-test.cc@1599
PS29, Line 1599: uuid_scan_count
Since the test cluster is set up in every test case, there should be no scanner 
in tablet servers at the beginning, maybe we don't need to count scanners 
before the actual scanning starts?


http://gerrit.cloudera.org:8080/#/c/18877/29/src/kudu/client/scan_token-test.cc@1683
PS29, Line 1683:   ASSERT_GT(distinct_machine, 0);
I still don't think it's a good way to test the 'CLOSEST_REPLICA' policy.

Because the selection for 'CLOSEST_REPLICA' is deterministic in a single 
process, see this test 
https://github.com/apache/kudu/blob/f88c33ccab88f2e6f9bf8c2155f7115e79645882/src/kudu/client/client-test.cc#L2653
 for details. Maybe we can use the 'GetTabletServer ' method to collect remote 
tablet servers using 'CLOSEST_REPLICA' policy and check that scanners do scan 
these tablet servers?


http://gerrit.cloudera.org:8080/#/c/18877/29/src/kudu/client/scan_token-test.cc@1718
PS29, Line 1718:   ASSERT_EQ(leader_row_count, first_replica_row_count);
Should it be ASSERT_GE? The first replica could also be a follower.



--
To view, visit http://gerrit.cloudera.org:8080/18877
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I413f99b6a0b6082c5453358b8333913e4c6264c2
Gerrit-Change-Number: 18877
Gerrit-PatchSet: 29
Gerrit-Owner: Yuqi Du <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: KeDeng <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <[email protected]>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <[email protected]>
Gerrit-Reviewer: Yingchun Lai <[email protected]>
Gerrit-Reviewer: Yuqi Du <[email protected]>
Gerrit-Comment-Date: Mon, 14 Nov 2022 09:02:34 +0000
Gerrit-HasComments: Yes

Reply via email to