Yuqi Du 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 23: (9 comments) Glad to receive your advices. You are very kind. Thanks. http://gerrit.cloudera.org:8080/#/c/18877/10/src/kudu/client/scan_token-test.cc File src/kudu/client/scan_token-test.cc: http://gerrit.cloudera.org:8080/#/c/18877/10/src/kudu/client/scan_token-test.cc@1542 PS10, Line 1542: // supplied range. : int64_t row_count_b = -1; > Could you try the latest code? some new features have been added. Done http://gerrit.cloudera.org:8080/#/c/18877/19/src/kudu/client/scan_token-test.cc File src/kudu/client/scan_token-test.cc: http://gerrit.cloudera.org:8080/#/c/18877/19/src/kudu/client/scan_token-test.cc@1491 PS19, Line 1491: ASSERT_OK(range_adder(client_.get(), -100, 0)); > Thanks your method. Although, it's not a deterministic test case for Leader I explain the case. 'flaky', you maybe mean the case is unsteady. That's not the case. If the case failed, that means the CLOSEST_REPLICA (last message I replies is wrong) is not valid. But If the case succeed that doesn't ensure CLOSEST_REPLICA is valid. So it's a necessary condition. http://gerrit.cloudera.org:8080/#/c/18877/20/src/kudu/client/scan_token-test.cc File src/kudu/client/scan_token-test.cc: http://gerrit.cloudera.org:8080/#/c/18877/20/src/kudu/client/scan_token-test.cc@1520 PS20, Line 1520: } > Is it necessary to insert 100,000 rows into the table? It may last a long t I can reduce it. 1000 http://gerrit.cloudera.org:8080/#/c/18877/20/src/kudu/client/scan_token-test.cc@1573 PS20, Line 1573: ASSER > nit: can be omitted, the same to other places. Done http://gerrit.cloudera.org:8080/#/c/18877/20/src/kudu/client/scan_token-test.cc@1658 PS20, Line 1658: vector<KuduScanToken*> tokens; > It would be better to reduce nested levels, for example by: Done http://gerrit.cloudera.org:8080/#/c/18877/20/src/kudu/client/scan_token-test.cc@1743 PS20, Line 1743: : : : : > Better to move the comments closer to the actual code, it would be more fri Done http://gerrit.cloudera.org:8080/#/c/18877/20/src/kudu/client/scan_token-test.cc@1809 PS20, Line 1809: > The test is using the latest code, how can this situation happen? CHECK and remove. http://gerrit.cloudera.org:8080/#/c/18877/20/src/kudu/client/scan_token-test.cc@1817 PS20, Line 1817: > check interned_replicas is not empty at first Done http://gerrit.cloudera.org:8080/#/c/18877/21/src/kudu/client/scan_token-test.cc File src/kudu/client/scan_token-test.cc: http://gerrit.cloudera.org:8080/#/c/18877/21/src/kudu/client/scan_token-test.cc@1498 PS21, Line 1498: ASSERT_OK(range_adder(client_.get(), -100, -50)); > OK, I'll do it later. Done -- 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: 23 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: Tue, 08 Nov 2022 08:22:42 +0000 Gerrit-HasComments: Yes
