From e7655bbf807712300ac7f5d7aaf5234305d02906 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 19 Oct 2016 15:04:58 -0700 Subject: [PATCH] Remove pluggability of ElectMasterService (#21031) This change makes the ElectMasterService local to ZenDiscovery, no longer created by guice, and thus also removes the ability for plugins to customize. This extension point is no longer used by anything. --- .../common/settings/ClusterSettings.java | 1 - .../discovery/DiscoveryModule.java | 24 ------------------- .../discovery/zen/ElectMasterService.java | 3 +-- .../discovery/zen/ZenDiscovery.java | 4 ++-- .../zen/ping/unicast/UnicastZenPing.java | 6 ++--- .../discovery/DiscoveryModuleTests.java | 17 ------------- .../zen/ElectMasterServiceTests.java | 2 +- .../discovery/zen/ZenDiscoveryUnitTests.java | 4 +--- .../zen/ping/unicast/UnicastZenPingTests.java | 10 ++++---- 9 files changed, 11 insertions(+), 60 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index b5a0564174a..522ba96e844 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -328,7 +328,6 @@ public final class ClusterSettings extends AbstractScopedSettings { NodeEnvironment.NODE_ID_SEED_SETTING, DiscoverySettings.INITIAL_STATE_TIMEOUT_SETTING, DiscoveryModule.DISCOVERY_TYPE_SETTING, - DiscoveryModule.ZEN_MASTER_SERVICE_TYPE_SETTING, FaultDetection.PING_RETRIES_SETTING, FaultDetection.PING_TIMEOUT_SETTING, FaultDetection.REGISTER_CONNECTION_LISTENER_SETTING, diff --git a/core/src/main/java/org/elasticsearch/discovery/DiscoveryModule.java b/core/src/main/java/org/elasticsearch/discovery/DiscoveryModule.java index 1e41204d64a..28a17ef22b7 100644 --- a/core/src/main/java/org/elasticsearch/discovery/DiscoveryModule.java +++ b/core/src/main/java/org/elasticsearch/discovery/DiscoveryModule.java @@ -47,20 +47,16 @@ public class DiscoveryModule extends AbstractModule { public static final Setting DISCOVERY_TYPE_SETTING = new Setting<>("discovery.type", "zen", Function.identity(), Property.NodeScope); - public static final Setting ZEN_MASTER_SERVICE_TYPE_SETTING = - new Setting<>("discovery.zen.masterservice.type", "zen", Function.identity(), Property.NodeScope); private final Settings settings; private final Map>> unicastHostProviders = new HashMap<>(); private final ExtensionPoint.ClassSet zenPings = new ExtensionPoint.ClassSet<>("zen_ping", ZenPing.class); private final Map> discoveryTypes = new HashMap<>(); - private final Map> masterServiceType = new HashMap<>(); public DiscoveryModule(Settings settings) { this.settings = settings; addDiscoveryType("none", NoneDiscovery.class); addDiscoveryType("zen", ZenDiscovery.class); - addElectMasterService("zen", ElectMasterService.class); } /** @@ -88,16 +84,6 @@ public class DiscoveryModule extends AbstractModule { discoveryTypes.put(type, clazz); } - /** - * Adds a custom zen master service type. - */ - public void addElectMasterService(String type, Class masterService) { - if (masterServiceType.containsKey(type)) { - throw new IllegalArgumentException("master service type [" + type + "] is already registered"); - } - this.masterServiceType.put(type, masterService); - } - public void addZenPing(Class clazz) { zenPings.registerExtension(clazz); } @@ -111,16 +97,6 @@ public class DiscoveryModule extends AbstractModule { } if (discoveryType.equals("none") == false) { - String masterServiceTypeKey = ZEN_MASTER_SERVICE_TYPE_SETTING.get(settings); - final Class masterService = masterServiceType.get(masterServiceTypeKey); - if (masterService == null) { - throw new IllegalArgumentException("Unknown master service type [" + masterServiceTypeKey + "]"); - } - if (masterService == ElectMasterService.class) { - bind(ElectMasterService.class).asEagerSingleton(); - } else { - bind(ElectMasterService.class).to(masterService).asEagerSingleton(); - } bind(ZenPingService.class).asEagerSingleton(); Multibinder unicastHostsProviderMultibinder = Multibinder.newSetBinder(binder(), UnicastHostsProvider.class); for (Class unicastHostProvider : diff --git a/core/src/main/java/org/elasticsearch/discovery/zen/ElectMasterService.java b/core/src/main/java/org/elasticsearch/discovery/zen/ElectMasterService.java index 26029add7a4..7116597bdaf 100644 --- a/core/src/main/java/org/elasticsearch/discovery/zen/ElectMasterService.java +++ b/core/src/main/java/org/elasticsearch/discovery/zen/ElectMasterService.java @@ -98,7 +98,6 @@ public class ElectMasterService extends AbstractComponent { } } - @Inject public ElectMasterService(Settings settings) { super(settings); this.minimumMasterNodes = DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.get(settings); @@ -175,7 +174,7 @@ public class ElectMasterService extends AbstractComponent { * Returns the given nodes sorted by likelihood of being elected as master, most likely first. * Non-master nodes are not removed but are rather put in the end */ - public List sortByMasterLikelihood(Iterable nodes) { + public static List sortByMasterLikelihood(Iterable nodes) { ArrayList sortedNodes = CollectionUtils.iterableAsArrayList(nodes); CollectionUtil.introSort(sortedNodes, ElectMasterService::compareNodes); return sortedNodes; diff --git a/core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java b/core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java index 833349e9d9a..b295f07c80a 100644 --- a/core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java +++ b/core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java @@ -147,14 +147,14 @@ public class ZenDiscovery extends AbstractLifecycleComponent implements Discover @Inject public ZenDiscovery(Settings settings, ThreadPool threadPool, TransportService transportService, final ClusterService clusterService, ClusterSettings clusterSettings, - ZenPingService pingService, ElectMasterService electMasterService) { + ZenPingService pingService) { super(settings); this.clusterService = clusterService; this.clusterName = clusterService.getClusterName(); this.transportService = transportService; this.discoverySettings = new DiscoverySettings(settings, clusterSettings); this.pingService = pingService; - this.electMaster = electMasterService; + this.electMaster = new ElectMasterService(settings); this.pingTimeout = PING_TIMEOUT_SETTING.get(settings); this.joinTimeout = JOIN_TIMEOUT_SETTING.get(settings); diff --git a/core/src/main/java/org/elasticsearch/discovery/zen/ping/unicast/UnicastZenPing.java b/core/src/main/java/org/elasticsearch/discovery/zen/ping/unicast/UnicastZenPing.java index aed0a1e1d36..bc466adfc7f 100644 --- a/core/src/main/java/org/elasticsearch/discovery/zen/ping/unicast/UnicastZenPing.java +++ b/core/src/main/java/org/elasticsearch/discovery/zen/ping/unicast/UnicastZenPing.java @@ -103,7 +103,6 @@ public class UnicastZenPing extends AbstractLifecycleComponent implements ZenPin private final ThreadPool threadPool; private final TransportService transportService; private final ClusterName clusterName; - private final ElectMasterService electMasterService; private final int concurrentConnects; @@ -132,12 +131,11 @@ public class UnicastZenPing extends AbstractLifecycleComponent implements ZenPin @Inject public UnicastZenPing(Settings settings, ThreadPool threadPool, TransportService transportService, - ElectMasterService electMasterService, @Nullable Set unicastHostsProviders) { + @Nullable Set unicastHostsProviders) { super(settings); this.threadPool = threadPool; this.transportService = transportService; this.clusterName = ClusterName.CLUSTER_NAME_SETTING.get(settings); - this.electMasterService = electMasterService; if (unicastHostsProviders != null) { for (UnicastHostsProvider unicastHostsProvider : unicastHostsProviders) { @@ -361,7 +359,7 @@ public class UnicastZenPing extends AbstractLifecycleComponent implements ZenPin } // sort the nodes by likelihood of being an active master - List sortedNodesToPing = electMasterService.sortByMasterLikelihood(nodesToPingSet); + List sortedNodesToPing = ElectMasterService.sortByMasterLikelihood(nodesToPingSet); // new add the unicast targets first List nodesToPing = CollectionUtils.arrayAsArrayList(configuredTargetNodes); diff --git a/core/src/test/java/org/elasticsearch/discovery/DiscoveryModuleTests.java b/core/src/test/java/org/elasticsearch/discovery/DiscoveryModuleTests.java index ec5a20c83e6..c21ec6b386c 100644 --- a/core/src/test/java/org/elasticsearch/discovery/DiscoveryModuleTests.java +++ b/core/src/test/java/org/elasticsearch/discovery/DiscoveryModuleTests.java @@ -33,21 +33,6 @@ public class DiscoveryModuleTests extends ModuleTestCase { } } - public void testRegisterMasterElectionService() { - Settings settings = Settings.builder().put(DiscoveryModule.ZEN_MASTER_SERVICE_TYPE_SETTING.getKey(), "custom").build(); - DiscoveryModule module = new DiscoveryModule(settings); - module.addElectMasterService("custom", DummyMasterElectionService.class); - assertBinding(module, ElectMasterService.class, DummyMasterElectionService.class); - assertBinding(module, Discovery.class, ZenDiscovery.class); - } - - public void testLoadUnregisteredMasterElectionService() { - Settings settings = Settings.builder().put(DiscoveryModule.ZEN_MASTER_SERVICE_TYPE_SETTING.getKey(), "foobar").build(); - DiscoveryModule module = new DiscoveryModule(settings); - module.addElectMasterService("custom", DummyMasterElectionService.class); - assertBindingFailure(module, "Unknown master service type [foobar]"); - } - public void testRegisterDefaults() { Settings settings = Settings.EMPTY; DiscoveryModule module = new DiscoveryModule(settings); @@ -60,6 +45,4 @@ public class DiscoveryModuleTests extends ModuleTestCase { module.addDiscoveryType("custom", NoopDiscovery.class); assertBinding(module, Discovery.class, NoopDiscovery.class); } - - } diff --git a/core/src/test/java/org/elasticsearch/discovery/zen/ElectMasterServiceTests.java b/core/src/test/java/org/elasticsearch/discovery/zen/ElectMasterServiceTests.java index b836d2cff9a..0c4862bf32a 100644 --- a/core/src/test/java/org/elasticsearch/discovery/zen/ElectMasterServiceTests.java +++ b/core/src/test/java/org/elasticsearch/discovery/zen/ElectMasterServiceTests.java @@ -76,7 +76,7 @@ public class ElectMasterServiceTests extends ESTestCase { public void testSortByMasterLikelihood() { List nodes = generateRandomNodes(); - List sortedNodes = electMasterService().sortByMasterLikelihood(nodes); + List sortedNodes = ElectMasterService.sortByMasterLikelihood(nodes); assertEquals(nodes.size(), sortedNodes.size()); DiscoveryNode prevNode = sortedNodes.get(0); for (int i = 1; i < sortedNodes.size(); i++) { diff --git a/core/src/test/java/org/elasticsearch/discovery/zen/ZenDiscoveryUnitTests.java b/core/src/test/java/org/elasticsearch/discovery/zen/ZenDiscoveryUnitTests.java index 53b7f95d829..e86afe7664b 100644 --- a/core/src/test/java/org/elasticsearch/discovery/zen/ZenDiscoveryUnitTests.java +++ b/core/src/test/java/org/elasticsearch/discovery/zen/ZenDiscoveryUnitTests.java @@ -218,9 +218,7 @@ public class ZenDiscoveryUnitTests extends ESTestCase { private ZenDiscovery buildZenDiscovery(Settings settings, TransportService service, ClusterService clusterService, ThreadPool threadPool) { ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); ZenPingService zenPingService = new ZenPingService(settings, Collections.emptySet()); - ElectMasterService electMasterService = new ElectMasterService(settings); - ZenDiscovery zenDiscovery = new ZenDiscovery(settings, threadPool, service, clusterService, - clusterSettings, zenPingService, electMasterService); + ZenDiscovery zenDiscovery = new ZenDiscovery(settings, threadPool, service, clusterService, clusterSettings, zenPingService); zenDiscovery.start(); return zenDiscovery; } diff --git a/core/src/test/java/org/elasticsearch/discovery/zen/ping/unicast/UnicastZenPingTests.java b/core/src/test/java/org/elasticsearch/discovery/zen/ping/unicast/UnicastZenPingTests.java index f9b2c61d627..e382f93f651 100644 --- a/core/src/test/java/org/elasticsearch/discovery/zen/ping/unicast/UnicastZenPingTests.java +++ b/core/src/test/java/org/elasticsearch/discovery/zen/ping/unicast/UnicastZenPingTests.java @@ -72,7 +72,6 @@ public class UnicastZenPingTests extends ESTestCase { ThreadPool threadPool = new TestThreadPool(getClass().getName()); NetworkService networkService = new NetworkService(settings, Collections.emptyList()); - ElectMasterService electMasterService = new ElectMasterService(settings); NetworkHandle handleA = startServices(settings, threadPool, networkService, "UZP_A", Version.CURRENT); NetworkHandle handleB = startServices(settings, threadPool, networkService, "UZP_B", Version.CURRENT); @@ -94,7 +93,7 @@ public class UnicastZenPingTests extends ESTestCase { .build(); Settings hostsSettingsMismatch = Settings.builder().put(hostsSettings).put(settingsMismatch).build(); - UnicastZenPing zenPingA = new UnicastZenPing(hostsSettings, threadPool, handleA.transportService, electMasterService, null); + UnicastZenPing zenPingA = new UnicastZenPing(hostsSettings, threadPool, handleA.transportService, null); zenPingA.setPingContextProvider(new PingContextProvider() { @Override public DiscoveryNodes nodes() { @@ -108,7 +107,7 @@ public class UnicastZenPingTests extends ESTestCase { }); zenPingA.start(); - UnicastZenPing zenPingB = new UnicastZenPing(hostsSettings, threadPool, handleB.transportService, electMasterService, null); + UnicastZenPing zenPingB = new UnicastZenPing(hostsSettings, threadPool, handleB.transportService, null); zenPingB.setPingContextProvider(new PingContextProvider() { @Override public DiscoveryNodes nodes() { @@ -122,8 +121,7 @@ public class UnicastZenPingTests extends ESTestCase { }); zenPingB.start(); - UnicastZenPing zenPingC = new UnicastZenPing(hostsSettingsMismatch, threadPool, handleC.transportService, electMasterService, - null) { + UnicastZenPing zenPingC = new UnicastZenPing(hostsSettingsMismatch, threadPool, handleC.transportService, null) { @Override protected Version getVersion() { return versionD; @@ -142,7 +140,7 @@ public class UnicastZenPingTests extends ESTestCase { }); zenPingC.start(); - UnicastZenPing zenPingD = new UnicastZenPing(hostsSettingsMismatch, threadPool, handleD.transportService, electMasterService, null); + UnicastZenPing zenPingD = new UnicastZenPing(hostsSettingsMismatch, threadPool, handleD.transportService, null); zenPingD.setPingContextProvider(new PingContextProvider() { @Override public DiscoveryNodes nodes() {