You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Wang Xixu (Code Review)" <ge...@cloudera.org> on 2023/03/02 10:04:11 UTC

[kudu-CR] [KUDU-3452] Create tablet without enough healthy tservers

Wang Xixu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19571


Change subject: [KUDU-3452] Create tablet without enough healthy tservers
......................................................................

[KUDU-3452] Create tablet without enough healthy tservers

Currently, create a three-replicas table when there are not
enough tablet servers will fail and will retry continuously
and always fail until the unavailabe tserver becomes healthy
again.

An already created tablet can still be on service even if one
of its 3 replicas become unavailable. Why can not create a
three-replicas table when only 2 tablet servers are healthy?

This patch adds a new flag:
--enable_create_tablet_without_enough_healthy_tservers
to support this feature. The original logic keeps the same.
When this flag is set true, create tablet without enough
healthy tservers can success. And its status is under
replicating. Then this tablet can be be read and write normally.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-tool-test.cc
2 files changed, 47 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/1
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 1
Gerrit-Owner: Wang Xixu <14...@qq.com>

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................


Patch Set 20:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/master/catalog_manager.cc@6664
PS17, Line 6664:   }
               : 
> Verify that the number of replicas isn't greater than the number of registe
Done


http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/master/catalog_manager.cc@6677
PS17, Line 6677: , MasterErrorPB::REPLI
> Please use consensus::MajoritySize(num_replicas) here instead.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 20
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Tue, 09 May 2023 09:04:17 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 4:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/19571/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19571/3//COMMIT_MSG@7
PS3, Line 7: Create tablet without enough healthy tservers
> Having a Kudu cluster with just 3 tablet servers is already quite limiting 
Thank you for your comments. Please visit the related Jira and see the discussion.


http://gerrit.cloudera.org:8080/#/c/19571/3//COMMIT_MSG@10
PS3, Line 10: will retry continuously
> It seems that the catalog manager's 'bgtasks' thread will continuously try 
Yes


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc@417
PS3, Line 417: DEFINE_bool(enable_create_tablet_without_enough_healthy_tservers, false,
> We should also tag this new flag 'hidden', just as the 'catalog_manager_che
Done


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc@418
PS3, Line 418: creati
> creating
Done


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc@5933
PS3, Line 5933:       // NOTE: if we fail to select replicas on the first pass (due to
              :       // insufficient Tablet Servers being online), we will still try
              :       // again unless the tablet/table creation is cancelled.
              :       RETURN_NOT_OK_PREPEND(SelectReplicasForTablet(policy, tablet.get()),
              :                             Substitute("error selecting replicas for tablet $0",
              :                                        tablet->id()));
              :     }
              :   }
              : 
              :   // Update the sys catalog with the new set of tablets/metadata.
              :   {
              :     SysCatalogTable::Actions actions;
              :     actions.tablets_to_add = deferred.tablets_to_add;
              :     actions.tablets_to_update = deferred.tablets_to_update;
              :     RETURN_NOT_OK_PREPEND(sys_catalog_->Write(std::move(actions)),
              :                           "error persisting updated tablet metadata");
              :   }
              : 
              :   // Ex
> How about encapsulate the logic into SelectNReplicasForTablet()? since it's
Done


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc@5933
PS3, Line 5933:       // NOTE: if we fail to select replicas on the first pass (due to
              :       // insufficient Tablet Servers being online), we will still try
              :       // again unless the tablet/table creation is cancelled.
              :       RETURN_NOT_OK_PREPEND(SelectReplicasForTablet(policy, tablet.get()),
              :                             Substitute("error selecting replicas for tablet $0",
              :                                        tablet->id()));
              :     }
              :   }
              : 
              :   // Update the sys catalog with the new set of tablets/metadata.
              :   {
              :     SysCatalogTable::Actions actions;
              :     actions.tablets_to_add = deferred.tablets_to_add;
              :     actions.tablets_to_update = deferred.tablets_to_update;
              :     RETURN_NOT_OK_PREPEND(sys_catalog_->Write(std::move(actions)),
              :                           "error persisting updated tablet metadata");
              :   }
              : 
              :   // Ex
> +1
Done


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc@6006
PS3, Line 6006:                                                 "alive tablet servers",
> I didn't find any where else using SelectReplicasForTablet(), why not remov
Done


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc@6006
PS3, Line 6006:                                                 "alive tablet servers",
> +1
Done


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/tools/kudu-tool-test.cc@9204
PS3, Line 9204:   NO_FATALS(cluster_->tablet_server(0)->Shutdown());
> nit: use cluster_->WaitForTabletServerCount() to ensure this.
WaitForTabletServerCount is a function of InternalMiniCluster, not a function of ExternalMiniCluster.  And I have not found a way to start an InternalMiniCluster with specified flags, like --catalog_manager_check_ts_count_for_create_table.


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/tools/kudu-tool-test.cc@9228
PS3, Line 9228:     ASSERT_EQ("UNDER_REPLICATED", items[i]["health"]);
> Would be great to test adding range partitions when there is only 2 tserver
Done


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/tools/kudu-tool-test.cc@9235
PS3, Line 9235: rer->AddColumn("new_c
> Why do you need RunActionStdoutString() here if there isn't any analysis pe
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 4
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 27 Mar 2023 10:28:01 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................


Patch Set 17:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/tools/kudu-tool-test.cc@9175
PS17, Line 9175: class NotEnoughHealthyTServersTest :
The test is irrelevant to kudu tools. Considering the size of this file is already pretty significant, it should be put somewhere else.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 17
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 08 May 2023 07:45:39 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................


Patch Set 23: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 23
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 15 May 2023 06:36:54 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 14:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/19571/14/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/14/src/kudu/master/catalog_manager.cc@6677
PS14, Line 6677: (num_replicas + 1) / 2 > num_live_tservers
I think this equivalent condition may be better readability.

if (num_live_tservers < (num_replicas + 1) / 2)


http://gerrit.cloudera.org:8080/#/c/19571/14/src/kudu/master/catalog_manager.cc@6678
PS14, Line 6678: InvalidArgument
using IllegalState ?


http://gerrit.cloudera.org:8080/#/c/19571/14/src/kudu/master/catalog_manager.cc@6682
PS14, Line 6682: registered_ts_descs.size()
num_live_tservers



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 14
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 06 May 2023 04:04:02 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 4:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/19571/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19571/4//COMMIT_MSG@10
PS4, Line 10: tablet servers
tservers


http://gerrit.cloudera.org:8080/#/c/19571/4//COMMIT_MSG@11
PS4, Line 11: unavailabe
nit: unavailable


http://gerrit.cloudera.org:8080/#/c/19571/4//COMMIT_MSG@11
PS4, Line 11: tserver becomes
tservers become


http://gerrit.cloudera.org:8080/#/c/19571/4//COMMIT_MSG@15
PS4, Line 15: Create
So creating


http://gerrit.cloudera.org:8080/#/c/19571/4//COMMIT_MSG@16
PS4, Line 16: when only 2
when there are only 2 healthy tservers


http://gerrit.cloudera.org:8080/#/c/19571/4//COMMIT_MSG@16
PS4, Line 16: support
nit: supported


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc@5945
PS3, Line 5945: 
nit: protocol


http://gerrit.cloudera.org:8080/#/c/19571/4/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/4/src/kudu/master/catalog_manager.cc@5988
PS4, Line 5988:                                                 
nit:  unexpected extra space ?


http://gerrit.cloudera.org:8080/#/c/19571/4/src/kudu/master/catalog_manager.cc@6001
PS4, Line 6001: replica_factor
nit: replication_factor


http://gerrit.cloudera.org:8080/#/c/19571/4/src/kudu/master/catalog_manager.cc@6002
PS4, Line 6002: or
Otherwise,


http://gerrit.cloudera.org:8080/#/c/19571/4/src/kudu/master/catalog_manager.cc@6004
PS4, Line 6004: protocal
nit: protocol


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc@9213
PS2, Line 9213: Start());
> Ksck can not make master know that tablet server is offline. Tablet server 
So?SleepFor (2 * FLAGS_tserver_unresponsive_timeout_ms ) ?  And above( L9203)


http://gerrit.cloudera.org:8080/#/c/19571/4/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/4/src/kudu/tools/kudu-tool-test.cc@9239
PS4, Line 9239:   NO_FATALS(cluster_->Restart());
It's no need restart all kudu cluster.  The case should make sure 'cluster_->tablet_server(0)->Start(); ' can resume the unhealthy state.


Another way to resume this table to health is adding a new tserver, you can add this case.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 4
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 27 Mar 2023 11:19:33 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 10: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 10
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sun, 23 Apr 2023 06:14:06 +0000
Gerrit-HasComments: No

[kudu-CR] [KUDU-3452] Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: [KUDU-3452] Create tablet without enough healthy tservers
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/master/catalog_manager.cc@5930
PS2, Line 5930:       master_->ts_manager()->GetDescriptorsAvailableForPlacement(&ts_descs);
> Why move the statement from outer to inner loop? I infer that at a unsteady
Why move the statement from outer to inner loop? It used healthy tablet servers totally before. But now the case is different. For example, there are 3 tablet servers, one is unhealthy. Some tablet defined three-replicas, some defined two-replicas. The case is different for different tablets. Therefore, the logic is: firstly, try to use the healthy tablet servers, if it is not enough, try to use unhealthy tablet servers.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 2
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Fri, 10 Mar 2023 06:01:54 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 11:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/master/catalog_manager.cc@5999
PS11, Line 5999: select policy.ts_num() replicas
place the majority of replicas


http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc@9318
PS11, Line 9318:     // Restart the second tablet server.
Why we need to start the second tablet server? Seems that three live tablet server is enough for creating 5-replica tables.


http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc@9362
PS11, Line 9362:   ASSERT_OK(AlterReplicationFactor(kOneReplicaTableName, 5));
Maybe we should also test that if alter table operations(e.g. add a column, add a range partition, etc.) can be applied to these 'UNDER_REPLICATED' tables?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 11
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 04 May 2023 16:17:43 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#13).

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................

