Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18945 )
Change subject: KUDU-3393 C++ client support split a tablet to mutil ranges and concurrent scan data ...................................................................... Patch Set 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/18945/3/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/18945/3/src/kudu/client/client-test.cc@439 PS3, Line 439: string buf; It's recommend to use ASSERT_* instead of CHECK_* in unit tests to avoid tests crash. The same to the other places. http://gerrit.cloudera.org:8080/#/c/18945/3/src/kudu/client/client-test.cc@6922 PS3, Line 6922: ASSERT_TRUE(it == it_next); : } : : { : KuduScanBatch::const_iterator it_end = batch.begin(); : std::advance(it_end, kRowNum); : ASSERT_TRUE(batch.end() == it_end); : } : : { : KuduScanBatch::const_iterator it(batch.begin()); : ASSERT_TRUE(it++ == batch.begin()); : : KuduScanBatch::const_iterator it_next(batch.begin()); : ASSERT_TRUE(++it_next == it); : } : : // Check the prefix increment iterator. : { : int count = 0; : for (KuduScanBatch::const_iterator it = batch.begin(); : it != batch.end(); ++it) { : ++count; : } : CHECK_EQ(ref_count, count); : } : : // Check the postfix increment iterator. : { : int count = 0; : for (KuduScanBatch::const_iterator it = batch.begin(); : it != batch.end(); it++) { : ++count; : } : CHECK_EQ(ref_count, count); : } : : // Check access to row via indirection operator * : // and member access through pointer operator -> : { : KuduScanBatch::const_iterator it(batch.begin()); : ASSERT_TRUE(it != batch.end()); : int32_t x = 0, y = 0; : ASSERT_OK((*it).GetInt32(0, &x)); : ASSERT_OK(it->GetInt32(0, &y)); : ASSERT_EQ(x, y); : } : : { : KuduScanBatch::const_iterator it_pre(batch.begin()); : KuduScanBatch::const_iterator it_post(batch.begin()); : for (; it_pre != batch.end(); ++it_pre, it_post++) { : ASSERT_TRUE(it_pre == it_post); : ASSERT_FALSE(it_pre != it_post); : } : } : : // Check that iterators which are going over different batches : // are different, even if they iterate over the same raw data. : { : KuduScanner other_scanner(client_table_.get()); : ASSERT_OK(other_scanner.Open()); : : KuduScanBatch other_batch; : > Do you mean I need to add this case to the integration test? But I think th OK. Besides using TestWorkload, is the default table created by SetUp() can be used? You can do a small refactor and/or add a new class inherit from ClientTest and override GenerateSplitRows() maybe. http://gerrit.cloudera.org:8080/#/c/18945/3/src/kudu/client/client-test.cc@6988 PS3, Line 6988: onst int other_ref_count(oth I'm not sure if I get your point, could you please describe more clearly about what cases do these tests want to check? thanks! http://gerrit.cloudera.org:8080/#/c/18945/3/src/kudu/client/client-test.cc@7010 PS3, Line 7010: ests that Why it's ASSERT_GE but not ASSERT_EQ? http://gerrit.cloudera.org:8080/#/c/18945/4/src/kudu/client/scan_token-internal.h File src/kudu/client/scan_token-internal.h: http://gerrit.cloudera.org:8080/#/c/18945/4/src/kudu/client/scan_token-internal.h@85 PS4, Line 85: void TargetChunkSizeBytes(uint64_t target_chunk_size_bytes) { How about using 'SplitSizeBytes' too? -- To view, visit http://gerrit.cloudera.org:8080/18945 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I207f9584cd558d32fcd9e8de7d6c25e517377272 Gerrit-Change-Number: 18945 Gerrit-PatchSet: 4 Gerrit-Owner: KeDeng <[email protected]> Gerrit-Reviewer: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Sat, 22 Oct 2022 16:38:34 +0000 Gerrit-HasComments: Yes
