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

(10 comments)

http://gerrit.cloudera.org:8080/#/c/18945/6/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:

http://gerrit.cloudera.org:8080/#/c/18945/6/src/kudu/client/client-test.cc@8603
PS6, Line 8603:     KuduSchemaBuilder b;
> Is it necessary to create a diifferent schema with ClientTest? If not, it's
Done


http://gerrit.cloudera.org:8080/#/c/18945/6/src/kudu/client/client-test.cc@8654
PS6, Line 8654:       ASSERT_OK(session->Apply(insert.release()));
              :         ASSERT_OK(session->Flush());
              :       }
              :     }
              :   }
              :
              :  protected:
              :   static constexpr const char* const kTableName = 
"client-testrange";
              :
              :   shared_ptr<KuduTable> range_table_;
              : };
              :
              : TEST_F(TableKeyRangeTest, TestGetTableKeyRange) {
              :   client::sp::shared_ptr<KuduTable> table;
              :   ASSERT_OK(client_->OpenTable(kTableName, &table));
              :   {
              :     // Create session
              :     shared_ptr<KuduSession> session = client_->NewSession();
              :     session->SetTimeoutMillis(10000);
              :     ASSERT_OK(session->SetFlushMode(KuduSession::MANUAL_FLUSH));
              :
              :
> Can use InsertTestRows to insert data?
Done


http://gerrit.cloudera.org:8080/#/c/18945/6/src/kudu/client/client-test.cc@8700
PS6, Line 8700:     // If the splitSizeBytes set to 0 , we search the meta 
cache.
> The test should cover the case that return more tokens than tablet count, a
Done


http://gerrit.cloudera.org:8080/#/c/18945/6/src/kudu/client/client.h
File src/kudu/client/client.h:

http://gerrit.cloudera.org:8080/#/c/18945/6/src/kudu/client/client.h@3359
PS6, Line 3359: setSplitSizeBy
> Use the function name 'setSplitSizeBytes' would be better.
Done


http://gerrit.cloudera.org:8080/#/c/18945/6/src/kudu/client/meta_cache.h
File src/kudu/client/meta_cache.h:

http://gerrit.cloudera.org:8080/#/c/18945/6/src/kudu/client/meta_cache.h@412
PS6, Line 412:   struct RangeWithRemoteTablet {
> It seems nobody use it?
Done


http://gerrit.cloudera.org:8080/#/c/18945/6/src/kudu/client/meta_cache.h@446
PS6, Line 446:                   Looku
> nit: split_size_bytes
Done


http://gerrit.cloudera.org:8080/#/c/18945/6/src/kudu/client/meta_cache.h@452
PS6, Line 452:   // 'remote_tablet' if not null, and calling 
'lookup_complete_cb' once the
> As shown in https://google.github.io/styleguide/cppguide.html#Inputs_and_Ou
Done


http://gerrit.cloudera.org:8080/#/c/18945/6/src/kudu/client/meta_cache.h@453
PS6, Line 453: s with non-failed LEADE
> nit: split_size_bytes, which is similar to Java client.
Done


http://gerrit.cloudera.org:8080/#/c/18945/6/src/kudu/client/meta_cache.cc
File src/kudu/client/meta_cache.cc:

http://gerrit.cloudera.org:8080/#/c/18945/6/src/kudu/client/meta_cache.cc@1324
PS6, Line 1324: oDelta& timeout,
> nit: split_size_bytes
Done


http://gerrit.cloudera.org:8080/#/c/18945/6/src/kudu/client/scan_token-internal.h
File src/kudu/client/scan_token-internal.h:

http://gerrit.cloudera.org:8080/#/c/18945/6/src/kudu/client/scan_token-internal.h@93
PS6, Line 93:   uint64_t split_size_bytes_ = 0;
> nit: same use split_size_bytes_
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: 7
Gerrit-Owner: KeDeng <[email protected]>
Gerrit-Reviewer: KeDeng <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <[email protected]>
Gerrit-Comment-Date: Fri, 04 Nov 2022 08:19:09 +0000
Gerrit-HasComments: Yes

Reply via email to