KeDeng 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 5:

(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 te
Done


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;
              :
> OK. Besides using TestWorkload, is the default table created by SetUp() can
Done


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 ab
If we do not set 'split_size_bytes', will use the meta data in the cache 
without send rpc, that is the situation before modification.
We will send rpc to tservers to get the split key ranges if we set the 
'split_size_bytes'.


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?
I found duplicate data when I debug this case, and I'm not sure whether this is 
a fault. I will do more tests to check it.


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 SplitSizeBytes(uint64_t split_size_bytes) {
> How about using 'SplitSizeBytes' too?
Done



--
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: 5
Gerrit-Owner: KeDeng <[email protected]>
Gerrit-Reviewer: KeDeng <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <[email protected]>
Gerrit-Comment-Date: Tue, 25 Oct 2022 11:51:15 +0000
Gerrit-HasComments: Yes

Reply via email to