Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/12138 )
Change subject: Support location awareness in READ_CLOSEST for the C++ client ...................................................................... Patch Set 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/12138/3/src/kudu/integration-tests/location_assignment-itest.cc File src/kudu/integration-tests/location_assignment-itest.cc: http://gerrit.cloudera.org:8080/#/c/12138/3/src/kudu/integration-tests/location_assignment-itest.cc@51 PS3, Line 51: > warning: using decl 'KuduClientBuilder' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/12138/3/src/kudu/integration-tests/location_assignment-itest.cc@78 PS3, Line 78: ThreadSafeRandom rng(SeedRandom()); : int client_loc_idx = rng.Uniform(FLAGS_num_tablet_servers); : for (int i = 0; i < FLAGS_num_tablet_servers; i++) { : auto location = Substitute("/L$0", i); : EmplaceOrDie(&info, Substitute("/L$0", i), i == client_loc_idx ? 2 : 1); : } > Nit: combine: Done http://gerrit.cloudera.org:8080/#/c/12138/3/src/kudu/integration-tests/location_assignment-itest.cc@90 PS3, Line 90: // Find the tablet server that will be colocated with the client. > Is this really necessary here? Won't BuildAndStart() fail in some way? Done http://gerrit.cloudera.org:8080/#/c/12138/3/src/kudu/integration-tests/location_assignment-itest.cc@109 PS3, Line 109: vector<string> rows; : ASSERT_OK(ScanToStrings(&scanner, &rows)); : ASSERT_TRUE(rows.empty()); : > Why do we need a new KuduClient and KuduTable? Can't we reuse what's in Tab Done -- To view, visit http://gerrit.cloudera.org:8080/12138 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2c6bcc7479c5cf2e17cb6e368ca89a1eb7f21713 Gerrit-Change-Number: 12138 Gerrit-PatchSet: 5 Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Mon, 07 Jan 2019 18:33:24 +0000 Gerrit-HasComments: Yes