KUDU-3452 Create tablet without enough healthy tservers

Currently, creating a three-replicas table when there are
not enough tablet servers will fail and the system catalog
will retry continuously and always fail until the unavailable
tablet servers becomes healthy again.

An under-replicated table is still available for reading and
writing, so it's enough to place just a majority of replicas
for each tablet at healthy tablet servers to make a newly
created table ready to use.

This patch adds a new flag:
--allow_creating_under_replicated_tables to support this
feature. The original logic is kept the same. When this flag
is set true, it's possible to create a tablet placing just a
majority of replicas at healthy tablet servers. Even if the
new tablet is created under-replicated, it's still available
for read and write operations.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/client/client.h
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-tool-test.cc
3 files changed, 283 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/13
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 13
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 15:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/19571/14/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/14/src/kudu/master/catalog_manager.cc@6677
PS14, Line 6677: num_live_tservers < (num_replicas + 1) / 2
> I think this equivalent condition may be better readability.
Done


http://gerrit.cloudera.org:8080/#/c/19571/14/src/kudu/master/catalog_manager.cc@6678
PS14, Line 6678: InvalidArgument
> using IllegalState ?
I think InvalidArgument is better. Here are arguments checking for creating table.


http://gerrit.cloudera.org:8080/#/c/19571/14/src/kudu/master/catalog_manager.cc@6682
PS14, Line 6682: num_live_tservers, registe
> num_live_tservers
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 15
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 06 May 2023 04:13:37 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................


Patch Set 17: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 17
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 08 May 2023 07:13:01 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc@5933
PS3, Line 5933:       int nreplicas = 0;
              :       // Get replica factor.
              :       {
              :         TableMetadataLock table_guard(tablet->table().get(), LockMode::READ);
              :         nreplicas = table_guard.data().pb.num_replicas();
              :       }
              :       // Try to create a table without enough alive tablet servers.
              :       if (alive_num_ts < nreplicas &&
              :           FLAGS_enable_create_tablet_without_enough_healthy_tservers) {
              :         // RAFT protocol require: (replica_factor + 1) / 2 >= the number of alive tservers,
              :         // or it can not run.
              :         if (alive_num_ts < (nreplicas + 1) / 2) {
              :           return Status::InvalidArgument(Substitute("RAFT protocal requires at least $0 "
              :                                                     "alive tablet servers, but only left $1 "
              :                                                     "alive tablet servers",
              :                                                     ((nreplicas + 1) / 2), alive_num_ts));
              :         }
              :         nreplicas = alive_num_ts;
              :       }
