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 <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <[email protected]> Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Reviewer: Yuqi Du <[email protected]> Gerrit-Comment-Date: Tue, 09 May 2023 00:26:03 +0000 Gerrit-HasComments: Yes
