From b03bc044f8d631e5399a835fdc90eadf13b07630 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Tue, 16 Feb 2021 07:57:39 +0100 Subject: Avoid recreation of ClusterController when config changes --- .../apps/clustercontroller/ClusterController.java | 13 +++++-------- .../ClusterControllerClusterConfigurer.java | 11 ++++++++++- .../ClusterControllerClusterConfigurerTest.java | 6 ++++-- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterController.java b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterController.java index ff7cbbf83da..05fad478cc7 100644 --- a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterController.java +++ b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterController.java @@ -31,16 +31,13 @@ public class ClusterController extends AbstractComponent private final Map status = new TreeMap<>(); /** - * Dependency injection constructor for controller. {@link VespaZooKeeperServer} argument given - * to ensure that zookeeper has started before we start polling it. + * Dependency injection constructor for controller. A {@link VespaZooKeeperServer} argument is required + * for all its users, to ensure that zookeeper has started before we start polling it, but + * should not be injected here, as that causes recreation of the cluster controller, and old and new + * will run master election, etc., concurrently, which breaks everything. */ - @SuppressWarnings("unused") @Inject - public ClusterController(VespaZooKeeperServer ignored) { - this(); - } - - ClusterController() { + public ClusterController() { metricWrapper = new JDiscMetricWrapper(null); } diff --git a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java index 4cb6c5d222a..c6a2ecc0c1c 100644 --- a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java +++ b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java @@ -1,6 +1,7 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.clustercontroller.apps.clustercontroller; +import com.google.inject.Inject; import com.yahoo.jdisc.Metric; import com.yahoo.vdslib.distribution.Distribution; import com.yahoo.vdslib.state.NodeType; @@ -9,6 +10,7 @@ import com.yahoo.vespa.config.content.FleetcontrollerConfig; import com.yahoo.cloud.config.SlobroksConfig; import com.yahoo.vespa.config.content.StorDistributionConfig; import com.yahoo.cloud.config.ZookeepersConfig; +import com.yahoo.vespa.zookeeper.VespaZooKeeperServer; import java.time.Duration; import java.util.Map; @@ -21,12 +23,19 @@ public class ClusterControllerClusterConfigurer { private final FleetControllerOptions options; + /** + * The {@link VespaZooKeeperServer} argument is required by the injected {@link ClusterController}, + * to ensure that zookeeper has started before it starts polling it. It must be done here to avoid + * duplicates being created by the dependency injection framework. + */ + @Inject public ClusterControllerClusterConfigurer(ClusterController controller, StorDistributionConfig distributionConfig, FleetcontrollerConfig fleetcontrollerConfig, SlobroksConfig slobroksConfig, ZookeepersConfig zookeepersConfig, - Metric metricImpl) throws Exception { + Metric metricImpl, + VespaZooKeeperServer started) throws Exception { this.options = configure(distributionConfig, fleetcontrollerConfig, slobroksConfig, zookeepersConfig); if (controller != null) { controller.setOptions(options, metricImpl); diff --git a/clustercontroller-apps/src/test/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurerTest.java b/clustercontroller-apps/src/test/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurerTest.java index 76eff0066b1..16b978b3ce9 100644 --- a/clustercontroller-apps/src/test/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurerTest.java +++ b/clustercontroller-apps/src/test/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurerTest.java @@ -57,7 +57,8 @@ public class ClusterControllerClusterConfigurerTest { new FleetcontrollerConfig(fleetcontrollerConfig), new SlobroksConfig(slobroksConfig), new ZookeepersConfig(zookeepersConfig), - metric + metric, + null ); assertTrue(configurer.getOptions() != null); assertEquals(0.123, configurer.getOptions().minNodeRatioPerGroup, 0.01); @@ -74,7 +75,8 @@ public class ClusterControllerClusterConfigurerTest { new FleetcontrollerConfig(fleetcontrollerConfig), new SlobroksConfig(slobroksConfig), new ZookeepersConfig(zookeepersConfig), - metric + metric, + null ); fail("Should not get here"); } catch (Exception e) { -- cgit v1.2.3