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 <[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: Sun, 09 Apr 2023 04:58:01 +0000 Gerrit-HasComments: Yes
