Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8280 )
Change subject: mini-cluster: support parallel multi-master clusters ...................................................................... Patch Set 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/8280/7/src/kudu/integration-tests/master_migration-itest.cc File src/kudu/integration-tests/master_migration-itest.cc: http://gerrit.cloudera.org:8080/#/c/8280/7/src/kudu/integration-tests/master_migration-itest.cc@89 PS7, Line 89: SO_REUSPORT SO_REUSEPORT http://gerrit.cloudera.org:8080/#/c/8280/7/src/kudu/integration-tests/registration-test.cc File src/kudu/integration-tests/registration-test.cc: http://gerrit.cloudera.org:8080/#/c/8280/7/src/kudu/integration-tests/registration-test.cc@272 PS7, Line 272: ASSERT_OK(cluster_->mini_master() : ->master() : ->WaitUntilCatalogManagerIsLeaderAndReadyForTests(MonoDelta::FromSeconds(10))); Hmm, the fact that you had to add this means the patch has changed some semantics. My guess is that InternalMiniCluster::Start used to do this for single masters, and when you unified its single/master code paths, this got dropped. Could you restore it there? Alternatively, if you do want to make these semantics more consistent and clear, I would recommend taking this further. ExternalMiniCluster provides no guarantees about the state of the catalogs after starting a cluster, regardless of the number of masters. I think that's how InternalMiniCluster should behave too; any tests that need the catalog to be initialized, or for a leader master to be elected should wait for that explicitly. But I understand if you feel like that's out of scope for this patch, in which case my request is that you restore the old semantics as I wrote above. -- To view, visit http://gerrit.cloudera.org:8080/8280 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0b0ff7bfc179d8fdb1ed306d1bbd12acddeb060c Gerrit-Change-Number: 8280 Gerrit-PatchSet: 7 Gerrit-Owner: Dan Burkert <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Thu, 03 May 2018 18:40:43 +0000 Gerrit-HasComments: Yes
