Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/20282 )
Change subject: KUDU-3498 Scanner keeps alive in periodically ...................................................................... Patch Set 8: (6 comments) http://gerrit.cloudera.org:8080/#/c/20282/6/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/20282/6/src/kudu/client/client-test.cc@2973 PS6, Line 2973: ASSERT_OK(KuduClientBuilder() : .add_master_server_addr(cluster_->mini_master()->bound_rpc_addr().ToString()) : .Build(&client_)); : : unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator()); : // Create a table with 3 hash partitions. : ASSERT_OK(table_creator->table_name(kTableName) : .schema(&schema_) : .num_replicas(3) : .add_hash_partitions({ "key" }, 3) : .timeout(MonoDelta::FromSeconds(60)) : > It would be great to do these prepare and clean up work in gtest's SetUp() Done http://gerrit.cloudera.org:8080/#/c/20282/6/src/kudu/client/client-test.cc@2985 PS6, Line 2985: : ASSERT_OK(client_->OpenTable(kTableName, &test_table_)); : NO_FATALS(InsertTestRows(test_table_.get(), 3000)); : : // Every tablet server owns 3 tablet replicas. : for (int i = 0; i < 3; i++) { : vector<string> tablet_ids = cluster_->mini_tablet_server(i)->ListTablets(); : ASSERT_EQ(3, tablet_ids.size()); : } : } : : shared_ptr<KuduTable> test_table_; : }; : : INSTANTIATE_TEST_SUITE_P(KeepAlivePeriodically, KeepAlivePeriodicallyTest, ::testing::Bool()); : : // Test case 1: 3 tablets is distributed in different tablet servers. : // When the scanner opens the next tablet, keepalive requests are sent : // to the other tablet server automatically. : TEST_P(KeepAlivePeriodicallyTest, TestScannerKeepAlivePeriodicallyCrossServers) { : SKIP_IF_SLOW_NOT_ALLOWED(); : : const auto keep_alive_periodically = GetParam(); > Is it enough to create a table just with hash partitions? Yes, it can. http://gerrit.cloudera.org:8080/#/c/20282/6/src/kudu/client/client-test.cc@3010 PS6, Line 3010: KuduScanner scanner(test_table_.get()); : ASSERT_OK(scanner.SetBatchSizeBytes(100)); : ASSERT_OK(scanner.Open()); : : // Start the keepalive timer. : if (keep_alive_periodically) { > How about using CreateTable() in ClientTest to simplify the code? CreateTable() in ClientTes can not add hash partitions. http://gerrit.cloudera.org:8080/#/c/20282/6/src/kudu/client/client-test.cc@3051 PS6, Line 3051: : : // Test case 2: The scanner are reading a tablet replica in a tablet : // server, if the tablet server is stopped, the client automatically : // restarts the scanning at some other tablet replica hosted by different : // tabl > How about: Done http://gerrit.cloudera.org:8080/#/c/20282/6/src/kudu/client/client-test.cc@3080 PS6, Line 3080: Stop the cu > Sepate the test in another test suite. Done http://gerrit.cloudera.org:8080/#/c/20282/6/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/20282/6/src/kudu/client/client.h@2958 PS6, Line 2958: /// @return It returns a non-OK if building a messeger failed or : /// the scanner is not opene > nit: indent Done -- To view, visit http://gerrit.cloudera.org:8080/20282 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1165d96814eb4bcd5db9b5cb60403fffc5b18c81 Gerrit-Change-Number: 20282 Gerrit-PatchSet: 8 Gerrit-Owner: Wang Xixu <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <[email protected]> Gerrit-Reviewer: Wang Xixu <[email protected]> Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Reviewer: Yuqi Du <[email protected]> Gerrit-Comment-Date: Thu, 21 Sep 2023 07:42:54 +0000 Gerrit-HasComments: Yes
