diff options
author | Tor Brede Vekterli <vekterli@verizonmedia.com> | 2021-03-19 11:21:20 +0100 |
---|---|---|
committer | Tor Brede Vekterli <vekterli@verizonmedia.com> | 2021-03-19 11:21:20 +0100 |
commit | d1719be14a56216131de9ab6d3db6239852488d8 (patch) | |
tree | 6e286022be5b63f021f08ac5ea6e639f08f92081 /clustercontroller-core | |
parent | 1a128c911792bd0d19f70f86aa42c10c9401dabb (diff) |
Guard against ever accidentally publishing a default constructed state
Since version 0 states were ambiguous with the sentinel values for
"not written to ZK/not tagged as official", this could be mis-interpreted.
Diffstat (limited to 'clustercontroller-core')
3 files changed, 20 insertions, 20 deletions
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcaster.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcaster.java index 0364d46f582..b29e232dc08 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcaster.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcaster.java @@ -26,7 +26,7 @@ public class SystemStateBroadcaster { private final static long minTimeBetweenNodeErrorLogging = 10 * 60 * 1000; private final Map<Node, Long> lastErrorReported = new TreeMap<>(); - private int lastOfficialStateVersion = 0; + private int lastOfficialStateVersion = -1; private int lastStateVersionBundleAcked = 0; private int lastClusterStateVersionConverged = 0; private ClusterStateBundle lastClusterStateBundleConverged; @@ -266,7 +266,7 @@ public class SystemStateBroadcaster { public boolean broadcastNewStateBundleIfRequired(DatabaseHandler.Context dbContext, Communicator communicator, int lastClusterStateVersionWrittenToZooKeeper) { - if (clusterStateBundle == null) { + if (clusterStateBundle == null || clusterStateBundle.getVersion() == 0) { return false; } if (clusterStateBundle.getVersion() != lastClusterStateVersionWrittenToZooKeeper) { @@ -300,7 +300,7 @@ public class SystemStateBroadcaster { } public boolean broadcastStateActivationsIfRequired(DatabaseHandler.Context dbContext, Communicator communicator) { - if (clusterStateBundle == null || !currentBundleVersionIsTaggedOfficial()) { + if (clusterStateBundle == null || clusterStateBundle.getVersion() == 0 || !currentBundleVersionIsTaggedOfficial()) { return false; } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/DatabaseHandler.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/DatabaseHandler.java index 4f02e31b426..3f04bbd9200 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/DatabaseHandler.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/DatabaseHandler.java @@ -87,7 +87,7 @@ public class DatabaseHandler { private final DatabaseListener dbListener = new DatabaseListener(); private final Data currentlyStored = new Data(); private final Data pendingStore = new Data(); - private int lastKnownStateBundleVersionWrittenBySelf = 0; + private int lastKnownStateBundleVersionWrittenBySelf = -1; private long lastZooKeeperConnectionAttempt = 0; private static final int minimumWaitBetweenFailedConnectionAttempts = 10000; private boolean lostZooKeeperConnectionEvent = false; diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcasterTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcasterTest.java index bb1c32638a2..cec2b0298fb 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcasterTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcasterTest.java @@ -86,39 +86,39 @@ public class SystemStateBroadcasterTest { @Test public void always_publish_baseline_cluster_state() { Fixture f = new Fixture(); - ClusterStateBundle stateBundle = ClusterStateBundleUtil.makeBundle("distributor:2 storage:2"); + ClusterStateBundle stateBundle = ClusterStateBundleUtil.makeBundle("version:3 distributor:2 storage:2"); ClusterFixture cf = ClusterFixture.forFlatCluster(2).bringEntireClusterUp().assignDummyRpcAddresses(); f.broadcaster.handleNewClusterStates(stateBundle); - f.broadcaster.broadcastNewStateBundleIfRequired(dbContextFrom(cf.cluster()), f.mockCommunicator, 0); + f.broadcaster.broadcastNewStateBundleIfRequired(dbContextFrom(cf.cluster()), f.mockCommunicator, 3); cf.cluster().getNodeInfo().forEach(nodeInfo -> verify(f.mockCommunicator).setSystemState(eq(stateBundle), eq(nodeInfo), any())); } @Test public void non_observed_startup_timestamps_are_published_per_node_for_baseline_state() { Fixture f = new Fixture(); - ClusterStateBundle stateBundle = ClusterStateBundleUtil.makeBundle("distributor:2 storage:2"); + ClusterStateBundle stateBundle = ClusterStateBundleUtil.makeBundle("version:3 distributor:2 storage:2"); ClusterFixture cf = ClusterFixture.forFlatCluster(2).bringEntireClusterUp().assignDummyRpcAddresses(); f.simulateNodePartitionedAwaySilently(cf); f.broadcaster.handleNewClusterStates(stateBundle); - f.broadcaster.broadcastNewStateBundleIfRequired(dbContextFrom(cf.cluster()), f.mockCommunicator, 0); + f.broadcaster.broadcastNewStateBundleIfRequired(dbContextFrom(cf.cluster()), f.mockCommunicator, 3); clusterNodeInfos(cf.cluster(), Node.ofDistributor(1), Node.ofStorage(0), Node.ofStorage(1)).forEach(nodeInfo -> { // Only distributor 0 should observe startup timestamps verify(f.mockCommunicator).setSystemState(eq(stateBundle), eq(nodeInfo), any()); }); - ClusterStateBundle expectedDistr0Bundle = ClusterStateBundleUtil.makeBundle("distributor:2 storage:2 .0.t:600 .1.t:700"); + ClusterStateBundle expectedDistr0Bundle = ClusterStateBundleUtil.makeBundle("version:3 distributor:2 storage:2 .0.t:600 .1.t:700"); verify(f.mockCommunicator).setSystemState(eq(expectedDistr0Bundle), eq(cf.cluster().getNodeInfo(Node.ofDistributor(0))), any()); } @Test public void bucket_space_states_are_published_verbatim_when_no_additional_timestamps_needed() { Fixture f = new Fixture(); - ClusterStateBundle stateBundle = ClusterStateBundleUtil.makeBundle("distributor:2 storage:2", - StateMapping.of("default", "distributor:2 storage:2 .0.s:d"), - StateMapping.of("upsidedown", "distributor:2 .0.s:d storage:2")); + ClusterStateBundle stateBundle = ClusterStateBundleUtil.makeBundle("version:3 distributor:2 storage:2", + StateMapping.of("default", "version:3 distributor:2 storage:2 .0.s:d"), + StateMapping.of("upsidedown", "version:3 distributor:2 .0.s:d storage:2")); ClusterFixture cf = ClusterFixture.forFlatCluster(2).bringEntireClusterUp().assignDummyRpcAddresses(); f.broadcaster.handleNewClusterStates(stateBundle); - f.broadcaster.broadcastNewStateBundleIfRequired(dbContextFrom(cf.cluster()), f.mockCommunicator, 0); + f.broadcaster.broadcastNewStateBundleIfRequired(dbContextFrom(cf.cluster()), f.mockCommunicator, 3); cf.cluster().getNodeInfo().forEach(nodeInfo -> verify(f.mockCommunicator).setSystemState(eq(stateBundle), eq(nodeInfo), any())); } @@ -126,21 +126,21 @@ public class SystemStateBroadcasterTest { @Test public void non_observed_startup_timestamps_are_published_per_bucket_space_state() { Fixture f = new Fixture(); - ClusterStateBundle stateBundle = ClusterStateBundleUtil.makeBundle("distributor:2 storage:2", - StateMapping.of("default", "distributor:2 storage:2 .0.s:d"), - StateMapping.of("upsidedown", "distributor:2 .0.s:d storage:2")); + ClusterStateBundle stateBundle = ClusterStateBundleUtil.makeBundle("version:3 distributor:2 storage:2", + StateMapping.of("default", "version:3 distributor:2 storage:2 .0.s:d"), + StateMapping.of("upsidedown", "version:3 distributor:2 .0.s:d storage:2")); ClusterFixture cf = ClusterFixture.forFlatCluster(2).bringEntireClusterUp().assignDummyRpcAddresses(); f.simulateNodePartitionedAwaySilently(cf); f.broadcaster.handleNewClusterStates(stateBundle); - f.broadcaster.broadcastNewStateBundleIfRequired(dbContextFrom(cf.cluster()), f.mockCommunicator, 0); + f.broadcaster.broadcastNewStateBundleIfRequired(dbContextFrom(cf.cluster()), f.mockCommunicator, 3); clusterNodeInfos(cf.cluster(), Node.ofDistributor(1), Node.ofStorage(0), Node.ofStorage(1)).forEach(nodeInfo -> { // Only distributor 0 should observe startup timestamps verify(f.mockCommunicator).setSystemState(eq(stateBundle), eq(nodeInfo), any()); }); - ClusterStateBundle expectedDistr0Bundle = ClusterStateBundleUtil.makeBundle("distributor:2 storage:2 .0.t:600 .1.t:700", - StateMapping.of("default", "distributor:2 storage:2 .0.s:d .0.t:600 .1.t:700"), - StateMapping.of("upsidedown", "distributor:2 .0.s:d storage:2 .0.t:600 .1.t:700")); + ClusterStateBundle expectedDistr0Bundle = ClusterStateBundleUtil.makeBundle("version:3 distributor:2 storage:2 .0.t:600 .1.t:700", + StateMapping.of("default", "version:3 distributor:2 storage:2 .0.s:d .0.t:600 .1.t:700"), + StateMapping.of("upsidedown", "version:3 distributor:2 .0.s:d storage:2 .0.t:600 .1.t:700")); verify(f.mockCommunicator).setSystemState(eq(expectedDistr0Bundle), eq(cf.cluster().getNodeInfo(Node.ofDistributor(0))), any()); } |