From 5609231a42961235bbdb041922a3b026401a8d30 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Mon, 19 Sep 2022 21:37:32 +0200 Subject: Simplify: Require wait task to be non-null --- .../clustercontroller/core/RpcServerTest.java | 39 +++++++++++++--------- .../clustercontroller/core/testutils/Waiter.java | 17 +++++----- 2 files changed, 32 insertions(+), 24 deletions(-) (limited to 'clustercontroller-core') diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/RpcServerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/RpcServerTest.java index be06ea5a0d5..41874b9a224 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/RpcServerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/RpcServerTest.java @@ -21,6 +21,7 @@ import com.yahoo.vdslib.state.State; import com.yahoo.vespa.clustercontroller.core.rpc.RpcServer; import com.yahoo.vespa.clustercontroller.core.testutils.LogFormatter; import com.yahoo.vespa.clustercontroller.core.testutils.WaitCondition; +import com.yahoo.vespa.clustercontroller.core.testutils.WaitTask; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -114,22 +115,28 @@ public class RpcServerTest extends FleetControllerTest { timer.advanceTime(options.nodeStateRequestTimeoutMS() + options.maxSlobrokDisconnectGracePeriod()); wait(new WaitCondition.StateWait(fleetController(), fleetController().getMonitor()) { - @Override - public String isConditionMet() { - if (currentState == null) { - return "No cluster state defined yet"; - } - NodeState distState = currentState.getNodeState(new Node(NodeType.DISTRIBUTOR, 0)); - if (distState.getState() != State.DOWN) { - return "Distributor not detected down yet: " + currentState.toString(); - } - NodeState storState = currentState.getNodeState(new Node(NodeType.STORAGE, 9)); - if (!storState.getState().oneOf("md")) { - return "Storage node not detected down yet: " + currentState.toString(); - } - return null; - } - }, null, timeout()); + @Override + public String isConditionMet() { + if (currentState == null) { + return "No cluster state defined yet"; + } + NodeState distState = currentState.getNodeState(new Node(NodeType.DISTRIBUTOR, 0)); + if (distState.getState() != State.DOWN) { + return "Distributor not detected down yet: " + currentState.toString(); + } + NodeState storState = currentState.getNodeState(new Node(NodeType.STORAGE, 9)); + if (!storState.getState().oneOf("md")) { + return "Storage node not detected down yet: " + currentState.toString(); + } + return null; + } + }, new WaitTask() { + @Override + public boolean performWaitTask() { + return false; + } + }, + timeout()); int rpcPort = fleetController().getRpcPort(); Target connection = supervisor.connect(new Spec("localhost", rpcPort)); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/testutils/Waiter.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/testutils/Waiter.java index a7de2f0eedb..5bff101906d 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/testutils/Waiter.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/testutils/Waiter.java @@ -10,6 +10,7 @@ import java.time.Instant; import java.util.Collections; import java.util.LinkedList; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; @@ -88,7 +89,9 @@ public interface Waiter { } public final void wait(WaitCondition c, WaitTask wt, Duration timeout) { - log.log(Level.INFO, "Waiting for " + c + (wt == null ? "" : " with wait task " + wt)); + Objects.requireNonNull(wt, "wait task cannot be null"); + + log.log(Level.INFO, "Waiting for " + c + " with wait task " + wt); Instant endTime = Instant.now().plus(timeout); String lastReason = null; while (true) { @@ -104,17 +107,15 @@ public interface Waiter { } try { boolean allowWait = true; - if (wt != null) { - if (wt.performWaitTask()) { - data.getMonitor().notifyAll(); - allowWait = false; - } + if (wt.performWaitTask()) { + data.getMonitor().notifyAll(); + allowWait = false; } Duration timeLeft = Duration.between(Instant.now(), endTime); if (timeLeft.isNegative()) - throw new IllegalStateException("Timed out waiting max " + timeout + " ms for " + c + (wt == null ? "" : "\n with wait task " + wt) + ",\n reason: " + reason); + throw new IllegalStateException("Timed out waiting max " + timeout + " ms for " + c + "\n with wait task " + wt + ",\n reason: " + reason); if (allowWait) - data.getMonitor().wait(wt == null ? WaitTask.defaultTaskFrequencyMillis : Math.min(wt.getWaitTaskFrequencyInMillis(), timeLeft.toMillis())); + data.getMonitor().wait(Math.min(wt.getWaitTaskFrequencyInMillis(), timeLeft.toMillis())); } catch (InterruptedException e) { throw new RuntimeException(e); } -- cgit v1.2.3 From 76e4699558a2648e1550d69110ff36a96840776f Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Mon, 19 Sep 2022 21:38:04 +0200 Subject: Remove unnecessary/useless test --- .../vespa/clustercontroller/core/RpcServerTest.java | 20 -------------------- 1 file changed, 20 deletions(-) (limited to 'clustercontroller-core') diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/RpcServerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/RpcServerTest.java index 41874b9a224..d120dc06c9f 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/RpcServerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/RpcServerTest.java @@ -78,26 +78,6 @@ public class RpcServerTest extends FleetControllerTest { slobrok.stop(); } - /** - * For some reason, the first test trying to set up a stable system here occasionally times out. - * The theory is that some test run before it does something that is not cleaned up in time. - * Trying to add a test that should provoke the failure, but not fail due to it to see if we can verify that - * assumption. - * - * (testRebinding() does not seem to be that test. Tests in StateChangeTest that runs before this test tests very - * similar things, so strange if it should be from them too though. Maybe last test there. - */ - @Test - void testFailOccasionallyAndIgnoreToSeeIfOtherTestsThenWork() { - try { - startingTest("RpcServerTest::testFailOccasionallyAndIgnoreToSeeIfOtherTestsThenWork"); - setUpFleetController(true, defaultOptions("mycluster")); - setUpVdsNodes(true); - waitForStableSystem(); - } catch (Throwable t) { - } - } - @Test void testGetSystemState() throws Exception { LogFormatter.initializeLogging(); -- cgit v1.2.3 From bd7b4d77d57c6d40d0da9387a89bdeffefaacd98 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Tue, 20 Sep 2022 10:42:13 +0200 Subject: configuredBucketSpaces never changes, make it final --- .../com/yahoo/vespa/clustercontroller/core/FleetController.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'clustercontroller-core') diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java index 53dc71b2a90..f2f9e52262b 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java @@ -32,7 +32,6 @@ import java.time.Instant; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.HashSet; import java.util.LinkedList; import java.util.List; @@ -99,7 +98,7 @@ public class FleetController implements NodeListener, SlobrokListener, SystemSta // Legacy behavior is an empty set of explicitly configured bucket spaces, which means that // only a baseline cluster state will be sent from the controller and no per-space state // deriving is done. - private Set configuredBucketSpaces = Collections.emptySet(); + private final Set configuredBucketSpaces = Set.of(FixedBucketSpaces.defaultSpace(), FixedBucketSpaces.globalSpace()); public FleetController(FleetControllerContext context, Timer timer, @@ -487,7 +486,6 @@ public class FleetController implements NodeListener, SlobrokListener, SystemSta cluster.setSlobrokGenerationCount(0); } - configuredBucketSpaces = Set.of(FixedBucketSpaces.defaultSpace(), FixedBucketSpaces.globalSpace()); stateVersionTracker.setMinMergeCompletionRatio(options.minMergeCompletionRatio()); communicator.propagateOptions(options); @@ -625,7 +623,7 @@ public class FleetController implements NodeListener, SlobrokListener, SystemSta if (tickStopTime >= tickStartTime) { metricUpdater.addTickTime(tickStopTime - tickStartTime, didWork); } - // Always sleep some to use avoid using too much CPU and avoid starving waiting threads + // Always sleep some to avoid using too much CPU and avoid starving waiting threads monitor.wait(didWork || waitingForCycle ? 1 : options.cycleWaitTime()); if ( ! isRunning()) { return; } tickStartTime = timer.getCurrentTimeInMillis(); -- cgit v1.2.3 From 7d145e39d3d043fe696c4e999040fb6d1c24b0c6 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Tue, 20 Sep 2022 11:21:20 +0200 Subject: Set slobrok connection specs in constructor --- .../clustercontroller/core/FleetController.java | 2 +- .../clustercontroller/core/rpc/SlobrokClient.java | 24 ++++++++++++++-------- .../core/FleetControllerTest.java | 3 +-- 3 files changed, 18 insertions(+), 11 deletions(-) (limited to 'clustercontroller-core') diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java index f2f9e52262b..80b79f4eb13 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java @@ -167,7 +167,7 @@ public class FleetController implements NodeListener, SlobrokListener, SystemSta options.nodeStateRequestTimeoutLatestPercentage(), options.nodeStateRequestRoundTripTimeMaxSeconds()); var database = new DatabaseHandler(context, new ZooKeeperDatabaseFactory(context), timer, options.zooKeeperServerAddress(), timer); - var lookUp = new SlobrokClient(context, timer); + var lookUp = new SlobrokClient(context, timer, options.slobrokConnectionSpecs()); var stateGenerator = new StateChangeHandler(context, timer, log); var stateBroadcaster = new SystemStateBroadcaster(context, timer, timer); var masterElectionHandler = new MasterElectionHandler(context, options.fleetControllerIndex(), options.fleetControllerCount(), timer, timer); diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlobrokClient.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlobrokClient.java index 8393e776fc2..d87e0e20908 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlobrokClient.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlobrokClient.java @@ -35,30 +35,37 @@ public class SlobrokClient implements NodeLookup { private Supervisor supervisor; private boolean freshMirror = false; - public SlobrokClient(FleetControllerContext context, Timer timer) { + public SlobrokClient(FleetControllerContext context, Timer timer, String[] connectionSpecs) { this.context = context; this.timer = timer; + this.connectionSpecs = connectionSpecs; + setup(); } - public boolean equalsExistingSpec(String spec[]) { + public boolean equalsExistingSpec(String[] spec) { if (spec == null && connectionSpecs == null) return true; - if (spec == null && connectionSpecs != null) return false; - if (spec != null && connectionSpecs == null) return false; + if (spec == null) return false; + if (connectionSpecs == null) return false; if (spec.length != connectionSpecs.length) return false; - for (int i=0, n=spec.length; i