Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17995 )
Change subject: [master] KUDU-3311 Allow to start with diff num of masters ...................................................................... Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/17995/4/src/kudu/integration-tests/master_replication-itest.cc File src/kudu/integration-tests/master_replication-itest.cc: http://gerrit.cloudera.org:8080/#/c/17995/4/src/kudu/integration-tests/master_replication-itest.cc@322 PS4, Line 322: ASSERT_OK(s); Wouldn't it be better to keep this test case negative by adding one more master? It seems its motivation was to test that changing master peer sets fails the startup. You could add another case where you add only one master that passes. http://gerrit.cloudera.org:8080/#/c/17995/4/src/kudu/master/master_options-test.cc File src/kudu/master/master_options-test.cc: http://gerrit.cloudera.org:8080/#/c/17995/4/src/kudu/master/master_options-test.cc@189 PS4, Line 189: // result in master bring up error. This comment suggests there should be a failure. Maybe add one more master to keep this failure and add a new test case where you add only one? http://gerrit.cloudera.org:8080/#/c/17995/4/src/kudu/master/master_options-test.cc@198 PS4, Line 198: an nit: extra word -- To view, visit http://gerrit.cloudera.org:8080/17995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I39aeee2f52a55a8c29770f748895d38c9adff8a2 Gerrit-Change-Number: 17995 Gerrit-PatchSet: 4 Gerrit-Owner: Anonymous Coward <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 04 Nov 2021 17:12:59 +0000 Gerrit-HasComments: Yes
