Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17674 )

Change subject: KUDU-3304: [Alter] Support to alter table's replication factor
......................................................................


Patch Set 6: Code-Review+1

(1 comment)

LGTM though seems there's still feedback from Alexey to address regarding 
testing.

http://gerrit.cloudera.org:8080/#/c/17674/4/src/kudu/integration-tests/alter_table-test.cc
File src/kudu/integration-tests/alter_table-test.cc:

http://gerrit.cloudera.org:8080/#/c/17674/4/src/kudu/integration-tests/alter_table-test.cc@173
PS4, Line 173: scoped_refptr<TabletReplica> leader_replica;
             :     MonoTime deadline = MonoTime::Now() + timeout;
             :     std::unique_ptr<ThreadPool> pool;
             :     CHECK_OK(ThreadPoolBuilder("WaitUntilLeader.Pool")
             :                      .set_max_threads(num_tservers())
             :                      
.set_idle_timeout(MonoDelta::FromMilliseconds(10))
             :                      .Build(&pool));
             :     std::atomic<bool> got_leader(false);
             :     for (int i = 0; i < num_tservers(); ++i) {
             :       CHECK_OK(pool->Submit(
             :         [this, i, deadline, &got_leader, &leader_replica]() {
             :           while (!got_leader && MonoTime::Now() < deadline) {
             :             vector<scoped_refptr<TabletReplica> > replicas;
             :             
cluster_->mini_tablet_server(i)->server()->tablet_manager()
             :                 ->GetTabletReplicas(&replicas);
             :             if (replicas.empty()) {
             :               SleepFor(MonoDelta::FromMilliseconds(50));
             :               continue;
             :             }
             :
             :             MonoDelta remaining_timeout = deadline - 
MonoTime::Now();
             :             auto s = 
replicas[0]->consensus()->WaitUntilLeaderForTests(remaining_timeout);
             :             if (!s.ok()) {
             :               SleepFor(MonoDelta::FromMilliseconds(50));
             :               continue;
             :             }
             :
             :             got_leader = true;
             :             leade
> I improved these code a little.
Sure, though keep in mind methods like cluster_->mini_tablet_server_by_uuid() 
exist to facilitate getting server state.



--
To view, visit http://gerrit.cloudera.org:8080/17674
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3aa2d5b12c508ba761fa9410ad1a465cf31fb7d7
Gerrit-Change-Number: 17674
Gerrit-PatchSet: 6
Gerrit-Owner: Yingchun Lai <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <[email protected]>
Gerrit-Comment-Date: Thu, 15 Jul 2021 17:54:39 +0000
Gerrit-HasComments: Yes

Reply via email to