How about encapsulate the logic into SelectNReplicasForTablet()? since it's a part of "select replicas" IMO.


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc@6006
PS3, Line 6006: Status CatalogManager::SelectReplicasForTablet(const PlacementPolicy& policy,
I didn't find any where else using SelectReplicasForTablet(), why not remove it?


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/tools/kudu-tool-test.cc@9204
PS3, Line 9204:   SleepFor(MonoDelta::FromMilliseconds(2000));
nit: use cluster_->WaitForTabletServerCount() to ensure this.


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/tools/kudu-tool-test.cc@9228
PS3, Line 9228: 
Is it able to alter the table in this case?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 3
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 20 Mar 2023 08:40:09 +0000
Gerrit-HasComments: Yes

[kudu-CR] [KUDU-3452] Create tablet without enough healthy tservers

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: [KUDU-3452] Create tablet without enough healthy tservers
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/19571/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19571/2//COMMIT_MSG@7
PS2, Line 7: [KUDU-3452] Create tablet without enough healthy tservers
add a tag like: [master] ? 

[KUDU-3452] ---> KUDU-3452

eg: 

[master] KUDU-3452 Create tablet without enough healthy tservers


http://gerrit.cloudera.org:8080/#/c/19571/2//COMMIT_MSG@21
PS2, Line 21: create tablet without enough
            : healthy tservers can success. And its status is under
            : replicating
We don't need the number of healthy tserver greater or equal 'replication_factor', but the number of healthy tservers should statisfy  (replication_factor+1/2), it's requirement of availability of raft consensus algorithm.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 2
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Tue, 07 Mar 2023 06:24:25 +0000
Gerrit-HasComments: Yes

[kudu-CR] [KUDU-3452] Create tablet without enough healthy tservers

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: [KUDU-3452] Create tablet without enough healthy tservers
......................................................................


Patch Set 2:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/master/catalog_manager.cc@5930
PS2, Line 5930:       master_->ts_manager()->GetDescriptorsAvailableForPlacement(&ts_descs);
Why move the statement from outer to inner loop? I infer that at a unsteady kudu cluster it can pick healthy tserver ASAP?

At this, I have different opinion about this which is relation to lines from L5933 to L5938, you can see below


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/master/catalog_manager.cc@5931
PS2, Line 5931:       TableMetadataLock table_guard(tablet->table().get(), LockMode::READ);
Whether the lock can put into {   TableMetadataLock table_guard(tablet->table().get(), LockMode::READ); ...  }  to reduce lock code ranges.


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/master/catalog_manager.cc@5932
PS2, Line 5932:       // Healthy tablet servers are not enough, use unhealthy tablet servers.
              :       if (FLAGS_enable_create_tablet_without_enough_healthy_tservers &&
              :         ts_descs.size() < table_guard.data().pb.num_replicas()) {
              :         ts_descs.clear();
              :         master_->ts_manager()->GetAllDescriptors(&ts_descs);
              :       }
At this, I have different opinion about this which may relation to lines from L5930.

For example, pick an unhealthy tserver, that means the left replica must create at the specific unhealthy tserver, but the tserver may dead for a long time, and during this time, a new tserver joins cluster, the tablet should have been unhealthy.

So IMO, if  healthy tservers are not enough by 'GetDescriptorsAvailableForPlacement', if the number statisfies the (n+1)/2, that's ok. When a third server joins or resume from fault, it can become health by recovering flow. And it does not matter putting  L5930  before the loop.


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc@9182
PS2, Line 9182: catalog_manager_check_ts_count_for_create_table
'catalog_manager_check_ts_count_for_create_table''s default value is true.  If the test case set it false. That means the feature should change the flag, right?


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc@9203
PS2, Line 9203:   SleepFor(MonoDelta::FromMilliseconds(3000));
replacing the 'SleepFor' by checking 'ksck not OK' ?


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc@9209
PS2, Line 9209:   NO_FATALS(workload2.Start());
IMO, it's better start a write thread and write some records to prove the table is avaliable.


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc@9213
PS2, Line 9213: FromMilliseconds
replacing the 'SleepFor' by checking 'ksck not OK' ?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 2
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Tue, 07 Mar 2023 06:14:27 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#5).

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................

KUDU-3452 Create tablet without enough healthy tservers

Currently, create a three-replicas table when there are not
enough tablet servers will fail and will retry continuously
and always fail until the unavailable tablet servers becomes
healthy again.

An already created tablet can still be on service even if
one of its 3 replicas become unavailable. So creating a
three-replicas table when there are only 2 tablet servers
can be supported.

This patch adds a new flag:
--enable_create_tablet_without_enough_healthy_tservers
to support this feature. The original logic keeps the same.
When this flag is set true, create tablet without enough
healthy tservers can success. And its status is under
replicating. Then this tablet can be be read and write normally.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-tool-test.cc
2 files changed, 96 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/5
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 5
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#23).

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................

KUDU-3452 Allow creating tablets under replicated tables

Currently, creating a three-replicas table when there are
not less than 3 healthy tablet servers will fail and the system
catalog will retry continuously and always fail until the
unavailable tablet servers becomes healthy again.

An under-replicated table is still available for reading and
writing, so it's enough to place just a majority of replicas
for each tablet at healthy tablet servers to make a newly
created table ready to use.

This patch adds a new flag:
--allow_creating_under_replicated_tables to support this
feature. The original logic is kept the same. When this flag
is set true, it's possible to create a tablet placing just a
majority of replicas at healthy tablet servers. Even if the
new tablet is created under-replicated, it's still available
for read and write operations.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/integration-tests/create-table-itest.cc
M src/kudu/master/catalog_manager.cc
2 files changed, 201 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/23
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 23
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................


Patch Set 17: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 17
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 06 May 2023 11:21:42 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#15).

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................

KUDU-3452 Create tablet without enough healthy tservers

Currently, creating a three-replicas table when there are
not enough tablet servers will fail and the system catalog
will retry continuously and always fail until the unavailable
tablet servers becomes healthy again.

An under-replicated table is still available for reading and
writing, so it's enough to place just a majority of replicas
for each tablet at healthy tablet servers to make a newly
created table ready to use.

This patch adds a new flag:
--allow_creating_under_replicated_tables to support this
feature. The original logic is kept the same. When this flag
is set true, it's possible to create a tablet placing just a
majority of replicas at healthy tablet servers. Even if the
new tablet is created under-replicated, it's still available
for read and write operations.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/client/client.h
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-tool-test.cc
3 files changed, 253 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/15
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 15
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................


Patch Set 17:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/master/catalog_manager.cc@6003
PS17, Line 6003:     // RAFT protocol requires: (replication_factor + 1) / 2 >= the number
               :     // of alive tservers, otherwise, it can not run.
nit: consider updating this explanatory comment

The Raft protocol requires at least (replication_factor / 2) + 1 live replicas to form a quorum.  Since Kudu places at most one replica of a tablet at one tablet server, it's necessary to have at least (replication_factor / 2) + 1 live tablet servers in a cluster to allow the tablet to serve read and write requests.


http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/master/catalog_manager.cc@6005
PS17, Line 6005: (nreplicas + 1) / 2
Please consider using consensus::MajoritySize(num_replicas) here instead.  '(nreplicas + 1) / 2' isn't correct for even replications factors: e.g., consider RF=4 -- in this case, the majority is 3 replicas, not 2.


http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/master/catalog_manager.cc@6006
PS17, Line 6006: "RAFT protocol requires at least $0 "
               :                                                 "alive tablet servers, but only left $1 "
               :                                                 "alive tablet servers
nit: consider updating to clarify

need at least $0 out of $1 replicas to form a Raft quorum, but only $1 tablet servers are online


http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/master/catalog_manager.cc@6664
PS17, Line 6664:   // Verify that the number of replicas isn't greater than the number of registered tablet
               :   // servers.
Verify that the number of replicas isn't greater than the number of registered tablet servers in the cluster. This is to make sure the cluster can provide the expected HA guarantees when all tablet servers are online.  Essentially, this allows to be sure no tablet stays under-replicated for indefinite time when all the nodes of the cluster are online.


http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/master/catalog_manager.cc@6677
PS17, Line 6677: (num_replicas + 1) / 2
Please use consensus::MajoritySize(num_replicas) here instead.


http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/tools/kudu-tool-test.cc@9175
PS17, Line 9175: class NotEnoughHealthyTServersTest :
> The test is irrelevant to kudu tools. Considering the size of this file is 
+1



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 17
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Tue, 09 May 2023 00:26:03 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 11:

(15 comments)

http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/master/catalog_manager.cc@418
PS10, Line 418: enough
> Ah, sure, sorry my mistake.
Done


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/master/catalog_manager.cc@5999
PS10, Line 5999: replic
> What does "enough" mean, does 1 out of RF = 3 is enough? How about to descr
Done


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/master/catalog_manager.cc@6008
PS10, Line 6008: "
> Remove duplicate brackets.
Done


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/master/catalog_manager.cc@6663
PS10, Line 6663: 
> nit: greater
Done


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9196
PS10, Line 9196: const
> nit: this could be constexpr as well
Done


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9227
PS10, Line 9227:                   .set_range_partition_columns({ "foo" })
> nit: Check the type of Status fastly, i.e. :
Done


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9234
PS10, Line 9234:     ASSERT_TRUE(out.empty());
> Also check the table 'kOverRegistedTSTableName' is not existed in the clust
Done


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9245
PS10, Line 9245:     }
> IIUC, ExternalDaemon::Shutdown() actually awaits for the process to be stop
Yes, here waits until catalog manager (i.e. master) sees just 2 tablet servers alive instead of 5 actually used by GetDescriptorsAvailableForPlacement().


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9247
PS10, Line 9247: 
> nit: not
Done


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9264
PS10, Line 9264:   }
> nit: Check the type of Status fastly, i.e. :
Done


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9267
PS10, Line 9267: 
> For a tablet with RF=5, for a positive scenario we are interested to see if
Done


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9272
PS10, Line 9272:               ->Type(client::KuduColumnSchema::INT32)
> +1
Done


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9296
PS10, Line 9296: RT_EQ(5, do
> Altering the replication factor is not a 'regular' sort of ALTER TABLE.  Co
Well, I have try to add a column, but it doesn't validate the number of replicas. Only altering the replication factor will validate the number of replicas. Please see Line 3629(function ValidateNumberReplicas()) in catalog_manager.cc.


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9299
PS10, Line 9299: i = 
> nit: will be
OK, I will specify it.


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9325
PS10, Line 9325: rapidjson::D
> nit: it's much cleaner to use variables local to the scope instead of relyi
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 11
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Wed, 26 Apr 2023 03:08:30 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#11).

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................

KUDU-3452 Create tablet without enough healthy tservers

Currently, creating a three-replicas table when there are
not enough tablet servers will fail and the system catalog
will retry continuously and always fail until the unavailable
tablet servers becomes healthy again.

An under-replicated table is still available for reading and
writing, so it's enough to place just a majority of replicas
for each tablet at healthy tablet servers to make a newly
created table ready to use.

This patch adds a new flag:
--allow_creating_under_replicated_tables to support this
feature. The original logic is kept the same. When this flag
is set true, it's possible to create a tablet placing just a
majority of replicas at healthy tablet servers. Even if the
new tablet is created under-replicated, it's still available
for read and write operations.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/client/client.h
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-tool-test.cc
3 files changed, 271 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/11
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 11
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#21).

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................

KUDU-3452 Allow creating tablets under replicated tables

Currently, creating a three-replicas table when there are
not less than 3 healthy tablet servers will fail and the system
catalog will retry continuously and always fail until the
unavailable tablet servers becomes healthy again.

An under-replicated table is still available for reading and
writing, so it's enough to place just a majority of replicas
for each tablet at healthy tablet servers to make a newly
created table ready to use.

This patch adds a new flag:
--allow_creating_under_replicated_tables to support this
feature. The original logic is kept the same. When this flag
is set true, it's possible to create a tablet placing just a
majority of replicas at healthy tablet servers. Even if the
new tablet is created under-replicated, it's still available
for read and write operations.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/integration-tests/create-table-itest.cc
M src/kudu/master/catalog_manager.cc
2 files changed, 199 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/21
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 21
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 3:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/19571/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19571/3//COMMIT_MSG@7
PS3, Line 7: Create tablet without enough healthy tservers
Having a Kudu cluster with just 3 tablet servers is already quite limiting -- if any of these 3 nodes fail, there isn't a place to replicas which are to migrate from the failed node.  So, a minimum recommended number of tablet servers nodes is a Kudu cluster is 4 already.

Allowing to create a table with RF=3 when only two tablet servers are alive is asking for a disaster, because the table's data will become non-available when just one more tablet server node goes down.

Why to create tables with RF=3 then?  You could create tables with RF=1 and have just one tablet server in your cluster.

I'm not sure I understand the reasoning behind this change.


http://gerrit.cloudera.org:8080/#/c/19571/3//COMMIT_MSG@10
PS3, Line 10: will retry continuously
I'm not sure I understand what will retry continuously here?  What is component that performs those retries once the request to create a table is rejected by the system catalog?


http://gerrit.cloudera.org:8080/#/c/19571/3//COMMIT_MSG@14
PS3, Line 14: An already created tablet can still be on service even if one
            : of its 3 replicas become unavailable.
What happens if another tablet server is going down?


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc@418
PS3, Line 418: create
creating


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc@418
PS3, Line 418: enough healthy
What does 'enough' here means?  Please be more specific.

Would be just 2 tablet servers enough for a table with replication factor of 5?


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/tools/kudu-tool-test.cc@9172
PS3, Line 9172: TEST_F(ToolTest, TestCreateTabletWithoutEnoughHealthyTServers) {
Would be great to have a test for various replication factors here.

In addition to RF=3, most commonly used ones are 1 and 5.


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/tools/kudu-tool-test.cc@9235
PS3, Line 9235: RunActionStdoutString
Why do you need RunActionStdoutString() here if there isn't any analysis performed on the result output?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 3
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 20 Mar 2023 18:01:45 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#8).

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................

KUDU-3452 Create tablet without enough healthy tservers

Currently, creating a three-replicas table when there are
not enough tablet servers will fail and the system catalog
will retry continuously and always fail until the unavailable
tablet servers becomes healthy again.

An under-replicated table is still available for reading and
writing, so it's enough to place just a majority of replicas
for each tablet at healthy tablet servers to make a newly
created table ready to use.

This patch adds a new flag:
--allow_creating_under_replicated_tables to support this
feature. The original logic is kept the same. When this flag
is set true, it's possible to create a tablet placing just a
majority of replicas at healthy tablet servers. Even if the
new tablet is created under-replicated, it's still available
for read and write operations.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-tool-test.cc
2 files changed, 203 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/8
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 8
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 12:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/master/catalog_manager.cc@5999
PS11, Line 5999: select policy.ts_num() replicas
> place the majority of replicas
Done


http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc@9189
PS11, Line 9189: INSTANTIATE_TEST_SUITE_P
> nit: add an empty line
Done


http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc@9283
PS11, Line 9283:                   .Create();
> The status is deterministic, I think it should be not timeout. What do you 
It will timeout. Because there are 5 tablet servers totally, and 3 tablet servers is stopped. Creating a table with RF=5 will timeout, because (5+1)/2 > 2. There are not enough live tablet severs to place the replicas.


http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc@9318
PS11, Line 9318:   {
> Why we need to start the second tablet server? Seems that three live tablet
The case with 3 live tablet servers creating 5-replica tables has been tested, see line 9308.
This case tests creating 5-replica tables with 4 live tablet servers.


http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc@9362
PS11, Line 9362:   // Add another column. Test alter table logic.
> Maybe we should also test that if alter table operations(e.g. add a column,
Done


http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc@9387
PS11, Line 9387: 
> nit: All tables
Done


http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc@9396
PS11, Line 9396: _FATALS(c
> I don't understand  the number 4.
There are 5 creating table commands, but only 4 commands will enter the stage of background creating tablets task. 
The first creating table with RF=6 will return directly, because its replication_factor > all register tablet severs, it will get a invalid argument error.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 12
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Fri, 05 May 2023 06:43:53 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 3:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/19571/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19571/2//COMMIT_MSG@7
PS2, Line 7: KUDU-3452 Create tablet without enough healthy tservers
> add a tag like: [master] ? 
Done


http://gerrit.cloudera.org:8080/#/c/19571/2//COMMIT_MSG@21
PS2, Line 21: create tablet without enough
            : healthy tservers can success. And its status is under
            : replicating
> We don't need the number of healthy tserver greater or equal 'replication_f
ok


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/master/catalog_manager.cc@5930
PS2, Line 5930:     const int alive_num_ts = ts_descs.size();
> Why move the statement from outer to inner loop? It used healthy tablet ser
Done


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/master/catalog_manager.cc@5931
PS2, Line 5931:     PlacementPolicy policy(std::move(ts_descs), &rng_);
> Whether the lock can put into {   TableMetadataLock table_guard(tablet->tab
Done


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/master/catalog_manager.cc@5932
PS2, Line 5932:     for (auto& tablet : deferred.needs_create_rpc) {
              :       int nreplicas = 0;
              :       // Get replica factor.
              :       {
              :         TableMetadataLock table_guard(tablet->table().get(), LockMode::READ);
              :        
> At this, I have different opinion about this which may relation to lines fr
Done


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc@9182
PS2, Line 9182: .
> 'catalog_manager_check_ts_count_for_create_table''s default value is true. 
Yes


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc@9203
PS2, Line 9203:   // Sleep for 2s to make the state of one tab
> replacing the 'SleepFor' by checking 'ksck not OK' ?
Ksck can not make master know that tablet server is offline. Tablet server will report its status through heart beat every tserver_unresponsive_timeout_ms time.


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc@9209
PS2, Line 9209:   workload2.set_table_name(kTableName2);
> IMO, it's better start a write thread and write some records to prove the t
Done


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc@9213
PS2, Line 9213: 2.rows_inserted(
> replacing the 'SleepFor' by checking 'ksck not OK' ?
Ksck can not make master know that tablet server is offline. Tablet server will report its status through heart beat every tserver_unresponsive_timeout_ms time.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 3
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 13 Mar 2023 07:44:02 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19571/15/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/15/src/kudu/tools/kudu-tool-test.cc@9338
PS15, Line 9338:         Substitute("cluster ksck $0 --sections=TABLE_SUMMARIES --ksck_format=json_compact",
If moving the test to client-test.cc, you are able to access the master status conviently by InternalMiniCluster, no needed to use CLI tools to list table or ksck.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 15
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 06 May 2023 07:54:18 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#18).

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................

KUDU-3452 Allow creating tablets under replicated tables

Currently, creating a three-replicas table when there are
not less than 3 healthy tablet servers will fail and the system
catalog will retry continuously and always fail until the
unavailable tablet servers becomes healthy again.

An under-replicated table is still available for reading and
writing, so it's enough to place just a majority of replicas
for each tablet at healthy tablet servers to make a newly
created table ready to use.

This patch adds a new flag:
--allow_creating_under_replicated_tables to support this
feature. The original logic is kept the same. When this flag
is set true, it's possible to create a tablet placing just a
majority of replicas at healthy tablet servers. Even if the
new tablet is created under-replicated, it's still available
for read and write operations.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/client/client.h
M src/kudu/integration-tests/create-table-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-tool-test.cc
4 files changed, 204 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/18
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 18
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................


Patch Set 23:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/19571/22/src/kudu/integration-tests/create-table-itest.cc
File src/kudu/integration-tests/create-table-itest.cc:

http://gerrit.cloudera.org:8080/#/c/19571/22/src/kudu/integration-tests/create-table-itest.cc@647
PS22, Line 647: constexpr int kTSUnresponsiveTimeoutMs = 4000;
              :   constexpr int kHeartbeatIntervalMs = 3000;
> Why is the heartbeat interval even longer than tserver unresponsive interva
Done


http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/master/catalog_manager.cc@6664
PS17, Line 6664:   }
               : 
> Seems this comment was not resolved in patch set 22.
Done


http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/master/catalog_manager.cc@6677
PS17, Line 6677: let servers are regist
> ditto.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 23
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Fri, 12 May 2023 02:14:52 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................


Patch Set 23:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19571/23/src/kudu/integration-tests/create-table-itest.cc
File src/kudu/integration-tests/create-table-itest.cc:

http://gerrit.cloudera.org:8080/#/c/19571/23/src/kudu/integration-tests/create-table-itest.cc@703
PS23, Line 703:   {
              :     // Restart the first tablet server.
              :     NO_FATALS(cluster_->tablet_server(0)->Restart());
              :     // Wait the restarted tablet server to send a heartbeat and be registered in catalog manaager.
              :     SleepFor(MonoDelta::FromMilliseconds(kHeartbeatIntervalMs));
              :   }
              : 
              :   // Create a table with RF=5. It should succeed.
              :   ASSERT_OK(create_table_func(kFiveReplicaTableName, 5));
> Thank you for your comments. I have tested this case for many times, but ha
What about using 'cluster_.WaitForTabletServerCount'? Maybe in some cases sleeping for a specified time may not be enough to make sure the tserver has registered on the master.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 23
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 15 May 2023 07:49:12 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#10).

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................

KUDU-3452 Create tablet without enough healthy tservers

Currently, creating a three-replicas table when there are
not enough tablet servers will fail and the system catalog
will retry continuously and always fail until the unavailable
tablet servers becomes healthy again.

An under-replicated table is still available for reading and
writing, so it's enough to place just a majority of replicas
for each tablet at healthy tablet servers to make a newly
created table ready to use.

This patch adds a new flag:
--allow_creating_under_replicated_tables to support this
feature. The original logic is kept the same. When this flag
is set true, it's possible to create a tablet placing just a
majority of replicas at healthy tablet servers. Even if the
new tablet is created under-replicated, it's still available
for read and write operations.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/client/client.h
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-tool-test.cc
3 files changed, 205 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/10
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 10
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/master/catalog_manager.cc@418
PS10, Line 418: enough
> Yingchun, to me it looks correct if you read the whole sentence: ... enough
Ah, sure, sorry my mistake.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 10
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Tue, 25 Apr 2023 05:06:50 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/19571/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19571/3//COMMIT_MSG@10
PS3, Line 10: will retry continuously
> I'm not sure I understand what will retry continuously here?  What is compo
It seems that the catalog manager's 'bgtasks' thread will continuously try to assign replicas for newly created tablets(in PREPARING state), and then it fails to select as many alive tservers as needed.


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc@417
PS3, Line 417: DEFINE_bool(enable_create_tablet_without_enough_healthy_tservers, false,
We should also tag this new flag 'hidden', just as the 'catalog_manager_check_ts_count_for_create_table' flag.


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc@5933
PS3, Line 5933:       int nreplicas = 0;
              :       // Get replica factor.
              :       {
              :         TableMetadataLock table_guard(tablet->table().get(), LockMode::READ);
              :         nreplicas = table_guard.data().pb.num_replicas();
              :       }
              :       // Try to create a table without enough alive tablet servers.
              :       if (alive_num_ts < nreplicas &&
              :           FLAGS_enable_create_tablet_without_enough_healthy_tservers) {
              :         // RAFT protocol require: (replica_factor + 1) / 2 >= the number of alive tservers,
              :         // or it can not run.
              :         if (alive_num_ts < (nreplicas + 1) / 2) {
              :           return Status::InvalidArgument(Substitute("RAFT protocal requires at least $0 "
              :                                                     "alive tablet servers, but only left $1 "
              :                                                     "alive tablet servers",
              :                                                     ((nreplicas + 1) / 2), alive_num_ts));
              :         }
              :         nreplicas = alive_num_ts;
              :       }
> How about encapsulate the logic into SelectNReplicasForTablet()? since it's
+1


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc@6006
PS3, Line 6006: Status CatalogManager::SelectReplicasForTablet(const PlacementPolicy& policy,
> I didn't find any where else using SelectReplicasForTablet(), why not remov
+1


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/tools/kudu-tool-test.cc@9228
PS3, Line 9228: 
> Is it able to alter the table in this case?
Would be great to test adding range partitions when there is only 2 tservers alive.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 3
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 23 Mar 2023 08:49:27 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#19).

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................

KUDU-3452 Allow creating tablets under replicated tables

Currently, creating a three-replicas table when there are
not less than 3 healthy tablet servers will fail and the system
catalog will retry continuously and always fail until the
unavailable tablet servers becomes healthy again.

An under-replicated table is still available for reading and
writing, so it's enough to place just a majority of replicas
for each tablet at healthy tablet servers to make a newly
created table ready to use.

This patch adds a new flag:
--allow_creating_under_replicated_tables to support this
feature. The original logic is kept the same. When this flag
is set true, it's possible to create a tablet placing just a
majority of replicas at healthy tablet servers. Even if the
new tablet is created under-replicated, it's still available
for read and write operations.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/integration-tests/create-table-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-tool-test.cc
3 files changed, 200 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/19
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 19
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................


Patch Set 23: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 23
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Fri, 12 May 2023 07:34:00 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#20).

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................

KUDU-3452 Allow creating tablets under replicated tables

Currently, creating a three-replicas table when there are
not less than 3 healthy tablet servers will fail and the system
catalog will retry continuously and always fail until the
unavailable tablet servers becomes healthy again.

An under-replicated table is still available for reading and
writing, so it's enough to place just a majority of replicas
for each tablet at healthy tablet servers to make a newly
created table ready to use.

This patch adds a new flag:
--allow_creating_under_replicated_tables to support this
feature. The original logic is kept the same. When this flag
is set true, it's possible to create a tablet placing just a
majority of replicas at healthy tablet servers. Even if the
new tablet is created under-replicated, it's still available
for read and write operations.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/integration-tests/create-table-itest.cc
M src/kudu/master/catalog_manager.cc
2 files changed, 199 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/20
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 20
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................

KUDU-3452 Allow creating tablets under replicated tables

Currently, creating a three-replicas table when there are
not less than 3 healthy tablet servers will fail and the system
catalog will retry continuously and always fail until the
unavailable tablet servers becomes healthy again.

An under-replicated table is still available for reading and
writing, so it's enough to place just a majority of replicas
for each tablet at healthy tablet servers to make a newly
created table ready to use.

This patch adds a new flag:
--allow_creating_under_replicated_tables to support this
feature. The original logic is kept the same. When this flag
is set true, it's possible to create a tablet placing just a
majority of replicas at healthy tablet servers. Even if the
new tablet is created under-replicated, it's still available
for read and write operations.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Reviewed-on: http://gerrit.cloudera.org:8080/19571
Tested-by: Kudu Jenkins
Reviewed-by: Yifan Zhang <ch...@163.com>
Reviewed-by: Alexey Serbin <al...@apache.org>
Reviewed-by: Yuqi Du <sh...@gmail.com>
Reviewed-by: Yingchun Lai <la...@apache.org>
---
M src/kudu/integration-tests/create-table-itest.cc
M src/kudu/master/catalog_manager.cc
2 files changed, 201 insertions(+), 2 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Yifan Zhang: Looks good to me, but someone else must approve
  Alexey Serbin: Looks good to me, but someone else must approve
  Yuqi Du: Looks good to me, but someone else must approve
  Yingchun Lai: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 24
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19571/6/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/6/src/kudu/master/catalog_manager.cc@6003
PS6, Line 6003:     if (policy.ts_num() < (nreplicas + 1) / 2) {
> It has been implemented in Line 6012.
'policy.ts_num()' returns the number of tservers that are available for placement[1], I mean we should also check the number of registered tservers[2]. I guess that if there are enough registered tservers to create all replicas of a table, to some extent it means that some tservers are just temporarily offline and there will eventually be enough available tservers to make the cluster healthy.

[1] https://github.com/apache/kudu/blob/f13f6580c0588d8596f76e8ad9e39feba7a13783/src/kudu/master/ts_manager.h#L92-L96
[2] https://github.com/apache/kudu/blob/f13f6580c0588d8596f76e8ad9e39feba7a13783/src/kudu/master/ts_manager.h#L88-L90



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 7
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Fri, 07 Apr 2023 03:38:36 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/19571/6/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/6/src/kudu/master/catalog_manager.cc@6003
PS6, Line 6003:     if (policy.ts_num() < (nreplicas + 1) / 2) {
nit: According to discussions in the JIRA ticket, we should also check for the total number of tablet servers registered with the catalog manager, to expose errors of a misconfigured Kudu cluster.


http://gerrit.cloudera.org:8080/#/c/19571/6/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/6/src/kudu/tools/kudu-tool-test.cc@9175
PS6, Line 9175: Restart the stopped tserver to bring the table to healthy.
nit: How about 'The table will become healthy once the unavailable tserver is brought back online'?


http://gerrit.cloudera.org:8080/#/c/19571/6/src/kudu/tools/kudu-tool-test.cc@9290
PS6, Line 9290: colum
nit: column



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 6
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 06 Apr 2023 09:35:28 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 7:

(14 comments)

http://gerrit.cloudera.org:8080/#/c/19571/7//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19571/7//COMMIT_MSG@9
PS7, Line 9: create
creating


http://gerrit.cloudera.org:8080/#/c/19571/7//COMMIT_MSG@10
PS7, Line 10: and will retry continuously
            : and always fail until the unavailable tablet servers becomes
            : healthy again.
What component will retry continuously?  The client, the system catalog, some other component, etc.?


http://gerrit.cloudera.org:8080/#/c/19571/7//COMMIT_MSG@14
PS7, Line 14: An already created tablet can still be on service even if
            : one of its 3 replicas become unavailable. So creating a
            : three-replicas table when there are only 2 tablet servers
            : can be supported.
This looks like just an example.  Maybe, you could make this comment expressing the essence of the idea behind the change:  having enough healthy tablet servers to place just a majority of replicas for a tablet?

Maybe, something like:

An under-replicated table is still available for reading and writing, so it's enough to place just a majority of replicas for each tablet at healthy tablet servers to make a newly created table ready to use.


http://gerrit.cloudera.org:8080/#/c/19571/7//COMMIT_MSG@21
PS7, Line 21: keeps
is kept


http://gerrit.cloudera.org:8080/#/c/19571/7//COMMIT_MSG@22
PS7, Line 22: create tablet without enough
            : healthy tservers can success
consider change to:

it's possible to create a tablet placing just a majority of replicas at healthy tablet servers


http://gerrit.cloudera.org:8080/#/c/19571/7//COMMIT_MSG@23
PS7, Line 23: And its status is under
            : replicating. Then this tablet can be be read and write normally.
consider changing to:

Even if the new tablet is created under-replicated, it's still available for read and write operations.


http://gerrit.cloudera.org:8080/#/c/19571/7/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/7/src/kudu/master/catalog_manager.cc@417
PS7, Line 417: enable_create_tablet_without_enough_healthy_tservers
How about naming this 'allow_creating_under_replicated_tables'?


http://gerrit.cloudera.org:8080/#/c/19571/7/src/kudu/master/catalog_manager.cc@418
PS7, Line 418: enough
What's "enough"?  It would be great to be more specific here, i.e. mention that there should be enough healthy tablet servers to place just the majority of tablet replicas.


http://gerrit.cloudera.org:8080/#/c/19571/7/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/7/src/kudu/tools/kudu-tool-test.cc@9176
PS7, Line 9176: No
nit: Not


http://gerrit.cloudera.org:8080/#/c/19571/7/src/kudu/tools/kudu-tool-test.cc@9186
PS7, Line 9186: replica
replicas


http://gerrit.cloudera.org:8080/#/c/19571/7/src/kudu/tools/kudu-tool-test.cc@9252
PS7, Line 9252: No
nit: Not


http://gerrit.cloudera.org:8080/#/c/19571/7/src/kudu/tools/kudu-tool-test.cc@9261
PS7, Line 9261: replica
replicas


http://gerrit.cloudera.org:8080/#/c/19571/7/src/kudu/tools/kudu-tool-test.cc@9263
PS7, Line 9263: heart beat
heartbeat


http://gerrit.cloudera.org:8080/#/c/19571/7/src/kudu/tools/kudu-tool-test.cc@9333
PS7, Line 9333: } // namespace tools
Please consider adding more test coverage for the following scenarios:
* a negative scenario: when there isn't enough tablet servers to place a majority of replicas for each tablet, a table cannot be created
* make sure both the positive and the negative scenarios works as expected for other replication factors, at least cover RF=1 and RF=5



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 7
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sun, 09 Apr 2023 04:58:01 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#3).

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................

KUDU-3452 Create tablet without enough healthy tservers

Currently, create a three-replicas table when there are not
enough tablet servers will fail and will retry continuously
and always fail until the unavailabe tserver becomes healthy
again.

An already created tablet can still be on service even if one
of its 3 replicas become unavailable. Create a three-replicas
table when only 2 tablet servers can be support.

This patch adds a new flag:
--enable_create_tablet_without_enough_healthy_tservers
to support this feature. The original logic keeps the same.
When this flag is set true, create tablet without enough
healthy tservers can success. And its status is under
replicating. Then this tablet can be be read and write normally.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/tools/kudu-tool-test.cc
3 files changed, 112 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/3
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 3
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 13:

(2 comments)

> Patch Set 12:
> 
> (7 comments)

http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc@9283
PS11, Line 9283:                   .Create();
> It will timeout. Because there are 5 tablet servers totally, and 3 tablet s
IMO.

only 2 tsevers available, your replication factor is 5. So this creating table should response fail to client rather than timeout.
timeout means it maybe ok(available), in fact, it not available, this table neither write nor read.

It's not reasonable, that means if all tservers are down, creating table request is also timeout.

Before this patch, number of enough available tservers is >= n(replication factor) (this table is 5). 
This patch make this enough condition relaxed, downgrade from n to (n+1)/2 , this table is 3,
very good.

If this kudu cluster don't satisfy the condition, creating table request should fail.


http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc@9396
PS11, Line 9396: _FATALS(c
> There are 5 creating table commands, but only 4 commands will enter the sta
IMO. The same above.

This patch's benefits are that kudu can tolerate part of tablet server failures.
If number of available tablet servers is not >= (replication_refactor + 1 ) / 2 , the creating table requests should fail.
because this incomplete table can provide neither write nor read availability.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 13
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Fri, 05 May 2023 09:50:37 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................


Patch Set 23:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/19571/23/src/kudu/integration-tests/create-table-itest.cc
File src/kudu/integration-tests/create-table-itest.cc:

http://gerrit.cloudera.org:8080/#/c/19571/23/src/kudu/integration-tests/create-table-itest.cc@703
PS23, Line 703:   {
              :     // Restart the first tablet server.
              :     NO_FATALS(cluster_->tablet_server(0)->Restart());
              :     // Wait the restarted tablet server to send a heartbeat and be registered in catalog manaager.
              :     SleepFor(MonoDelta::FromMilliseconds(kHeartbeatIntervalMs));
              :   }
              : 
              :   // Create a table with RF=5. It should succeed.
              :   ASSERT_OK(create_table_func(kFiveReplicaTableName, 5));
> Just curious: have you seen any flakiness in this combo when running the sc
Thank you for your comments. I have tested this case for many times, but have not see any flakiness.


http://gerrit.cloudera.org:8080/#/c/19571/23/src/kudu/integration-tests/create-table-itest.cc@740
PS23, Line 740: kudu::tools::RunKuduTool
> Wrap this call into ASSERT_OK()?
No, it can not. The cluster is not healthy at this moment. Some tables are under replicated. Please see line 749.


http://gerrit.cloudera.org:8080/#/c/19571/23/src/kudu/integration-tests/create-table-itest.cc@769
PS23, Line 769: kudu::tools::RunKuduTool
> Wrap this call into ASSERT_OK()?
It can not. For case adding one new tablet server, see line 756, the cluster will never become healthy, because the third tablet server is always stopped.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 23
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 13 May 2023 04:50:42 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................


Patch Set 23:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19571/23/src/kudu/integration-tests/create-table-itest.cc
File src/kudu/integration-tests/create-table-itest.cc:

http://gerrit.cloudera.org:8080/#/c/19571/23/src/kudu/integration-tests/create-table-itest.cc@703
PS23, Line 703:   {
              :     // Restart the first tablet server.
              :     NO_FATALS(cluster_->tablet_server(0)->Restart());
              :     // Wait the restarted tablet server to send a heartbeat and be registered in catalog manaager.
              :     SleepFor(MonoDelta::FromMilliseconds(kHeartbeatIntervalMs));
              :   }
              : 
              :   // Create a table with RF=5. It should succeed.
              :   ASSERT_OK(create_table_func(kFiveReplicaTableName, 5));
> What about using 'cluster_.WaitForTabletServerCount'? Maybe in some cases s
WaitForTabletServerCount() will get all registered tablet servers, not the available registered tablet servers.
I can not find a better way to make sure the restarted tablet server has been registered an available tablet server.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 23
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 15 May 2023 08:26:59 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#9).

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................

KUDU-3452 Create tablet without enough healthy tservers

Currently, creating a three-replicas table when there are
not enough tablet servers will fail and the system catalog
will retry continuously and always fail until the unavailable
tablet servers becomes healthy again.

An under-replicated table is still available for reading and
writing, so it's enough to place just a majority of replicas
for each tablet at healthy tablet servers to make a newly
created table ready to use.

This patch adds a new flag:
--allow_creating_under_replicated_tables to support this
feature. The original logic is kept the same. When this flag
is set true, it's possible to create a tablet placing just a
majority of replicas at healthy tablet servers. Even if the
new tablet is created under-replicated, it's still available
for read and write operations.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-tool-test.cc
2 files changed, 203 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/9
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 9
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 15: Code-Review+1

(1 comment)

LGTM.

http://gerrit.cloudera.org:8080/#/c/19571/14/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/14/src/kudu/master/catalog_manager.cc@6678
PS14, Line 6678: InvalidArgument
> I think InvalidArgument is better. Here are arguments checking for creating
ok



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 15
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 06 May 2023 04:16:48 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................


Patch Set 17:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19571/16/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/16/src/kudu/tools/kudu-tool-test.cc@9218
PS16, Line 9218: string& table_nam
> nit:
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 17
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 06 May 2023 10:50:38 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 8:

(15 comments)

http://gerrit.cloudera.org:8080/#/c/19571/7//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19571/7//COMMIT_MSG@9
PS7, Line 9: creati
> creating
Done


http://gerrit.cloudera.org:8080/#/c/19571/7//COMMIT_MSG@10
PS7, Line 10: ail and the system catalog
            : will retry continuously and always fail until the unavailable
            : tablet servers
> What component will retry continuously?  The client, the system catalog, so
The system catalog will retry continuously.


http://gerrit.cloudera.org:8080/#/c/19571/7//COMMIT_MSG@14
PS7, Line 14: An under-replicated table is still available for reading and
            : writing, so it's enough to place just a majority of replicas
            : for each tablet at healthy tablet servers to make a newly
            : created table rea
> This looks like just an example.  Maybe, you could make this comment expres
Done


http://gerrit.cloudera.org:8080/#/c/19571/7//COMMIT_MSG@21
PS7, Line 21: . Whe
> is kept
Done


http://gerrit.cloudera.org:8080/#/c/19571/7//COMMIT_MSG@22
PS7, Line 22: o create a tablet placing just a
            : majority of replicas at heal
> consider change to:
Done


http://gerrit.cloudera.org:8080/#/c/19571/7//COMMIT_MSG@23
PS7, Line 23: y tablet servers. Even if the
            : new tablet is created under-replicated, it's still available
> consider changing to:
Done


http://gerrit.cloudera.org:8080/#/c/19571/6/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/6/src/kudu/master/catalog_manager.cc@6003
PS6, Line 6003:     // of alive tservers, otherwise, it can not run.
> 'policy.ts_num()' returns the number of tservers that are available for pla
I got it. Thank you!


http://gerrit.cloudera.org:8080/#/c/19571/7/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/7/src/kudu/master/catalog_manager.cc@417
PS7, Line 417: allow_creating_under_replicated_tables, false,
> How about naming this 'allow_creating_under_replicated_tables'?
Done


http://gerrit.cloudera.org:8080/#/c/19571/7/src/kudu/master/catalog_manager.cc@418
PS7, Line 418: e are 
> What's "enough"?  It would be great to be more specific here, i.e. mention 
Done


http://gerrit.cloudera.org:8080/#/c/19571/7/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/7/src/kudu/tools/kudu-tool-test.cc@9176
PS7, Line 9176: :W
> nit: Not
Done


http://gerrit.cloudera.org:8080/#/c/19571/7/src/kudu/tools/kudu-tool-test.cc@9186
PS7, Line 9186: 
> replicas
Done


http://gerrit.cloudera.org:8080/#/c/19571/7/src/kudu/tools/kudu-tool-test.cc@9252
PS7, Line 9252: m_
> nit: Not
Done


http://gerrit.cloudera.org:8080/#/c/19571/7/src/kudu/tools/kudu-tool-test.cc@9261
PS7, Line 9261:  of one
> replicas
Done


http://gerrit.cloudera.org:8080/#/c/19571/7/src/kudu/tools/kudu-tool-test.cc@9263
PS7, Line 9263: 
> heartbeat
Done


http://gerrit.cloudera.org:8080/#/c/19571/7/src/kudu/tools/kudu-tool-test.cc@9333
PS7, Line 9333:     ASSERT_EQ(3, doc["table_summaries"].Size());
> Please consider adding more test coverage for the following scenarios:
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 8
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 15 Apr 2023 08:25:31 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 10:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/master/catalog_manager.cc@418
PS10, Line 418: enough
> nit: not enough
Yingchun, to me it looks correct if you read the whole sentence: ... enough healthy tablet servers ... to place just the majority of tablet replicas


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/master/catalog_manager.cc@6663
PS10, Line 6663: larger
nit: greater


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9196
PS10, Line 9196: const
nit: this could be constexpr as well


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9245
PS10, Line 9245:   SleepFor(MonoDelta::FromMilliseconds(6000));
> It would be better to check the 3 tservers are actually shutdown successfll
IIUC, ExternalDaemon::Shutdown() actually awaits for the process to be stopped/exited, so I'm not sure I understand why it's necessary to wait here.  Probably, the idea is to wait until catalog manager (i.e. master) sees just 2 tablet servers alive instead of 5, as Yingchun mentioned?  If so, you can wait for some state of the mini-cluster (or a genetic condition) using ASSERT_EVENTUALLY().


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9247
PS10, Line 9247: no
nit: not


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9267
PS10, Line 9267: Restart 2
For a tablet with RF=5, for a positive scenario we are interested to see if the system allows to create a table when only 3 healthy tablet servers are present.  Here we have 4 instead of 3 if stating back 2 tablet servers.


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9272
PS10, Line 9272:   SleepFor(MonoDelta::FromMilliseconds(4000));
> Same, check the 4 tservers are actually alive.
+1


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9296
PS10, Line 9296: alter table
Altering the replication factor is not a 'regular' sort of ALTER TABLE.  Could you also add a 'regular' ALTER TABLE operation, e.g., adding a column for a table that's currently under-replicated?  Does it fail (timeout, etc.) or succeed?

Thanks!


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9299
PS10, Line 9299: will
nit: will be

Also, what table?  At this point there are at least two tables.  So, is kOneReplicaTableName table expected to be under-replicated?


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9325
PS10, Line 9325: out.clear();
nit: it's much cleaner to use variables local to the scope instead of relying on the out-of-the scope variables -- consider declaring a new 'out' variable local to this scope



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 10
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 24 Apr 2023 22:18:08 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/19571/6/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/6/src/kudu/master/catalog_manager.cc@6003
PS6, Line 6003:     if (policy.ts_num() < (nreplicas + 1) / 2) {
> nit: According to discussions in the JIRA ticket, we should also check for 
It has been implemented in Line 6012.


http://gerrit.cloudera.org:8080/#/c/19571/6/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/6/src/kudu/tools/kudu-tool-test.cc@9175
PS6, Line 9175: Restart the stopped tserver to bring the table to healthy.
> nit: How about 'The table will become healthy once the unavailable tserver 
Done


http://gerrit.cloudera.org:8080/#/c/19571/6/src/kudu/tools/kudu-tool-test.cc@9290
PS6, Line 9290: colum
> nit: column
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 6
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 06 Apr 2023 13:51:54 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................


Patch Set 20:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/master/catalog_manager.cc@6003
PS17, Line 6003:     // The Raft protocol requires at least (replication_factor / 2) + 1 live replicas
               :     // to form a quorum.  Since Kudu places at most 
> nit: consider updating this explanatory comment
Done


http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/master/catalog_manager.cc@6005
PS17, Line 6005: ary to have at leas
> Please consider using consensus::MajoritySize(num_replicas) here instead.  
Done


http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/master/catalog_manager.cc@6006
PS17, Line 6006: to serve read and write requests.
               :     if (policy.ts_num() < consensus::MajoritySize(nreplicas)) {
               :       return Status::InvalidArgument(
> nit: consider updating to clarify
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 20
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Tue, 09 May 2023 09:03:53 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#22).

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................

KUDU-3452 Allow creating tablets under replicated tables

Currently, creating a three-replicas table when there are
not less than 3 healthy tablet servers will fail and the system
catalog will retry continuously and always fail until the
unavailable tablet servers becomes healthy again.

An under-replicated table is still available for reading and
writing, so it's enough to place just a majority of replicas
for each tablet at healthy tablet servers to make a newly
created table ready to use.

This patch adds a new flag:
--allow_creating_under_replicated_tables to support this
feature. The original logic is kept the same. When this flag
is set true, it's possible to create a tablet placing just a
majority of replicas at healthy tablet servers. Even if the
new tablet is created under-replicated, it's still available
for read and write operations.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/integration-tests/create-table-itest.cc
M src/kudu/master/catalog_manager.cc
2 files changed, 198 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/22
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 22
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................


Patch Set 23: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 23
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 15 May 2023 09:55:08 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 10:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/master/catalog_manager.cc@418
PS10, Line 418: enough
nit: not enough


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/master/catalog_manager.cc@5999
PS10, Line 5999: enough
What does "enough" mean, does 1 out of RF = 3 is enough? How about to describe  "all requested replicas number of" or sth. like that?

Besides, the logic below is to update "nreplicas" but not "create a tablet", so how about update the comments to "Update nreplicas if needed." or sth.


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/master/catalog_manager.cc@6008
PS10, Line 6008: (
Remove duplicate brackets.


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9227
PS10, Line 9227:     ASSERT_EQ(s.ToString(),
nit: Check the type of Status fastly, i.e. :

 ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString();


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9234
PS10, Line 9234:   // Ksck will be OK.
Also check the table 'kOverRegistedTSTableName' is not existed in the cluster?


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9245
PS10, Line 9245:   SleepFor(MonoDelta::FromMilliseconds(6000));
It would be better to check the 3 tservers are actually shutdown successflly, maybe ListTabletServers got 3 alive tservers.


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9264
PS10, Line 9264:     ASSERT_EQ(s.ToString(), "Timed out: Timed out waiting for Table Creation");
nit: Check the type of Status fastly, i.e. :

 ASSERT_TRUE(s.IsTimedOut()) << s.ToString();


http://gerrit.cloudera.org:8080/#/c/19571/10/src/kudu/tools/kudu-tool-test.cc@9272
PS10, Line 9272:   SleepFor(MonoDelta::FromMilliseconds(4000));
Same, check the 4 tservers are actually alive.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 10
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 24 Apr 2023 03:03:56 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 14:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc@9283
PS11, Line 9283:     // Wait the restarted tablet server to send 
> IMO.
Done


http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc@9396
PS11, Line 9396: 
> IMO. The same above.
Good advice. Thank you.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 14
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 06 May 2023 02:58:34 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 15:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/19571/15//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19571/15//COMMIT_MSG@7
PS15, Line 7: enough
What does "enough" mean, does 1 out of RF = 3 is enough? How about to describe  "all requested replicas number of" or sth. like that?


http://gerrit.cloudera.org:8080/#/c/19571/15//COMMIT_MSG@10
PS15, Line 10: enough
Same.


http://gerrit.cloudera.org:8080/#/c/19571/15/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/15/src/kudu/tools/kudu-tool-test.cc@9205
PS15, Line 9205:   opts.extra_master_flags.emplace_back("--catalog_manager_check_ts_count_for_create_table=false");
nit: they can be simplified by:

 cluster_opts_.extra_master_flags = {
   "--catalog_manager_check_ts_count_for_alter_table=false",
   Substitute("--tserver_unresponsive_timeout_ms=$0", kTSUnresponsiveTimeoutMs)),
   ...
 }


http://gerrit.cloudera.org:8080/#/c/19571/15/src/kudu/tools/kudu-tool-test.cc@9237
PS15, Line 9237:     ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString();
nit: because many place would return InvalidArgument, so also check the error meesage is as expect, it contains some key words like "not enough registered tablet servers to create a table with the requested replication"


http://gerrit.cloudera.org:8080/#/c/19571/15/src/kudu/tools/kudu-tool-test.cc@9239
PS15, Line 9239:     RunTool(Substitute("table list $0 --tables=$1", master_addr, kOverRegistedTSTableName),
How about use client to OpenTable and then check the return status is NotFound? CLI tool is too heavy.


http://gerrit.cloudera.org:8080/#/c/19571/15/src/kudu/tools/kudu-tool-test.cc@9244
PS15, Line 9244:   // Ksck will be OK.
Why check ksck?


http://gerrit.cloudera.org:8080/#/c/19571/15/src/kudu/tools/kudu-tool-test.cc@9262
PS15, Line 9262:     shared_ptr<KuduClient> client;
The 2nd time this logic appears, how about encapsulate it as a function?


http://gerrit.cloudera.org:8080/#/c/19571/15/src/kudu/tools/kudu-tool-test.cc@9277
PS15, Line 9277:     ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString();
Same, check the error meesage contains some key words.


http://gerrit.cloudera.org:8080/#/c/19571/15/src/kudu/tools/kudu-tool-test.cc@9309
PS15, Line 9309:     NO_FATALS(workload.Start());
The TestWorkload will start write threads to write data into the table, I don't think it is what you need, right?
How about using the encapsulate function above?


http://gerrit.cloudera.org:8080/#/c/19571/15/src/kudu/tools/kudu-tool-test.cc@9312
PS15, Line 9312: insert some data
Is inserting data necessary? If the table status is health, it's not needed to do that.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 15
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 06 May 2023 07:42:50 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................


Patch Set 16:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/19571/15//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19571/15//COMMIT_MSG@7
PS15, Line 7:  under
> What does "enough" mean, does 1 out of RF = 3 is enough? How about to descr
Done


http://gerrit.cloudera.org:8080/#/c/19571/15//COMMIT_MSG@10
PS15, Line 10: less t
> Same.
Done


http://gerrit.cloudera.org:8080/#/c/19571/15/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/15/src/kudu/tools/kudu-tool-test.cc@9205
PS15, Line 9205:   opts.extra_master_flags = {
> nit: they can be simplified by:
Done


http://gerrit.cloudera.org:8080/#/c/19571/15/src/kudu/tools/kudu-tool-test.cc@9237
PS15, Line 9237:     Status s = create_table_func(kOverRegistedTSTableName, 6);
> nit: because many place would return InvalidArgument, so also check the err
Done


http://gerrit.cloudera.org:8080/#/c/19571/15/src/kudu/tools/kudu-tool-test.cc@9239
PS15, Line 9239:     ASSERT_STR_CONTAINS(s.ToString(), "not enough registered tablet servers to");
> How about use client to OpenTable and then check the return status is NotFo
Done


http://gerrit.cloudera.org:8080/#/c/19571/15/src/kudu/tools/kudu-tool-test.cc@9244
PS15, Line 9244: 
> Why check ksck?
No need. Remove it.


http://gerrit.cloudera.org:8080/#/c/19571/15/src/kudu/tools/kudu-tool-test.cc@9277
PS15, Line 9277:     // Wait the restarted tablet server to send a heartbeat and be registered in catalog manaager.
> Same, check the error meesage contains some key words.
Done


http://gerrit.cloudera.org:8080/#/c/19571/15/src/kudu/tools/kudu-tool-test.cc@9309
PS15, Line 9309:       } else {
> The TestWorkload will start write threads to write data into the table, I d
Done


http://gerrit.cloudera.org:8080/#/c/19571/15/src/kudu/tools/kudu-tool-test.cc@9312
PS15, Line 9312: 
> Is inserting data necessary? If the table status is health, it's not needed
Done


http://gerrit.cloudera.org:8080/#/c/19571/15/src/kudu/tools/kudu-tool-test.cc@9338
PS15, Line 9338:   }
> If moving the test to client-test.cc, you are able to access the master sta
InternalMiniCluster can not set flags like --catalog_manager_check_ts_count_for_create_table=false.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 16
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 06 May 2023 09:57:01 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 6:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/19571/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19571/4//COMMIT_MSG@11
PS4, Line 11: unavailabl
> nit: unavailable
Done


http://gerrit.cloudera.org:8080/#/c/19571/4//COMMIT_MSG@11
PS4, Line 11:  tablet servers
> tservers become
Done


http://gerrit.cloudera.org:8080/#/c/19571/4//COMMIT_MSG@15
PS4, Line 15: le. So
> So creating
Done


http://gerrit.cloudera.org:8080/#/c/19571/4//COMMIT_MSG@16
PS4, Line 16: replicas ta
> when there are only 2 healthy tservers
Done


http://gerrit.cloudera.org:8080/#/c/19571/4//COMMIT_MSG@16
PS4, Line 16:  2 tabl
> nit: supported
Done


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc@5933
PS3, Line 5933:       // NOTE: if we fail to select replicas on the first pass (due to
              :       // insufficient Tablet Servers being online), we will still try
              :       // again unless the tablet/table creation is cancelled.
              :       RETURN_NOT_OK_PREPEND(SelectReplicasForTablet(policy, tablet.get()),
              :                             Substitute("error selecting replicas for tablet $0",
              :                                        tablet->id()));
              :     }
              :   }
              : 
              :   // Update the sys catalog with the new set of tablets/metadata.
              :   {
              :     SysCatalogTable::Actions actions;
              :     actions.tablets_to_add = deferred.tablets_to_add;
              :     actions.tablets_to_update = deferred.tablets_to_update;
              :     RETURN_NOT_OK_PREPEND(sys_catalog_->Write(std::move(actions)),
              :                           "error persisting updated tablet metadata");
              :   }
              : 
              :   // Ex
> Done
Done


http://gerrit.cloudera.org:8080/#/c/19571/3/src/kudu/master/catalog_manager.cc@6006
PS3, Line 6006:                                                 "alive tablet servers",
> Done
Done


http://gerrit.cloudera.org:8080/#/c/19571/4/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/4/src/kudu/master/catalog_manager.cc@5988
PS4, Line 5988:                                                T
> nit:  unexpected extra space ?
Done


http://gerrit.cloudera.org:8080/#/c/19571/4/src/kudu/master/catalog_manager.cc@6001
PS4, Line 6001: replication_fa
> nit: replication_factor
Done


http://gerrit.cloudera.org:8080/#/c/19571/4/src/kudu/master/catalog_manager.cc@6002
PS4, Line 6002: of
> Otherwise,
Done


http://gerrit.cloudera.org:8080/#/c/19571/4/src/kudu/master/catalog_manager.cc@6004
PS4, Line 6004: protocol
> nit: protocol
Done


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc@9213
PS2, Line 9213: name(kTableName2
> So?SleepFor (2 * FLAGS_tserver_unresponsive_timeout_ms ) ?  And above( L920
tserver_unresponsive_timeout_ms is a server flag. CLI can not get the value directly.


http://gerrit.cloudera.org:8080/#/c/19571/4/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/4/src/kudu/tools/kudu-tool-test.cc@9239
PS4, Line 9239: 
> It's no need restart all kudu cluster.  The case should make sure 'cluster_
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 6
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Tue, 28 Mar 2023 03:04:20 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#6).

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................

KUDU-3452 Create tablet without enough healthy tservers

Currently, create a three-replicas table when there are not
enough tablet servers will fail and will retry continuously
and always fail until the unavailable tablet servers becomes
healthy again.

An already created tablet can still be on service even if
one of its 3 replicas become unavailable. So creating a
three-replicas table when there are only 2 tablet servers
can be supported.

This patch adds a new flag:
--enable_create_tablet_without_enough_healthy_tservers
to support this feature. The original logic keeps the same.
When this flag is set true, create tablet without enough
healthy tservers can success. And its status is under
replicating. Then this tablet can be be read and write normally.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-tool-test.cc
2 files changed, 181 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/6
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 6
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] [KUDU-3452] Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#2).

Change subject: [KUDU-3452] Create tablet without enough healthy tservers
......................................................................

[KUDU-3452] Create tablet without enough healthy tservers

Currently, create a three-replicas table when there are not
enough tablet servers will fail and will retry continuously
and always fail until the unavailabe tserver becomes healthy
again.

An already created tablet can still be on service even if one
of its 3 replicas become unavailable. Create a three-replicas
table when only 2 tablet servers can be support.

This patch adds a new flag:
--enable_create_tablet_without_enough_healthy_tservers
to support this feature. The original logic keeps the same.
When this flag is set true, create tablet without enough
healthy tservers can success. And its status is under
replicating. Then this tablet can be be read and write normally.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-tool-test.cc
2 files changed, 60 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/2
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 2
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#7).

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................

KUDU-3452 Create tablet without enough healthy tservers

Currently, create a three-replicas table when there are not
enough tablet servers will fail and will retry continuously
and always fail until the unavailable tablet servers becomes
healthy again.

An already created tablet can still be on service even if
one of its 3 replicas become unavailable. So creating a
three-replicas table when there are only 2 tablet servers
can be supported.

This patch adds a new flag:
--enable_create_tablet_without_enough_healthy_tservers
to support this feature. The original logic keeps the same.
When this flag is set true, create tablet without enough
healthy tservers can success. And its status is under
replicating. Then this tablet can be be read and write normally.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-tool-test.cc
2 files changed, 181 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/7
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 7
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#4).

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................

KUDU-3452 Create tablet without enough healthy tservers

Currently, create a three-replicas table when there are not
enough tablet servers will fail and will retry continuously
and always fail until the unavailabe tserver becomes healthy
again.

An already created tablet can still be on service even if one
of its 3 replicas become unavailable. Create a three-replicas
table when only 2 tablet servers can be support.

This patch adds a new flag:
--enable_create_tablet_without_enough_healthy_tservers
to support this feature. The original logic keeps the same.
When this flag is set true, create tablet without enough
healthy tservers can success. And its status is under
replicating. Then this tablet can be be read and write normally.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-tool-test.cc
2 files changed, 97 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/4
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 4
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................


Patch Set 24: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19571/23/src/kudu/integration-tests/create-table-itest.cc
File src/kudu/integration-tests/create-table-itest.cc:

http://gerrit.cloudera.org:8080/#/c/19571/23/src/kudu/integration-tests/create-table-itest.cc@769
PS23, Line 769: kudu::tools::RunKuduTool
> It can not. For case adding one new tablet server, see line 756, the cluste
It would be great if you could add a comment about that and wrap those calls into ignore_result.

This patch is already pushed, but maybe next time it would be nice add those in such cases.

Thanks!



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 24
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 15 May 2023 16:49:14 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................


Patch Set 23: Code-Review+1

(3 comments)

Overall looks good to me.  Almost there!

Just a few nits in the test scenario.

http://gerrit.cloudera.org:8080/#/c/19571/23/src/kudu/integration-tests/create-table-itest.cc
File src/kudu/integration-tests/create-table-itest.cc:

http://gerrit.cloudera.org:8080/#/c/19571/23/src/kudu/integration-tests/create-table-itest.cc@703
PS23, Line 703:   {
              :     // Restart the first tablet server.
              :     NO_FATALS(cluster_->tablet_server(0)->Restart());
              :     // Wait the restarted tablet server to send a heartbeat and be registered in catalog manaager.
              :     SleepFor(MonoDelta::FromMilliseconds(kHeartbeatIntervalMs));
              :   }
              : 
              :   // Create a table with RF=5. It should succeed.
              :   ASSERT_OK(create_table_func(kFiveReplicaTableName, 5));
Just curious: have you seen any flakiness in this combo when running the scenario with --stress_cpu_threads=16?

If yes, maybe it would be a more stable if instead of waiting for some specified heartbeat interval the create_table_func would be put into ASSERT_EVENTUALLY() clause?

Same for the similar wait-and-then-create-table sub-scenarios below.


http://gerrit.cloudera.org:8080/#/c/19571/23/src/kudu/integration-tests/create-table-itest.cc@740
PS23, Line 740: kudu::tools::RunKuduTool
Wrap this call into ASSERT_OK()?


http://gerrit.cloudera.org:8080/#/c/19571/23/src/kudu/integration-tests/create-table-itest.cc@769
PS23, Line 769: kudu::tools::RunKuduTool
Wrap this call into ASSERT_OK()?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 23
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 13 May 2023 03:57:51 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#12).

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................

KUDU-3452 Create tablet without enough healthy tservers

Currently, creating a three-replicas table when there are
not enough tablet servers will fail and the system catalog
will retry continuously and always fail until the unavailable
tablet servers becomes healthy again.

An under-replicated table is still available for reading and
writing, so it's enough to place just a majority of replicas
for each tablet at healthy tablet servers to make a newly
created table ready to use.

This patch adds a new flag:
--allow_creating_under_replicated_tables to support this
feature. The original logic is kept the same. When this flag
is set true, it's possible to create a tablet placing just a
majority of replicas at healthy tablet servers. Even if the
new tablet is created under-replicated, it's still available
for read and write operations.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/client/client.h
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-tool-test.cc
3 files changed, 283 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/12
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 12
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#17).

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................

KUDU-3452 Allow creating tablets under replicated tables

Currently, creating a three-replicas table when there are
not less than 3 healthy tablet servers will fail and the system
catalog will retry continuously and always fail until the
unavailable tablet servers becomes healthy again.

An under-replicated table is still available for reading and
writing, so it's enough to place just a majority of replicas
for each tablet at healthy tablet servers to make a newly
created table ready to use.

This patch adds a new flag:
--allow_creating_under_replicated_tables to support this
feature. The original logic is kept the same. When this flag
is set true, it's possible to create a tablet placing just a
majority of replicas at healthy tablet servers. Even if the
new tablet is created under-replicated, it's still available
for read and write operations.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/client/client.h
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-tool-test.cc
3 files changed, 215 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/17
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 17
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#14).

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................

KUDU-3452 Create tablet without enough healthy tservers

Currently, creating a three-replicas table when there are
not enough tablet servers will fail and the system catalog
will retry continuously and always fail until the unavailable
tablet servers becomes healthy again.

An under-replicated table is still available for reading and
writing, so it's enough to place just a majority of replicas
for each tablet at healthy tablet servers to make a newly
created table ready to use.

This patch adds a new flag:
--allow_creating_under_replicated_tables to support this
feature. The original logic is kept the same. When this flag
is set true, it's possible to create a tablet placing just a
majority of replicas at healthy tablet servers. Even if the
new tablet is created under-replicated, it's still available
for read and write operations.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/client/client.h
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-tool-test.cc
3 files changed, 253 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/14
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 14
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................


Patch Set 16:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19571/16/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/16/src/kudu/tools/kudu-tool-test.cc@9218
PS16, Line 9218: string&table_name
nit:

> string& table_name



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 16
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 06 May 2023 10:13:12 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19571

to look at the new patch set (#16).

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................

KUDU-3452 Allow creating tablets under replicated tables

Currently, creating a three-replicas table when there are
not less than 3 healthy tablet servers will fail and the system
catalog will retry continuously and always fail until the
unavailable tablet servers becomes healthy again.

An under-replicated table is still available for reading and
writing, so it's enough to place just a majority of replicas
for each tablet at healthy tablet servers to make a newly
created table ready to use.

This patch adds a new flag:
--allow_creating_under_replicated_tables to support this
feature. The original logic is kept the same. When this flag
is set true, it's possible to create a tablet placing just a
majority of replicas at healthy tablet servers. Even if the
new tablet is created under-replicated, it's still available
for read and write operations.

Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
---
M src/kudu/client/client.h
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-tool-test.cc
3 files changed, 215 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/19571/16
-- 
To view, visit http://gerrit.cloudera.org:8080/19571
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 16
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3452 Create tablet without enough healthy tservers

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 11:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/19571/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19571/4//COMMIT_MSG@10
PS4, Line 10: ugh tablet ser
> tservers
Done


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc@9213
PS2, Line 9213: 
> tserver_unresponsive_timeout_ms is a server flag. CLI can not get the value
At this test case, it's deterministic because you have set it at L9184.
(  // Set heart beat timeout 1s.  opts.extra_master_flags.emplace_back("--tserver_unresponsive_timeout_ms=1000"); ),

For reference only.


http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc@9189
PS11, Line 9189: INSTANTIATE_TEST_SUITE_P
nit: add an empty line


http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc@9283
PS11, Line 9283:     ASSERT_TRUE(s.IsTimedOut()) << s.ToString();
The status is deterministic, I think it should be not timeout. What do you think?


http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc@9387
PS11, Line 9387: table
nit: All tables


http://gerrit.cloudera.org:8080/#/c/19571/11/src/kudu/tools/kudu-tool-test.cc@9396
PS11, Line 9396: ASSERT_EQ
I don't understand  the number 4.
I have found 3 kFiveReplicaTableName, kOneReplicaTableName, kNotEnoughReplicasTableName , will be ok.

But this table 'kOverRegistedTSTableName', 2 creating tables, fail reasons:

1: replication_factor > number of (available)tservers.

2: At this moment. there are not enough available tservers(only 2 alive, this table need at least 3), so I think tablets of this table should  be gced, it should not pending to wait tservers recover.

What do you think?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 11
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 04 May 2023 10:11:36 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3452 Allow creating tablets under replicated tables

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Allow creating tablets under replicated tables
......................................................................


Patch Set 22:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/19571/22/src/kudu/integration-tests/create-table-itest.cc
File src/kudu/integration-tests/create-table-itest.cc:

http://gerrit.cloudera.org:8080/#/c/19571/22/src/kudu/integration-tests/create-table-itest.cc@647
PS22, Line 647: constexpr int kTSUnresponsiveTimeoutMs = 2000;
              :   constexpr int kHeartbeatIntervalMs = 3000;
Why is the heartbeat interval even longer than tserver unresponsive interval? In that case, tservers could be considered unavailable between heartbeats.


http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/master/catalog_manager.cc@6664
PS17, Line 6664:   }
               : 
> Done
Seems this comment was not resolved in patch set 22.


http://gerrit.cloudera.org:8080/#/c/19571/17/src/kudu/master/catalog_manager.cc@6677
PS17, Line 6677: , MasterErrorPB::REPLI
> Done
ditto.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 22
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 11 May 2023 16:39:48 +0000
Gerrit-HasComments: Yes