diff options
100 files changed, 666 insertions, 778 deletions
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java index 43412311436..ddb9357f11f 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java @@ -10,7 +10,7 @@ import com.yahoo.vdslib.distribution.ConfiguredNode; import com.yahoo.vdslib.state.*; import com.yahoo.vdslib.distribution.Distribution; import com.yahoo.vdslib.distribution.Group; -import com.yahoo.vespa.clustercontroller.core.status.statuspage.VdsClusterHtmlRendrer; +import com.yahoo.vespa.clustercontroller.core.status.statuspage.VdsClusterHtmlRenderer; import com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStateRequest; import java.util.*; @@ -42,7 +42,7 @@ public class ContentCluster { } public void writeHtmlState( - final VdsClusterHtmlRendrer vdsClusterHtmlRendrer, + final VdsClusterHtmlRenderer vdsClusterHtmlRenderer, final StringBuilder sb, final Timer timer, final ClusterStateBundle state, @@ -51,8 +51,8 @@ public class ContentCluster { final FleetControllerOptions options, final EventLog eventLog) { - final VdsClusterHtmlRendrer.Table table = - vdsClusterHtmlRendrer.createNewClusterHtmlTable(clusterName, slobrokGenerationCount); + final VdsClusterHtmlRenderer.Table table = + vdsClusterHtmlRenderer.createNewClusterHtmlTable(clusterName, slobrokGenerationCount); final List<Group> groups = LeafGroups.enumerateFrom(distribution.getRootGroup()); diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/LegacyIndexPageRequestHandler.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/LegacyIndexPageRequestHandler.java index 6d11c4f1239..f799492d164 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/LegacyIndexPageRequestHandler.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/LegacyIndexPageRequestHandler.java @@ -6,7 +6,7 @@ import com.yahoo.vespa.clustercontroller.core.*; import com.yahoo.vespa.clustercontroller.core.Timer; import com.yahoo.vespa.clustercontroller.core.status.statuspage.StatusPageResponse; import com.yahoo.vespa.clustercontroller.core.status.statuspage.StatusPageServer; -import com.yahoo.vespa.clustercontroller.core.status.statuspage.VdsClusterHtmlRendrer; +import com.yahoo.vespa.clustercontroller.core.status.statuspage.VdsClusterHtmlRenderer; import java.util.*; @@ -62,7 +62,7 @@ public class LegacyIndexPageRequestHandler implements StatusPageServer.RequestHa if (masterElectionHandler.isAmongNthFirst(data.getOptions().stateGatherCount)) { // Table overview of all the nodes cluster.writeHtmlState( - new VdsClusterHtmlRendrer(), + new VdsClusterHtmlRenderer(), content, timer, stateVersionTracker.getVersionedClusterStateBundle(), diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/statuspage/VdsClusterHtmlRendrer.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/statuspage/VdsClusterHtmlRenderer.java index 0daa9ff7d37..6d21aa430a2 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/statuspage/VdsClusterHtmlRendrer.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/statuspage/VdsClusterHtmlRenderer.java @@ -15,11 +15,10 @@ import java.util.Map; import java.util.TimeZone; import java.util.TreeMap; - /** - * Renders webpage with status regarding cluster. + * Renders web page with cluster status. */ -public class VdsClusterHtmlRendrer { +public class VdsClusterHtmlRenderer { private static final TimeZone utcTimeZone = TimeZone.getTimeZone("UTC"); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/AggregatedStatsMergePendingCheckerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/AggregatedStatsMergePendingCheckerTest.java index 304579cf75a..65e6e0c2a03 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/AggregatedStatsMergePendingCheckerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/AggregatedStatsMergePendingCheckerTest.java @@ -15,38 +15,37 @@ public class AggregatedStatsMergePendingCheckerTest { private final AggregatedClusterStats mockAggregatedStats = mock(AggregatedClusterStats.class); private final AggregatedStatsMergePendingChecker checker; - public Fixture(ContentClusterStatsBuilder builder, - boolean hasUpdatesFromAllDistributors, - double minMergeCompletionRatio) { + Fixture(ContentClusterStatsBuilder builder, + boolean hasUpdatesFromAllDistributors, + double minMergeCompletionRatio) { when(mockAggregatedStats.getStats()).thenReturn(builder.build()); when(mockAggregatedStats.hasUpdatesFromAllDistributors()).thenReturn(hasUpdatesFromAllDistributors); this.checker = new AggregatedStatsMergePendingChecker(mockAggregatedStats, minMergeCompletionRatio); } - public static Fixture fromBucketsPending(long bucketsPending) { + static Fixture fromBucketsPending(long bucketsPending) { return new Fixture(new ContentClusterStatsBuilder() .add(1, "default", 5, bucketsPending), true, 1.0); } - public static Fixture fromBucketsPending(long bucketsPending, - double minMergeCompletionRatio) { + static Fixture fromBucketsPending(long bucketsPending, double minMergeCompletionRatio) { return new Fixture(new ContentClusterStatsBuilder() .add(1, "default", 5, bucketsPending), true, minMergeCompletionRatio); } - public static Fixture fromInvalidBucketStats() { + static Fixture fromInvalidBucketStats() { return new Fixture(new ContentClusterStatsBuilder() .add(1, "default"), true, 1.0); } - public static Fixture fromIncompleteStats() { + static Fixture fromIncompleteStats() { return new Fixture(new ContentClusterStatsBuilder(), false, 1.0); } - public boolean mayHaveMergesPending(String bucketSpace, int contentNodeIndex) { + boolean mayHaveMergesPending(String bucketSpace, int contentNodeIndex) { return checker.mayHaveMergesPending(bucketSpace, contentNodeIndex); } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFixture.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFixture.java index 5692e961cbe..2df9279e450 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFixture.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFixture.java @@ -24,9 +24,9 @@ public class ClusterFixture { public final ContentCluster cluster; public final Distribution distribution; public final FakeTimer timer; - public final EventLogInterface eventLog; - public final StateChangeHandler nodeStateChangeHandler; - public final ClusterStateGenerator.Params params = new ClusterStateGenerator.Params(); + private final EventLogInterface eventLog; + final StateChangeHandler nodeStateChangeHandler; + private final ClusterStateGenerator.Params params = new ClusterStateGenerator.Params(); public ClusterFixture(ContentCluster cluster, Distribution distribution) { this.cluster = cluster; @@ -37,7 +37,7 @@ public class ClusterFixture { this.params.cluster(this.cluster); } - public StateChangeHandler createNodeStateChangeHandlerForCluster() { + private StateChangeHandler createNodeStateChangeHandlerForCluster() { final int controllerIndex = 0; MetricUpdater metricUpdater = new MetricUpdater(new NoMetricReporter(), controllerIndex); return new StateChangeHandler(timer, eventLog, metricUpdater); @@ -51,7 +51,7 @@ public class ClusterFixture { return this; } - public ClusterFixture markEntireClusterDown() { + ClusterFixture markEntireClusterDown() { cluster.clusterInfo().getConfiguredNodes().forEach((idx, node) -> { reportStorageNodeState(idx, State.DOWN); reportDistributorNodeState(idx, State.DOWN); @@ -69,7 +69,7 @@ public class ClusterFixture { nodeInfo.setReportedState(nodeState, timer.getCurrentTimeInMillis()); } - public ClusterFixture reportStorageNodeState(final int index, State state, String description) { + ClusterFixture reportStorageNodeState(final int index, State state, String description) { final Node node = new Node(NodeType.STORAGE, index); final NodeState nodeState = new NodeState(NodeType.STORAGE, state); nodeState.setDescription(description); @@ -77,23 +77,23 @@ public class ClusterFixture { return this; } - public ClusterFixture reportStorageNodeState(final int index, State state) { + ClusterFixture reportStorageNodeState(final int index, State state) { return reportStorageNodeState(index, state, "mockdesc"); } - public ClusterFixture reportStorageNodeState(final int index, NodeState nodeState) { + ClusterFixture reportStorageNodeState(final int index, NodeState nodeState) { doReportNodeState(new Node(NodeType.STORAGE, index), nodeState); return this; } - public ClusterFixture reportDistributorNodeState(final int index, State state) { + ClusterFixture reportDistributorNodeState(final int index, State state) { final Node node = new Node(NodeType.DISTRIBUTOR, index); final NodeState nodeState = new NodeState(NodeType.DISTRIBUTOR, state); doReportNodeState(node, nodeState); return this; } - public ClusterFixture reportDistributorNodeState(final int index, NodeState nodeState) { + ClusterFixture reportDistributorNodeState(final int index, NodeState nodeState) { doReportNodeState(new Node(NodeType.DISTRIBUTOR, index), nodeState); return this; } @@ -108,18 +108,18 @@ public class ClusterFixture { nodeStateChangeHandler.proposeNewNodeState(stateBefore, nodeInfo, nodeState); } - public ClusterFixture proposeStorageNodeWantedState(final int index, State state, String description) { + ClusterFixture proposeStorageNodeWantedState(final int index, State state, String description) { final Node node = new Node(NodeType.STORAGE, index); final NodeState nodeState = new NodeState(NodeType.STORAGE, state); doProposeWantedState(node, nodeState, description); return this; } - public ClusterFixture proposeStorageNodeWantedState(final int index, State state) { + ClusterFixture proposeStorageNodeWantedState(final int index, State state) { return proposeStorageNodeWantedState(index, state, "mockdesc"); } - public ClusterFixture proposeDistributorWantedState(final int index, State state) { + ClusterFixture proposeDistributorWantedState(final int index, State state) { final ClusterState stateBefore = rawGeneratedClusterState(); final Node node = new Node(NodeType.DISTRIBUTOR, index); final NodeState nodeState = new NodeState(NodeType.DISTRIBUTOR, state); @@ -131,12 +131,12 @@ public class ClusterFixture { return this; } - public ClusterFixture disableAutoClusterTakedown() { + ClusterFixture disableAutoClusterTakedown() { setMinNodesUp(0, 0, 0.0, 0.0); return this; } - public ClusterFixture setMinNodesUp(int minDistNodes, int minStorNodes, double minDistRatio, double minStorRatio) { + ClusterFixture setMinNodesUp(int minDistNodes, int minStorNodes, double minDistRatio, double minStorRatio) { params.minStorageNodesUp(minStorNodes) .minDistributorNodesUp(minDistNodes) .minRatioOfStorageNodesUp(minStorRatio) @@ -144,7 +144,7 @@ public class ClusterFixture { return this; } - public ClusterFixture setMinNodeRatioPerGroup(double upRatio) { + ClusterFixture setMinNodeRatioPerGroup(double upRatio) { params.minNodeRatioPerGroup(upRatio); return this; } @@ -154,22 +154,22 @@ public class ClusterFixture { return this; } - static public Map<NodeType, Integer> buildTransitionTimeMap(int distributorTransitionTime, int storageTransitionTime) { + static Map<NodeType, Integer> buildTransitionTimeMap(int distributorTransitionTime, int storageTransitionTime) { Map<NodeType, Integer> maxTransitionTime = new TreeMap<>(); maxTransitionTime.put(NodeType.DISTRIBUTOR, distributorTransitionTime); maxTransitionTime.put(NodeType.STORAGE, storageTransitionTime); return maxTransitionTime; } - public void disableTransientMaintenanceModeOnDown() { + void disableTransientMaintenanceModeOnDown() { this.params.transitionTimes(0); } - public void enableTransientMaintenanceModeOnDown(final int transitionTimeMs) { + void enableTransientMaintenanceModeOnDown(final int transitionTimeMs) { this.params.transitionTimes(transitionTimeMs); } - public ClusterFixture markNodeAsConfigRetired(int nodeIndex) { + ClusterFixture markNodeAsConfigRetired(int nodeIndex) { Set<ConfiguredNode> configuredNodes = new HashSet<>(cluster.getConfiguredNodes().values()); configuredNodes.remove(new ConfiguredNode(nodeIndex, false)); configuredNodes.add(new ConfiguredNode(nodeIndex, true)); @@ -177,20 +177,20 @@ public class ClusterFixture { return this; } - public AnnotatedClusterState annotatedGeneratedClusterState() { + AnnotatedClusterState annotatedGeneratedClusterState() { params.currentTimeInMilllis(timer.getCurrentTimeInMillis()); return ClusterStateGenerator.generatedStateFrom(params); } - public ClusterState rawGeneratedClusterState() { + private ClusterState rawGeneratedClusterState() { return annotatedGeneratedClusterState().getClusterState(); } - public String generatedClusterState() { + String generatedClusterState() { return annotatedGeneratedClusterState().getClusterState().toString(); } - public String verboseGeneratedClusterState() { + String verboseGeneratedClusterState() { return annotatedGeneratedClusterState().getClusterState().toString(true); } @@ -203,7 +203,7 @@ public class ClusterFixture { return new ClusterFixture(cluster, distribution); } - public static ClusterFixture forHierarchicCluster(DistributionBuilder.GroupBuilder root) { + static ClusterFixture forHierarchicCluster(DistributionBuilder.GroupBuilder root) { List<ConfiguredNode> nodes = DistributionBuilder.buildConfiguredNodes(root.totalNodeCount()); Distribution distribution = DistributionBuilder.forHierarchicCluster(root); ContentCluster cluster = new ContentCluster("foo", nodes, distribution, 0, 0.0); @@ -211,7 +211,7 @@ public class ClusterFixture { return new ClusterFixture(cluster, distribution); } - public ClusterStateGenerator.Params generatorParams() { + ClusterStateGenerator.Params generatorParams() { return new ClusterStateGenerator.Params().cluster(cluster); } @@ -219,11 +219,11 @@ public class ClusterFixture { return this.cluster; } - public static Node storageNode(int index) { + static Node storageNode(int index) { return new Node(NodeType.STORAGE, index); } - public static Node distributorNode(int index) { + static Node distributorNode(int index) { return new Node(NodeType.DISTRIBUTOR, index); } } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStateBundleTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStateBundleTest.java index 339d305e823..9ec3902fc1d 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStateBundleTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStateBundleTest.java @@ -1,7 +1,11 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.clustercontroller.core; -import com.yahoo.vdslib.state.*; +import com.yahoo.vdslib.state.ClusterState; +import com.yahoo.vdslib.state.Node; +import com.yahoo.vdslib.state.NodeState; +import com.yahoo.vdslib.state.NodeType; +import com.yahoo.vdslib.state.State; import org.junit.Test; import java.util.function.Function; diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStateGeneratorTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStateGeneratorTest.java index 958fc1ae232..2f91e569b89 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStateGeneratorTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStateGeneratorTest.java @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.clustercontroller.core; -import com.yahoo.vdslib.distribution.ConfiguredNode; import com.yahoo.vdslib.state.DiskState; import com.yahoo.vdslib.state.Node; import com.yahoo.vdslib.state.NodeState; @@ -9,7 +8,6 @@ import com.yahoo.vdslib.state.NodeType; import com.yahoo.vdslib.state.State; import org.junit.Test; -import java.util.List; import java.util.Optional; import static com.yahoo.vespa.clustercontroller.core.matchers.HasStateReasonForNode.hasStateReasonForNode; diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStateViewTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStateViewTest.java index b456965d549..b7f4b0c7ddb 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStateViewTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStateViewTest.java @@ -13,16 +13,14 @@ import static org.mockito.Mockito.*; /** * @author hakonhall - * @since 5.34 */ public class ClusterStateViewTest { - final NodeInfo nodeInfo = mock(NodeInfo.class); - final Node node = mock(Node.class); - final ClusterStatsAggregator statsAggregator = mock(ClusterStatsAggregator.class); - final ClusterState clusterState = mock(ClusterState.class); - final ClusterStateView clusterStateView = new ClusterStateView(clusterState, statsAggregator); + private final NodeInfo nodeInfo = mock(NodeInfo.class); + private final ClusterStatsAggregator statsAggregator = mock(ClusterStatsAggregator.class); + private final ClusterState clusterState = mock(ClusterState.class); + private final ClusterStateView clusterStateView = new ClusterStateView(clusterState, statsAggregator); - HostInfo createHostInfo(String version) { + private HostInfo createHostInfo(String version) { return HostInfo.createHostInfo("{ \"cluster-state-version\": " + version + " }"); } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregatorTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregatorTest.java index 7f313b31a6c..9a295e95fff 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregatorTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregatorTest.java @@ -4,7 +4,7 @@ package com.yahoo.vespa.clustercontroller.core; import com.google.common.collect.Sets; import org.junit.Test; -import java.util.*; +import java.util.Set; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -12,19 +12,18 @@ import static org.junit.Assert.assertTrue; /** * @author hakonhall - * @since 5.34 */ public class ClusterStatsAggregatorTest { private static class Fixture { private ClusterStatsAggregator aggregator; - public Fixture(Set<Integer> distributorNodes, + Fixture(Set<Integer> distributorNodes, Set<Integer> contentNodes) { aggregator = new ClusterStatsAggregator(distributorNodes, contentNodes); } - public void update(int distributorIndex, ContentClusterStatsBuilder clusterStats) { + void update(int distributorIndex, ContentClusterStatsBuilder clusterStats) { aggregator.updateForDistributor(distributorIndex, clusterStats.build()); } @@ -36,14 +35,14 @@ public class ClusterStatsAggregatorTest { assertEquals(expectedStats.build(), aggregator.getAggregatedStatsForDistributor(distributorIndex)); } - public boolean hasUpdatesFromAllDistributors() { + boolean hasUpdatesFromAllDistributors() { return aggregator.getAggregatedStats().hasUpdatesFromAllDistributors(); } } private static class FourNodesFixture extends Fixture { - public FourNodesFixture() { + FourNodesFixture() { super(distributorNodes(1, 2), contentNodes(3, 4)); update(1, new ContentClusterStatsBuilder() diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsChangeTrackerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsChangeTrackerTest.java index 6261469beca..2d0c4296c28 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsChangeTrackerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsChangeTrackerTest.java @@ -14,17 +14,17 @@ public class ClusterStatsChangeTrackerTest { private static class StatsBuilder { private final ContentClusterStatsBuilder builder = new ContentClusterStatsBuilder(); - public StatsBuilder bucketsPending(int contentNodeIndex) { + StatsBuilder bucketsPending(int contentNodeIndex) { builder.add(contentNodeIndex, "global", 5, 1); return this; } - public StatsBuilder inSync(int contentNodeIndex) { + StatsBuilder inSync(int contentNodeIndex) { builder.add(contentNodeIndex, "global", 5, 0); return this; } - public ContentClusterStats build() { + ContentClusterStats build() { return builder.build(); } } @@ -47,17 +47,17 @@ public class ClusterStatsChangeTrackerTest { tracker = new ClusterStatsChangeTracker(aggregator.getAggregatedStats(), minMergeCompletionRatio); } - public static Fixture empty() { + static Fixture empty() { return new Fixture(Sets.newHashSet(0, 1), 1.0); } - public static Fixture fromStats(StatsBuilder builder) { + static Fixture fromStats(StatsBuilder builder) { Fixture result = new Fixture(Sets.newHashSet(0, 1), 1.0); result.updateStats(builder); return result; } - public void newAggregatedStats(StatsBuilder builder) { + void newAggregatedStats(StatsBuilder builder) { aggregator = new ClusterStatsAggregator(Sets.newHashSet(1), contentNodeIndices); updateStats(builder); tracker.updateAggregatedStats(aggregator.getAggregatedStats(), minMergeCompletionRatio); @@ -67,7 +67,7 @@ public class ClusterStatsChangeTrackerTest { aggregator.updateForDistributor(1, builder.build()); } - public boolean statsHaveChanged() { + boolean statsHaveChanged() { return tracker.statsHaveChanged(); } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterHtmlRendrerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterHtmlRendrerTest.java index 9a3edf8e681..11b07c037ae 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterHtmlRendrerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterHtmlRendrerTest.java @@ -4,7 +4,7 @@ package com.yahoo.vespa.clustercontroller.core; import com.google.common.collect.Sets; import com.yahoo.vdslib.state.ClusterState; import com.yahoo.vespa.clustercontroller.core.hostinfo.HostInfo; -import com.yahoo.vespa.clustercontroller.core.status.statuspage.VdsClusterHtmlRendrer; +import com.yahoo.vespa.clustercontroller.core.status.statuspage.VdsClusterHtmlRenderer; import org.codehaus.jettison.json.JSONException; import org.codehaus.jettison.json.JSONWriter; import org.junit.Before; @@ -12,30 +12,28 @@ import org.junit.Test; import java.io.StringWriter; import java.io.Writer; -import java.text.ParseException; import java.util.TreeMap; import static org.hamcrest.core.Is.is; -import static org.junit.Assert.*; +import static org.junit.Assert.assertThat; import static org.mockito.Mockito.mock; public class ContentClusterHtmlRendrerTest { - VdsClusterHtmlRendrer rendrer = new VdsClusterHtmlRendrer(); - final static int slobrokGeneration = 34; - final static String clusterName = "clustername"; - final TreeMap<Integer, NodeInfo> storageNodeInfoByIndex = new TreeMap<>(); - final TreeMap<Integer, NodeInfo> distributorNodeInfoByIndex = new TreeMap<>(); - String result; + private VdsClusterHtmlRenderer renderer = new VdsClusterHtmlRenderer(); + private final static int slobrokGeneration = 34; + private final static String clusterName = "clustername"; + private final TreeMap<Integer, NodeInfo> storageNodeInfoByIndex = new TreeMap<>(); + private final TreeMap<Integer, NodeInfo> distributorNodeInfoByIndex = new TreeMap<>(); + private String result; @Before - public void before() throws JSONException, ParseException { + public void before() throws JSONException { final ClusterStateBundle stateBundle = ClusterStateBundle.ofBaselineOnly( AnnotatedClusterState.withoutAnnotations( ClusterState.stateFromString("version:34633 bits:24 distributor:211 storage:211"))); - final ClusterState state = stateBundle.getBaselineClusterState(); final EventLog eventLog = new EventLog(new FakeTimer(), null); - final VdsClusterHtmlRendrer.Table table = rendrer.createNewClusterHtmlTable(clusterName, slobrokGeneration); + final VdsClusterHtmlRenderer.Table table = renderer.createNewClusterHtmlTable(clusterName, slobrokGeneration); final ContentCluster contentCluster = mock(ContentCluster.class); @@ -71,7 +69,7 @@ public class ContentClusterHtmlRendrerTest { } @Test - public void testVtagRendering() throws Exception { + public void testVtagRendering() { // 9 distribution nodes should have green tag on release1. assertThat(result.split("<td bgcolor=\"#c0ffc0\" align=\"right\"><nobr>release1</nobr></td>").length, is(10)); // 1 distribution node should have warning on release1bad. diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStatsBuilder.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStatsBuilder.java index dd0205658cb..ce79b492638 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStatsBuilder.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStatsBuilder.java @@ -15,7 +15,7 @@ public class ContentClusterStatsBuilder { return add(nodeIndex, bucketSpace, ContentNodeStats.BucketSpaceStats.of(bucketsTotal, bucketsPending)); } - public ContentClusterStatsBuilder addInvalid(int nodeIndex, String bucketSpace, long bucketsTotal, long bucketsPending) { + ContentClusterStatsBuilder addInvalid(int nodeIndex, String bucketSpace, long bucketsTotal, long bucketsPending) { return add(nodeIndex, bucketSpace, ContentNodeStats.BucketSpaceStats.invalid(bucketsTotal, bucketsPending)); } @@ -38,7 +38,7 @@ public class ContentClusterStatsBuilder { return this; } - public ContentClusterStats build() { + ContentClusterStats build() { Map<Integer, ContentNodeStats> nodeToStatsMap = new HashMap<>(); stats.forEach((nodeIndex, nodeStatsBuilder) -> nodeToStatsMap.put(nodeIndex, nodeStatsBuilder.build())); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStatsBuilder.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStatsBuilder.java index f4d5b86d6da..f3cce04198d 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStatsBuilder.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStatsBuilder.java @@ -13,7 +13,7 @@ public class ContentNodeStatsBuilder { this.nodeIndex = nodeIndex; } - public static ContentNodeStatsBuilder forNode(int nodeIndex) { + static ContentNodeStatsBuilder forNode(int nodeIndex) { return new ContentNodeStatsBuilder(nodeIndex); } @@ -26,7 +26,7 @@ public class ContentNodeStatsBuilder { return this; } - public ContentNodeStats build() { + ContentNodeStats build() { return new ContentNodeStats(nodeIndex, stats); } } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DatabaseTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DatabaseTest.java index 27dcd009c96..327a0e229fc 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DatabaseTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DatabaseTest.java @@ -27,7 +27,7 @@ public class DatabaseTest extends FleetControllerTest { private static Logger log = Logger.getLogger(DatabaseTest.class.getName()); // Note: different semantics than FleetControllerTest.setWantedState - protected void setWantedState(Node n, NodeState ns, Map<Node, NodeState> wantedStates) { + private void setWantedState(Node n, NodeState ns, Map<Node, NodeState> wantedStates) { int rpcPort = fleetController.getRpcPort(); if (supervisor == null) { supervisor = new Supervisor(new Transport()); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DistributionBitCountTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DistributionBitCountTest.java index 77164b678c6..dc19770568e 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DistributionBitCountTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DistributionBitCountTest.java @@ -6,7 +6,6 @@ import com.yahoo.vdslib.state.ClusterState; import com.yahoo.vdslib.state.NodeState; import com.yahoo.vdslib.state.NodeType; import com.yahoo.vdslib.state.State; -import org.junit.Ignore; import org.junit.Test; import static org.junit.Assert.assertEquals; diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DistributionBuilder.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DistributionBuilder.java index e2eefd02c5f..67a676d388f 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DistributionBuilder.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DistributionBuilder.java @@ -12,46 +12,46 @@ import java.util.stream.IntStream; public class DistributionBuilder { // TODO support nested groups - public static class GroupBuilder { + static class GroupBuilder { final int groupCount; - public List<Integer> groupsWithNodeCount; + List<Integer> groupsWithNodeCount; - public GroupBuilder(int groupCount) { + GroupBuilder(int groupCount) { this.groupCount = groupCount; } - public GroupBuilder(int... nodeCounts) { + GroupBuilder(int... nodeCounts) { this.groupCount = nodeCounts.length; this.groupsWithNodeCount = IntStream.of(nodeCounts).boxed() .collect(Collectors.toList()); } - public GroupBuilder eachWithNodeCount(int nodeCount) { + GroupBuilder eachWithNodeCount(int nodeCount) { groupsWithNodeCount = IntStream.range(0, groupCount) .map(i -> nodeCount).boxed() .collect(Collectors.toList()); return this; } - public int totalNodeCount() { + int totalNodeCount() { return groupsWithNodeCount.stream().reduce(0, Integer::sum); } - public String groupDistributionSpec() { + String groupDistributionSpec() { return IntStream.range(0, groupCount).mapToObj(i -> "1") .collect(Collectors.joining("|")) + "|*"; } } - public static GroupBuilder withGroups(int groups) { + static GroupBuilder withGroups(int groups) { return new GroupBuilder(groups); } - public static GroupBuilder withGroupNodes(int... nodeCounts) { + static GroupBuilder withGroupNodes(int... nodeCounts) { return new GroupBuilder(nodeCounts); } - public static List<ConfiguredNode> buildConfiguredNodes(int nodeCount) { + static List<ConfiguredNode> buildConfiguredNodes(int nodeCount) { return IntStream.range(0, nodeCount) .mapToObj(i -> new ConfiguredNode(i, false)) .collect(Collectors.toList()); @@ -82,7 +82,7 @@ public class DistributionBuilder { return new Distribution(new StorDistributionConfig(configBuilder)); } - public static Distribution forHierarchicCluster(GroupBuilder root) { + static Distribution forHierarchicCluster(GroupBuilder root) { List<ConfiguredNode> nodes = buildConfiguredNodes(root.totalNodeCount()); StorDistributionConfig.Builder configBuilder = new StorDistributionConfig.Builder(); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DummyCommunicator.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DummyCommunicator.java index 8314839336e..5d5daa25e30 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DummyCommunicator.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DummyCommunicator.java @@ -16,18 +16,18 @@ import java.util.TreeMap; public class DummyCommunicator implements Communicator, NodeLookup { List<Node> newNodes; - Timer timer; + private Timer timer; private boolean shouldDeferDistributorClusterStateAcks = false; private List<Pair<Waiter<SetClusterStateRequest>, DummySetClusterStateRequest>> deferredClusterStateAcks = new ArrayList<>(); - public void setShouldDeferDistributorClusterStateAcks(boolean shouldDeferDistributorClusterStateAcks) { + void setShouldDeferDistributorClusterStateAcks(boolean shouldDeferDistributorClusterStateAcks) { this.shouldDeferDistributorClusterStateAcks = shouldDeferDistributorClusterStateAcks; } - class DummyGetNodeStateRequest extends GetNodeStateRequest { + static class DummyGetNodeStateRequest extends GetNodeStateRequest { Waiter<GetNodeStateRequest> waiter; - public DummyGetNodeStateRequest(NodeInfo nodeInfo, Waiter<GetNodeStateRequest> waiter) { + DummyGetNodeStateRequest(NodeInfo nodeInfo, Waiter<GetNodeStateRequest> waiter) { super(nodeInfo); this.waiter = waiter; @@ -39,17 +39,17 @@ public class DummyCommunicator implements Communicator, NodeLookup { } } - public class DummySetClusterStateRequest extends SetClusterStateRequest { + public static class DummySetClusterStateRequest extends SetClusterStateRequest { - public DummySetClusterStateRequest(NodeInfo nodeInfo, ClusterState state) { + DummySetClusterStateRequest(NodeInfo nodeInfo, ClusterState state) { super(nodeInfo, state.getVersion()); } } - public class DummyActivateClusterStateVersionRequest extends ActivateClusterStateVersionRequest { + public static class DummyActivateClusterStateVersionRequest extends ActivateClusterStateVersionRequest { - public DummyActivateClusterStateVersionRequest(NodeInfo nodeInfo, int stateVersion) { + DummyActivateClusterStateVersionRequest(NodeInfo nodeInfo, int stateVersion) { super(nodeInfo, stateVersion); } @@ -57,7 +57,7 @@ public class DummyCommunicator implements Communicator, NodeLookup { private Map<Node, DummyGetNodeStateRequest> getNodeStateRequests = new TreeMap<>(); - public DummyCommunicator(List<Node> nodeList, Timer timer) { + DummyCommunicator(List<Node> nodeList, Timer timer) { this.newNodes = nodeList; this.timer = timer; } @@ -74,11 +74,11 @@ public class DummyCommunicator implements Communicator, NodeLookup { } - public boolean setNodeState(Node node, State state, String description) throws Exception { + public boolean setNodeState(Node node, State state, String description) { return setNodeState(node, new NodeState(node.getType(), state).setDescription(description), ""); } - public boolean setNodeState(Node node, NodeState state, String hostInfo) throws Exception { + public boolean setNodeState(Node node, NodeState state, String hostInfo) { DummyGetNodeStateRequest req = getNodeStateRequests.remove(node); if (req == null) { @@ -113,12 +113,12 @@ public class DummyCommunicator implements Communicator, NodeLookup { waiter.done(req); } - public void sendAllDeferredDistributorClusterStateAcks() { + void sendAllDeferredDistributorClusterStateAcks() { deferredClusterStateAcks.forEach(reqAndWaiter -> reqAndWaiter.getFirst().done(reqAndWaiter.getSecond())); deferredClusterStateAcks.clear(); } - public void sendPartialDeferredDistributorClusterStateAcks() { + void sendPartialDeferredDistributorClusterStateAcks() { if (deferredClusterStateAcks.isEmpty()) { throw new IllegalStateException("Tried to send partial ACKs with no ACKs deferred"); } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DummyVdsNode.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DummyVdsNode.java index 27415cde407..b611d573ff5 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DummyVdsNode.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DummyVdsNode.java @@ -1,8 +1,17 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.clustercontroller.core; -import com.yahoo.jrt.*; +import com.yahoo.jrt.Acceptor; +import com.yahoo.jrt.ErrorCode; +import com.yahoo.jrt.Int32Value; +import com.yahoo.jrt.ListenFailedException; +import com.yahoo.jrt.Method; +import com.yahoo.jrt.Request; +import com.yahoo.jrt.Spec; import com.yahoo.jrt.StringValue; +import com.yahoo.jrt.StringValue; +import com.yahoo.jrt.Supervisor; +import com.yahoo.jrt.Transport; import com.yahoo.jrt.slobrok.api.BackOffPolicy; import com.yahoo.jrt.slobrok.api.Register; import com.yahoo.jrt.slobrok.api.SlobrokList; @@ -11,8 +20,12 @@ import com.yahoo.vdslib.state.*; import com.yahoo.vespa.clustercontroller.core.rpc.RPCCommunicator; import com.yahoo.vespa.clustercontroller.core.rpc.RPCUtil; -import java.net.UnknownHostException; -import java.util.*; +import java.util.Iterator; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.TreeMap; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -25,7 +38,7 @@ public class DummyVdsNode { public static Logger log = Logger.getLogger(DummyVdsNode.class.getName()); - private String slobrokConnectionSpecs[]; + private String[] slobrokConnectionSpecs; private String clusterName; private NodeType type; private int index; @@ -39,15 +52,14 @@ public class DummyVdsNode { private final Timer timer; private boolean failSetSystemStateRequests = false; private boolean resetTimestampOnReconnect = false; - private long startTimestamp; private Map<Node, Long> highestStartTimestamps = new TreeMap<Node, Long>(); - public int timedOutStateReplies = 0; - public int outdatedStateReplies = 0; - public int immediateStateReplies = 0; - public int setNodeStateReplies = 0; + int timedOutStateReplies = 0; + int outdatedStateReplies = 0; + int immediateStateReplies = 0; + int setNodeStateReplies = 0; private boolean registeredInSlobrok = false; - class Req { + static class Req { Request request; long timeout; @@ -56,7 +68,7 @@ public class DummyVdsNode { this.timeout = timeout; } } - class BackOff implements BackOffPolicy { + static class BackOff implements BackOffPolicy { public void reset() {} public double get() { return 0.01; } public boolean shouldWarn(double v) { return false; } @@ -113,11 +125,7 @@ public class DummyVdsNode { nodeState.setStartTimestamp(timer.getCurrentTimeInMillis() / 1000); } - public void resetStartTimestamp() { - resetTimestampOnReconnect = true; - } - - public int getPendingNodeStateCount() { return waitingRequests.size(); } + int getPendingNodeStateCount() { return waitingRequests.size(); } public void shutdown() { messageResponder.interrupt(); @@ -125,9 +133,9 @@ public class DummyVdsNode { disconnect(); } - public int connect() throws ListenFailedException, UnknownHostException { + public int connect() throws ListenFailedException { if (resetTimestampOnReconnect) { - startTimestamp = timer.getCurrentTimeInMillis() / 1000; + long startTimestamp = timer.getCurrentTimeInMillis() / 1000; nodeState.setStartTimestamp(startTimestamp); resetTimestampOnReconnect = false; } @@ -146,23 +154,23 @@ public class DummyVdsNode { return (registeredInSlobrok && supervisor != null); } - public void registerSlobrok() { + void registerSlobrok() { register.registerName(getSlobrokName()); register.registerName(getSlobrokName() + "/default"); registeredInSlobrok = true; } - public void disconnectSlobrok() { + void disconnectSlobrok() { register.unregisterName(getSlobrokName()); register.unregisterName(getSlobrokName() + "/default"); registeredInSlobrok = false; } - public void disconnect() { disconnectImmediately(); } - public void disconnectImmediately() { disconnect(false, 0, false); } - public void disconnectBreakConnection() { disconnect(true, FleetControllerTest.timeoutMS, false); } - public void disconnectAsShutdown() { disconnect(true, FleetControllerTest.timeoutMS, true); } - public void disconnect(boolean waitForPendingNodeStateRequest, long timeoutms, boolean setStoppingStateFirst) { + void disconnect() { disconnectImmediately(); } + void disconnectImmediately() { disconnect(false, 0, false); } + void disconnectBreakConnection() { disconnect(true, FleetControllerTest.timeoutMS, false); } + void disconnectAsShutdown() { disconnect(true, FleetControllerTest.timeoutMS, true); } + private void disconnect(boolean waitForPendingNodeStateRequest, long timeoutms, boolean setStoppingStateFirst) { log.log(LogLevel.DEBUG, "Dummy node " + DummyVdsNode.this.toString() + ": Breaking connection." + (waitForPendingNodeStateRequest ? " Waiting for pending state first." : "")); if (waitForPendingNodeStateRequest) { this.waitForPendingGetNodeStateRequest(timeoutms); @@ -198,7 +206,7 @@ public class DummyVdsNode { public int getStateCommunicationVersion() { return stateCommunicationVersion; } - public void waitForSystemStateVersion(int version, long timeout) { + void waitForSystemStateVersion(int version, long timeout) { try { long startTime = System.currentTimeMillis(); while (getLatestSystemStateVersion().orElse(-1) < version) { @@ -233,7 +241,7 @@ public class DummyVdsNode { } } - public void waitForPendingGetNodeStateRequest(long timeout) { + private void waitForPendingGetNodeStateRequest(long timeout) { long startTime = System.currentTimeMillis(); long endTime = startTime + timeout; log.log(LogLevel.DEBUG, "Dummy node " + this + " waiting for pending node state request."); @@ -261,7 +269,7 @@ public class DummyVdsNode { } } - public void replyToPendingNodeStateRequests() { + void replyToPendingNodeStateRequests() { for(Req req : waitingRequests) { log.log(LogLevel.DEBUG, "Dummy node " + this + " answering pending node state request."); req.request.returnValues().add(new StringValue(nodeState.serialize())); @@ -297,7 +305,7 @@ public class DummyVdsNode { } } - public List<ClusterState> getSystemStatesReceived() { + List<ClusterState> getSystemStatesReceived() { synchronized(timer) { return clusterStateBundles.stream() .map(ClusterStateBundle::getBaselineClusterState) @@ -320,11 +328,11 @@ public class DummyVdsNode { public ClusterState getClusterState() { return Optional.ofNullable(getClusterStateBundle()) - .map(b -> b.getBaselineClusterState()) + .map(ClusterStateBundle::getBaselineClusterState) .orElse(null); } - public String getSlobrokName() { + String getSlobrokName() { return "storage/cluster." + clusterName + "/" + type + "/" + index; } @@ -415,7 +423,7 @@ public class DummyVdsNode { } } - public boolean sendGetNodeStateReply(int index) { + boolean sendGetNodeStateReply(int index) { for (Iterator<Req> it = waitingRequests.iterator(); it.hasNext(); ) { Req r = it.next(); if (r.request.parameters().size() > 2 && r.request.parameters().get(2).asInt32() == index) { @@ -488,7 +496,7 @@ public class DummyVdsNode { } } - public void failSetSystemState(boolean failSystemStateRequests) { + void failSetSystemState(boolean failSystemStateRequests) { synchronized (timer) { this.failSetSystemStateRequests = failSystemStateRequests; } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DummyVdsNodeOptions.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DummyVdsNodeOptions.java index bf63aebe022..30e7c9a8417 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DummyVdsNodeOptions.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DummyVdsNodeOptions.java @@ -9,5 +9,5 @@ import com.yahoo.vespa.clustercontroller.core.rpc.RPCCommunicator; */ public class DummyVdsNodeOptions { // 0 - 4.1, 1 - 4.2-5.0.10, 2 - 5.0.11+, 3 - 6.220+, 4 - 7.24+ - public int stateCommunicationVersion = RPCCommunicator.ACTIVATE_CLUSTER_STATE_VERSION_RPC_VERSION; + int stateCommunicationVersion = RPCCommunicator.ACTIVATE_CLUSTER_STATE_VERSION_RPC_VERSION; } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/EventLogTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/EventLogTest.java index 1dfe1249ed0..8888ab2d7d0 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/EventLogTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/EventLogTest.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.clustercontroller.core; import com.yahoo.vdslib.state.Node; import com.yahoo.vdslib.state.NodeType; -import com.yahoo.vespa.clustercontroller.utils.util.MetricReporter; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.runners.MockitoJUnitRunner; diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FakeTimer.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FakeTimer.java index d2fa0b51eed..e161c4aacb7 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FakeTimer.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FakeTimer.java @@ -13,8 +13,8 @@ import java.util.logging.Logger; */ public class FakeTimer implements Timer { private static Logger log = Logger.getLogger(FakeTimer.class.getName()); - // Don't start at zero. Clock users may initialize a 'last run' entry with 0, and we want first time to always look like a timeout - long currentTime = (long)30 * 365 * 24 * 60 * 60 * 1000; + // Don't start at zero. Clock users may initialize a 'last run' entry with 0, and we want first time to always look like a timeout + private long currentTime = (long) 30 * 365 * 24 * 60 * 60 * 1000; public synchronized long getCurrentTimeInMillis() { return currentTime; diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java index 5ecc33ae964..ae64bee6bbf 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java @@ -34,14 +34,17 @@ import org.junit.rules.TestRule; import org.junit.rules.TestWatcher; import org.junit.runner.Description; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.FileInputStream; -import java.io.IOException; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; +import java.util.TreeSet; import java.util.logging.Logger; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -52,20 +55,20 @@ import java.util.stream.Collectors; public abstract class FleetControllerTest implements Waiter { private static Logger log = Logger.getLogger(FleetControllerTest.class.getName()); - protected static final int DEFAULT_NODE_COUNT = 10; + private static final int DEFAULT_NODE_COUNT = 10; - protected Supervisor supervisor; + Supervisor supervisor; protected FakeTimer timer = new FakeTimer(); - protected boolean usingFakeTimer = false; + boolean usingFakeTimer = false; protected Slobrok slobrok; protected FleetControllerOptions options; - protected ZooKeeperTestServer zooKeeperServer; + ZooKeeperTestServer zooKeeperServer; protected FleetController fleetController; protected List<DummyVdsNode> nodes = new ArrayList<>(); - protected String testName; + private String testName; - public final static int timeoutS; - public final static int timeoutMS; + final static int timeoutS; + final static int timeoutMS; private final Waiter waiter = new Waiter.Impl(new DataRetriever() { @Override public Object getMonitor() { return timer; } @@ -83,7 +86,7 @@ public abstract class FleetControllerTest implements Waiter { timeoutMS = timeoutS * 1000; } - class BackOff implements BackOffPolicy { + static class BackOff implements BackOffPolicy { private int counter = 0; public void reset() { counter = 0; } public double get() { ++counter; return 0.01; } @@ -131,7 +134,7 @@ public abstract class FleetControllerTest implements Waiter { return opts; } - protected void setUpSystem(boolean useFakeTimer, FleetControllerOptions options) throws Exception { + void setUpSystem(boolean useFakeTimer, FleetControllerOptions options) throws Exception { log.log(LogLevel.DEBUG, "Setting up system"); slobrok = new Slobrok(); this.options = options; @@ -145,7 +148,7 @@ public abstract class FleetControllerTest implements Waiter { this.usingFakeTimer = useFakeTimer; } - protected FleetController createFleetController(boolean useFakeTimer, FleetControllerOptions options, boolean startThread, StatusPageServerInterface status) throws Exception { + FleetController createFleetController(boolean useFakeTimer, FleetControllerOptions options, boolean startThread, StatusPageServerInterface status) throws Exception { Timer timer = useFakeTimer ? this.timer : new RealTimer(); MetricUpdater metricUpdater = new MetricUpdater(new NoMetricReporter(), options.fleetControllerIndex); EventLog log = new EventLog(timer, metricUpdater); @@ -197,13 +200,14 @@ public abstract class FleetControllerTest implements Waiter { } } - protected void stopFleetController() throws Exception { + void stopFleetController() throws Exception { if (fleetController != null) { fleetController.shutdown(); fleetController = null; } } - protected void startFleetController() throws Exception { + + void startFleetController() throws Exception { if (fleetController == null) { fleetController = createFleetController(usingFakeTimer, options, true, null); } else { @@ -224,7 +228,7 @@ public abstract class FleetControllerTest implements Waiter { setUpVdsNodes(useFakeTimer, options, startDisconnected, nodeIndexes); } protected void setUpVdsNodes(boolean useFakeTimer, DummyVdsNodeOptions options, boolean startDisconnected, Set<Integer> nodeIndexes) throws Exception { - String connectionSpecs[] = new String[1]; + String[] connectionSpecs = new String[1]; connectionSpecs[0] = "tcp/localhost:" + slobrok.port(); for (int nodeIndex : nodeIndexes) { nodes.add(new DummyVdsNode(useFakeTimer ? timer : new RealTimer(), options, connectionSpecs, this.options.clusterName, true, nodeIndex)); @@ -241,7 +245,7 @@ public abstract class FleetControllerTest implements Waiter { * the returned list is twice as large as configuredNodes. */ protected List<DummyVdsNode> setUpVdsNodes(boolean useFakeTimer, DummyVdsNodeOptions options, boolean startDisconnected, List<ConfiguredNode> configuredNodes) throws Exception { - String connectionSpecs[] = new String[1]; + String[] connectionSpecs = new String[1]; connectionSpecs[0] = "tcp/localhost:" + slobrok.port(); nodes = new ArrayList<>(); final boolean distributor = true; @@ -254,15 +258,15 @@ public abstract class FleetControllerTest implements Waiter { return nodes; } - protected static Set<Integer> asIntSet(Integer... idx) { - return Arrays.asList(idx).stream().collect(Collectors.toSet()); + static Set<Integer> asIntSet(Integer... idx) { + return new HashSet<>(Arrays.asList(idx)); } - protected static Set<ConfiguredNode> asConfiguredNodes(Set<Integer> indices) { + static Set<ConfiguredNode> asConfiguredNodes(Set<Integer> indices) { return indices.stream().map(idx -> new ConfiguredNode(idx, false)).collect(Collectors.toSet()); } - protected void waitForStateExcludingNodeSubset(String expectedState, Set<Integer> excludedNodes) throws Exception { + void waitForStateExcludingNodeSubset(String expectedState, Set<Integer> excludedNodes) throws Exception { // Due to the implementation details of the test base, this.waitForState() will always // wait until all nodes added in the test have received the latest cluster state. Since we // want to entirely ignore node #6, it won't get a cluster state at all and the test will @@ -286,7 +290,7 @@ public abstract class FleetControllerTest implements Waiter { subsetWaiter.waitForState(expectedState); } - protected static Map<NodeType, Integer> transitionTimes(int milliseconds) { + static Map<NodeType, Integer> transitionTimes(int milliseconds) { Map<NodeType, Integer> maxTransitionTime = new TreeMap<>(); maxTransitionTime.put(NodeType.DISTRIBUTOR, milliseconds); maxTransitionTime.put(NodeType.STORAGE, milliseconds); @@ -330,23 +334,11 @@ public abstract class FleetControllerTest implements Waiter { public ClusterState waitForInitProgressPassed(Node n, double progress) { return waiter.waitForInitProgressPassed(n, progress); } public ClusterState waitForClusterStateIncludingNodesWithMinUsedBits(int bitcount, int nodecount) { return waiter.waitForClusterStateIncludingNodesWithMinUsedBits(bitcount, nodecount); } - protected void waitForNodeStateReported(int nodeIndex, NodeState state, int ms) { - long timeoutAtTime = System.currentTimeMillis() + ms; - while (true) { - Node node = nodes.get(nodeIndex).getNode(); - NodeState ns = fleetController.getReportedNodeState(node); - if ((ns == null && state == null) || (ns != null && state != null && ns.equals(state))) break; - if (System.currentTimeMillis() > timeoutAtTime) { - throw new IllegalStateException("Failed to find " + node + " in nodestate " + state + " before timeout of " + ms + " milliseconds."); - } - } - } - public void wait(WaitCondition c, WaitTask wt, int timeoutMS) { waiter.wait(c, wt, timeoutMS); } - public void waitForCompleteCycle() { + void waitForCompleteCycle() { fleetController.waitForCompleteCycle(timeoutMS); } @@ -354,7 +346,7 @@ public abstract class FleetControllerTest implements Waiter { verifyNodeEvents(n, exp, null); } - private class ExpectLine { + private static class ExpectLine { Pattern regex; int matchedCount = 0; int minCount = 1; @@ -398,7 +390,7 @@ public abstract class FleetControllerTest implements Waiter { * <li>The rest of the line is a regular expression. * </ul> */ - protected void verifyNodeEvents(Node n, String exp, String ignoreRegex) { + private void verifyNodeEvents(Node n, String exp, String ignoreRegex) { Pattern ignorePattern = (ignoreRegex == null ? null : Pattern.compile(ignoreRegex)); List<NodeEvent> events = fleetController.getNodeEvents(n); String[] expectLines = exp.split("\n"); @@ -475,64 +467,13 @@ public abstract class FleetControllerTest implements Waiter { } } - protected String generateHostInfo(double averagePutLatency, long operationCount) { - return ("{\n" + - " \"metrics\":\n" + - " {\n" + - " \"snapshot\":\n" + - " {\n" + - " \"from\":1335527020,\n" + - " \"to\":1335527320\n" + - " },\n" + - " \"values\":\n" + - " [\n" + - " {\n" + - " \"name\":\"vds.filestor.disk_0.allthreads.put.sum.latency\",\n" + - " \"values\":\n" + - " {\n" + - " \"average\":" + averagePutLatency + ",\n" + - " \"rate\":123.00000\n" + - " }\n" + - " },\n" + - " {\n" + - " \"name\":\"vds.filestor.disk_0.allthreads.operations\",\n" + - " \"values\":\n" + - " {\n" + - " \"count\":" + operationCount + ",\n" + - " \"rate\":3.266666\n" + - " }\n" + - " }\n" + - " ]\n" + - " }\n" + - "}\n"); - } - - protected String readFile(String filename) throws IOException { - FileInputStream stream = new FileInputStream(new File(filename)); - ByteArrayOutputStream output = new ByteArrayOutputStream(); - try { - byte [] buf = new byte[4096]; - while (true) { - int read = stream.read(buf); - if (read<=0) { - break; - } - output.write(buf, 0, read); - } - output.close(); - return output.toString(); - } finally { - stream.close(); - } - } - public static Set<ConfiguredNode> toNodes(Integer ... indexes) { - return Arrays.asList(indexes).stream() + return Arrays.stream(indexes) .map(i -> new ConfiguredNode(i, false)) .collect(Collectors.toSet()); } - protected void setWantedState(DummyVdsNode node, State state, String reason) { + void setWantedState(DummyVdsNode node, State state, String reason) { if (supervisor == null) { supervisor = new Supervisor(new Transport()); } @@ -544,10 +485,10 @@ public abstract class FleetControllerTest implements Waiter { req.parameters().add(new StringValue(ns.serialize())); connection.invokeSync(req, timeoutS); if (req.isError()) { - assertTrue("Failed to invoke setNodeState(): " + req.errorCode() + ": " + req.errorMessage(), false); + fail("Failed to invoke setNodeState(): " + req.errorCode() + ": " + req.errorMessage()); } if (!req.checkReturnTypes("s")) { - assertTrue("Failed to invoke setNodeState(): Invalid return types.", false); + fail("Failed to invoke setNodeState(): Invalid return types."); } } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/GroupAutoTakedownLiveConfigTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/GroupAutoTakedownLiveConfigTest.java index 4805257ea7a..10d7bc94ae0 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/GroupAutoTakedownLiveConfigTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/GroupAutoTakedownLiveConfigTest.java @@ -1,12 +1,9 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.clustercontroller.core; -import com.yahoo.vdslib.state.NodeType; import org.junit.Test; -import java.util.HashMap; -import java.util.Map; -import java.util.stream.Collectors; +import java.util.HashSet; import static org.junit.Assert.assertFalse; @@ -20,8 +17,7 @@ public class GroupAutoTakedownLiveConfigTest extends FleetControllerTest { { FleetControllerOptions options = defaultOptions("mycluster"); options.setStorageDistribution(DistributionBuilder.forHierarchicCluster(groupBuilder)); - options.nodes = DistributionBuilder.buildConfiguredNodes(groupBuilder.totalNodeCount()) - .stream().collect(Collectors.toSet()); + options.nodes = new HashSet<>(DistributionBuilder.buildConfiguredNodes(groupBuilder.totalNodeCount())); options.minNodeRatioPerGroup = minNodeRatio; options.maxTransitionTime = transitionTimes(0); return options; @@ -40,8 +36,7 @@ public class GroupAutoTakedownLiveConfigTest extends FleetControllerTest { private void reconfigureWithDistribution(DistributionBuilder.GroupBuilder groupBuilder) { FleetControllerOptions newOptions = this.options.clone(); - newOptions.nodes = DistributionBuilder.buildConfiguredNodes(groupBuilder.totalNodeCount()) - .stream().collect(Collectors.toSet()); + newOptions.nodes = new HashSet<>(DistributionBuilder.buildConfiguredNodes(groupBuilder.totalNodeCount())); newOptions.storageDistribution = DistributionBuilder.forHierarchicCluster(groupBuilder); updateConfigLive(newOptions); } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/GroupAutoTakedownTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/GroupAutoTakedownTest.java index 41c5922c932..7ab920f384b 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/GroupAutoTakedownTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/GroupAutoTakedownTest.java @@ -21,12 +21,8 @@ import java.util.Set; import static org.hamcrest.core.AllOf.allOf; import static org.hamcrest.core.IsCollectionContaining.hasItem; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class GroupAutoTakedownTest { diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/GroupAvailabilityCalculatorTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/GroupAvailabilityCalculatorTest.java index cef3e39937d..07bd18c3667 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/GroupAvailabilityCalculatorTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/GroupAvailabilityCalculatorTest.java @@ -22,10 +22,7 @@ public class GroupAvailabilityCalculatorTest { } } - private static GroupAvailabilityCalculator calcForFlatCluster( - final int nodeCount, - final double minNodeRatioPerGroup) - { + private static GroupAvailabilityCalculator calcForFlatCluster(int nodeCount, double minNodeRatioPerGroup) { return GroupAvailabilityCalculator.builder() .withDistribution(DistributionBuilder.forFlatCluster(nodeCount)) .withMinNodeRatioPerGroup(minNodeRatioPerGroup) diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/LeafGroupsTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/LeafGroupsTest.java index 47d2913b253..6302af5ebc9 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/LeafGroupsTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/LeafGroupsTest.java @@ -7,7 +7,7 @@ import org.junit.Test; import java.util.List; import static org.hamcrest.core.Is.is; -import static org.junit.Assert.*; +import static org.junit.Assert.assertThat; public class LeafGroupsTest { diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MaintenanceWhenPendingGlobalMergesTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MaintenanceWhenPendingGlobalMergesTest.java index 294c57d520c..6619495fdd4 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MaintenanceWhenPendingGlobalMergesTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MaintenanceWhenPendingGlobalMergesTest.java @@ -5,7 +5,7 @@ import com.yahoo.document.FixedBucketSpaces; import com.yahoo.vdslib.state.ClusterState; import org.junit.Test; -import java.util.*; +import java.util.Arrays; import static com.yahoo.vespa.clustercontroller.core.NodeStateReason.MAY_HAVE_MERGES_PENDING; import static com.yahoo.vespa.clustercontroller.core.NodeStateReason.NODE_TOO_UNSTABLE; @@ -42,7 +42,7 @@ public class MaintenanceWhenPendingGlobalMergesTest { private static class AnnotatedClusterStateBuilder extends AnnotatedClusterState.Builder { - public static AnnotatedClusterStateBuilder ofState(String stateStr) { + static AnnotatedClusterStateBuilder ofState(String stateStr) { return (AnnotatedClusterStateBuilder) new AnnotatedClusterStateBuilder().clusterState(stateStr); } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MasterElectionTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MasterElectionTest.java index 821e98800d3..f919bd867c6 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MasterElectionTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MasterElectionTest.java @@ -18,10 +18,10 @@ import org.junit.Test; import org.junit.rules.TestRule; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.concurrent.TimeoutException; import java.util.logging.Logger; +import java.util.stream.Stream; import static org.hamcrest.Matchers.greaterThan; import static org.junit.Assert.assertEquals; @@ -33,8 +33,8 @@ public class MasterElectionTest extends FleetControllerTest { private static Logger log = Logger.getLogger(MasterElectionTest.class.getName()); - protected Supervisor supervisor; - protected List<FleetController> fleetControllers = new ArrayList<>(); + private Supervisor supervisor; + private List<FleetController> fleetControllers = new ArrayList<>(); @Rule public TestRule cleanupZookeeperLogsOnSuccess = new CleanupZookeeperLogsOnSuccess(); @@ -60,9 +60,8 @@ public class MasterElectionTest extends FleetControllerTest { } } - public FleetControllerOptions adjustConfig(FleetControllerOptions o, - int fleetControllerIndex, int fleetControllerCount) throws Exception - { + private FleetControllerOptions adjustConfig(FleetControllerOptions o, + int fleetControllerIndex, int fleetControllerCount) { FleetControllerOptions options = o.clone(); options.zooKeeperSessionTimeout = defaultZkSessionTimeoutInMillis(); options.zooKeeperServerAddress = zooKeeperServer.getAddress(); @@ -73,7 +72,7 @@ public class MasterElectionTest extends FleetControllerTest { return options; } - protected void waitForZookeeperDisconnected() throws TimeoutException { + private void waitForZookeeperDisconnected() throws TimeoutException { long maxTime = System.currentTimeMillis() + timeoutMS; for(FleetController f : fleetControllers) { while (true) { @@ -86,11 +85,11 @@ public class MasterElectionTest extends FleetControllerTest { waitForCompleteCycles(); } - protected void waitForCompleteCycle(int findex) { + private void waitForCompleteCycle(int findex) { fleetControllers.get(findex).waitForCompleteCycle(timeoutMS); } - protected void waitForCompleteCycles() { + private void waitForCompleteCycles() { for (int i = 0; i < fleetControllers.size(); ++i) { waitForCompleteCycle(i); } @@ -137,7 +136,7 @@ public class MasterElectionTest extends FleetControllerTest { // Too few for there to be a master at this point for (int i=0; i<fleetControllers.size(); ++i) { if (fleetControllers.get(i).isRunning()) waitForCompleteCycle(i); - assertEquals("Fleet controller " + i, false, fleetControllers.get(i).isMaster()); + assertFalse("Fleet controller " + i, fleetControllers.get(i).isMaster()); } log.log(LogLevel.INFO, "STARTING FLEET CONTROLLER 2"); @@ -162,11 +161,11 @@ public class MasterElectionTest extends FleetControllerTest { // Too few for there to be a master at this point for (int i=0; i<fleetControllers.size(); ++i) { if (fleetControllers.get(i).isRunning()) waitForCompleteCycle(i); - assertEquals(false, fleetControllers.get(i).isMaster()); + assertFalse(fleetControllers.get(i).isMaster()); } } - protected void waitForMaster(int master) { + private void waitForMaster(int master) { log.log(LogLevel.INFO, "Entering waitForMaster"); boolean isOnlyMaster = false; for (int i=0; i < FleetControllerTest.timeoutMS; i+=100) { @@ -207,11 +206,11 @@ public class MasterElectionTest extends FleetControllerTest { this.lastState = initialState; } - public static StrictlyIncreasingVersionChecker bootstrappedWith(ClusterState initialState) { + static StrictlyIncreasingVersionChecker bootstrappedWith(ClusterState initialState) { return new StrictlyIncreasingVersionChecker(initialState); } - public void updateAndVerify(ClusterState currentState) { + void updateAndVerify(ClusterState currentState) { final ClusterState last = lastState; lastState = currentState; if (currentState.getVersion() <= last.getVersion()) { @@ -234,16 +233,16 @@ public class MasterElectionTest extends FleetControllerTest { fleetController = fleetControllers.get(0); // Required to prevent waitForStableSystem from NPE'ing waitForStableSystem(); waitForMaster(0); - Arrays.asList(0, 1, 2, 3, 4).stream().forEach(this::waitForCompleteCycle); + Stream.of(0, 1, 2, 3, 4).forEach(this::waitForCompleteCycle); StrictlyIncreasingVersionChecker checker = StrictlyIncreasingVersionChecker.bootstrappedWith( fleetControllers.get(0).getClusterState()); fleetControllers.get(0).shutdown(); waitForMaster(1); - Arrays.asList(1, 2, 3, 4).stream().forEach(this::waitForCompleteCycle); + Stream.of(1, 2, 3, 4).forEach(this::waitForCompleteCycle); checker.updateAndVerify(fleetControllers.get(1).getClusterState()); fleetControllers.get(1).shutdown(); waitForMaster(2); // Still a quorum available - Arrays.asList(2, 3, 4).stream().forEach(this::waitForCompleteCycle); + Stream.of(2, 3, 4).forEach(this::waitForCompleteCycle); checker.updateAndVerify(fleetControllers.get(2).getClusterState()); } @@ -287,7 +286,7 @@ public class MasterElectionTest extends FleetControllerTest { // Noone can be master if server is unavailable log.log(LogLevel.INFO, "Checking master status"); for (int i=0; i<fleetControllers.size(); ++i) { - assertEquals("Index " + i, false, fleetControllers.get(i).isMaster()); + assertFalse("Index " + i, fleetControllers.get(i).isMaster()); } zooKeeperServer = new ZooKeeperTestServer(); @@ -372,7 +371,7 @@ public class MasterElectionTest extends FleetControllerTest { for (int retry = 0; retry < FleetControllerTest.timeoutS * 10; ++retry) { req = new Request("getMaster"); connections.get(nodeIndex).invokeSync(req, FleetControllerTest.timeoutS); - assertEquals(req.errorMessage(), false, req.isError()); + assertFalse(req.errorMessage(), req.isError()); if (req.returnValues().get(0).asInt32() == 0 && req.returnValues().get(1).asString().equals("All 3 nodes agree that 0 is current master.")) { break; @@ -390,7 +389,7 @@ public class MasterElectionTest extends FleetControllerTest { // 5 minutes is not long enough period to wait before letting this node be master. timer.advanceTime(300 * 1000); // 5 minutes - int remainingNodes[] = { 1, 2 }; + int[] remainingNodes = { 1, 2 }; waitForMasterReason( "2 of 3 nodes agree 1 should be master, but old master cooldown period of 3600000 ms has not passed yet. To ensure it has got time to realize it is no longer master before we elect a new one, currently there is no master.", -1, connections, remainingNodes); @@ -409,7 +408,7 @@ public class MasterElectionTest extends FleetControllerTest { for (int i=0; i<FleetControllerTest.timeoutS * 10; ++i) { req = new Request("getMaster"); connections.get(1).invokeSync(req, FleetControllerTest.timeoutS); - assertEquals(req.errorMessage(), false, req.isError()); + assertFalse(req.errorMessage(), req.isError()); if (req.returnValues().get(0).asInt32() != -1) break; // We may have bad timing causing node not to have realized it is master yet } @@ -419,7 +418,7 @@ public class MasterElectionTest extends FleetControllerTest { for (int i=0; i<FleetControllerTest.timeoutS * 10; ++i) { req = new Request("getMaster"); connections.get(2).invokeSync(req, FleetControllerTest.timeoutS); - assertEquals(req.errorMessage(), false, req.isError()); + assertFalse(req.errorMessage(), req.isError()); if (req.returnValues().get(0).asInt32() != -1) break; } assertEquals(req.toString(), 1, req.returnValues().get(0).asInt32()); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NoZooKeeperTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NoZooKeeperTest.java index dff33d7a6fc..1b4837757c5 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NoZooKeeperTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NoZooKeeperTest.java @@ -2,7 +2,7 @@ package com.yahoo.vespa.clustercontroller.core; import org.junit.Test; -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; public class NoZooKeeperTest extends FleetControllerTest { @@ -15,7 +15,7 @@ public class NoZooKeeperTest extends FleetControllerTest { setUpVdsNodes(true, new DummyVdsNodeOptions()); waitForStableSystem(); - assertEquals(true, nodes.get(0).isDistributor()); + assertTrue(nodes.get(0).isDistributor()); nodes.get(0).disconnect(); waitForState("version:\\d+ distributor:10 .0.s:d storage:10"); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeSlobrokConfigurationMembershipTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeSlobrokConfigurationMembershipTest.java index 9003e369b59..f9bb93c7bb5 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeSlobrokConfigurationMembershipTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeSlobrokConfigurationMembershipTest.java @@ -2,14 +2,10 @@ package com.yahoo.vespa.clustercontroller.core; import com.yahoo.vdslib.distribution.ConfiguredNode; -import com.yahoo.vespa.clustercontroller.core.testutils.Waiter; import org.junit.Test; -import java.util.Arrays; -import java.util.List; import java.util.Set; import java.util.TreeSet; -import java.util.stream.Collectors; import static org.junit.Assert.assertTrue; diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeCheckerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeCheckerTest.java index fc466664e56..303376e7a5e 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeCheckerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeCheckerTest.java @@ -41,8 +41,8 @@ public class NodeStateChangeCheckerTest { private static final Node nodeStorage = new Node(NodeType.STORAGE, 1); private static final NodeState UP_NODE_STATE = new NodeState(NodeType.STORAGE, State.UP); - public static final NodeState MAINTENANCE_NODE_STATE = createNodeState(State.MAINTENANCE, "Orchestrator"); - public static final NodeState DOWN_NODE_STATE = createNodeState(State.DOWN, "RetireEarlyExpirer"); + private static final NodeState MAINTENANCE_NODE_STATE = createNodeState(State.MAINTENANCE, "Orchestrator"); + private static final NodeState DOWN_NODE_STATE = createNodeState(State.DOWN, "RetireEarlyExpirer"); private static NodeState createNodeState(State state, String description) { return new NodeState(NodeType.STORAGE, state).setDescription(description); @@ -129,7 +129,7 @@ public class NodeStateChangeCheckerTest { nodeDistributor, defaultAllUpClusterState(), SetUnitStateRequest.Condition.FORCE, UP_NODE_STATE, newState); assertTrue(result.settingWantedStateIsAllowed()); - assertTrue(!result.wantedStateAlreadySet()); + assertFalse(result.wantedStateAlreadySet()); } @Test 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 214ccfa1c5b..532919f05da 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 @@ -25,7 +25,6 @@ import com.yahoo.vespa.config.content.StorDistributionConfig; import org.junit.Test; import java.util.ArrayList; -import java.util.Arrays; import java.util.LinkedList; import java.util.List; import java.util.Set; @@ -34,6 +33,7 @@ import java.util.logging.Logger; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; /** @@ -43,7 +43,7 @@ public class RpcServerTest extends FleetControllerTest { public static Logger log = Logger.getLogger(RpcServerTest.class.getName()); - protected Supervisor supervisor; + private Supervisor supervisor; public void tearDown() throws Exception { if (supervisor != null) { @@ -56,7 +56,7 @@ public class RpcServerTest extends FleetControllerTest { public void testRebinding() throws Exception { startingTest("RpcServerTest::testRebinding"); Slobrok slobrok = new Slobrok(); - String slobrokConnectionSpecs[] = new String[1]; + String[] slobrokConnectionSpecs = new String[1]; slobrokConnectionSpecs[0] = "tcp/localhost:" + slobrok.port(); RpcServer server = new RpcServer(timer, new Object(), "mycluster", 0, new BackOff()); server.setSlobrokConnectionSpecs(slobrokConnectionSpecs, 0); @@ -470,7 +470,7 @@ public class RpcServerTest extends FleetControllerTest { } } - public StorDistributionConfig getDistConfig(Set<Integer> nodes) { + private StorDistributionConfig getDistConfig(Set<Integer> nodes) { List<StorDistributionConfig.Group.Nodes.Builder> nodeList = new LinkedList<>(); for (int i : nodes) { StorDistributionConfig.Group.Nodes.Builder nodeConfig = new StorDistributionConfig.Group.Nodes.Builder(); @@ -490,7 +490,7 @@ public class RpcServerTest extends FleetControllerTest { public void testSetNodeState() throws Exception { startingTest("RpcServerTest::testSetNodeState"); FleetControllerOptions options = defaultOptions("mycluster"); - Set<Integer> nodeIndexes = new TreeSet<>(Arrays.asList(new Integer[]{4, 6, 9, 10, 14, 16, 21, 22, 23, 25})); + Set<Integer> nodeIndexes = new TreeSet<>(List.of(4, 6, 9, 10, 14, 16, 21, 22, 23, 25)); options.setStorageDistribution(new Distribution(getDistConfig(nodeIndexes))); setUpFleetController(true, options); setUpVdsNodes(true, new DummyVdsNodeOptions(), false, nodeIndexes); @@ -598,7 +598,7 @@ public class RpcServerTest extends FleetControllerTest { setUpVdsNodes(true, new DummyVdsNodeOptions()); waitForStableSystem(); - assertEquals(true, nodes.get(0).isDistributor()); + assertTrue(nodes.get(0).isDistributor()); nodes.get(0).disconnect(); waitForState("version:\\d+ distributor:10 .0.s:d storage:10"); @@ -613,8 +613,8 @@ public class RpcServerTest extends FleetControllerTest { connection.invokeSync(req, timeoutS); assertEquals(req.errorMessage(), ErrorCode.NONE, req.errorCode()); assertTrue(req.toString(), req.checkReturnTypes("SS")); - String slobrok[] = req.returnValues().get(0).asStringArray().clone(); - String rpc[] = req.returnValues().get(1).asStringArray().clone(); + String[] slobrok = req.returnValues().get(0).asStringArray().clone(); + String[] rpc = req.returnValues().get(1).asStringArray().clone(); assertEquals(20, slobrok.length); assertEquals(20, rpc.length); @@ -622,13 +622,13 @@ public class RpcServerTest extends FleetControllerTest { // Verify that we can connect to all addresses returned. for (int i=0; i<20; ++i) { if (slobrok[i].equals("storage/cluster.mycluster/distributor/0")) { - if (j < 10 && !"".equals(rpc[i])) { + if (i < 10 && !"".equals(rpc[i])) { continue; } assertEquals(slobrok[i], "", rpc[i]); continue; } - assertTrue(slobrok[i], !rpc[i].equals("")); + assertNotEquals("", rpc[i]); Request req2 = new Request("getnodestate2"); req2.parameters().add(new StringValue("unknown")); Target connection2 = supervisor.connect(new Spec(rpc[i])); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/RpcVersionAutoDowngradeTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/RpcVersionAutoDowngradeTest.java index b76f1fc20bf..e7c9f3b179a 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/RpcVersionAutoDowngradeTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/RpcVersionAutoDowngradeTest.java @@ -4,7 +4,6 @@ package com.yahoo.vespa.clustercontroller.core; import com.yahoo.vdslib.distribution.ConfiguredNode; import com.yahoo.vdslib.state.NodeState; import com.yahoo.vdslib.state.State; -import com.yahoo.vespa.clustercontroller.core.testutils.StateWaiter; import org.junit.Test; import java.util.ArrayList; diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeHandlerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeHandlerTest.java index 7ef6d298885..68926b4d10d 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeHandlerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeHandlerTest.java @@ -27,14 +27,14 @@ import static org.junit.Assert.assertTrue; public class StateChangeHandlerTest { private static final Logger log = Logger.getLogger(StateChangeHandlerTest.class.getName()); - private class Config { + private static class Config { int nodeCount = 3; int stableStateTime = 1000 * 60000; int maxSlobrokDisconnectPeriod = 60000; int maxPrematureCrashes = 3; } - private class TestNodeStateOrHostInfoChangeHandler implements NodeStateOrHostInfoChangeHandler { + private static class TestNodeStateOrHostInfoChangeHandler implements NodeStateOrHostInfoChangeHandler { LinkedList<String> events = new LinkedList<>(); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeTest.java index ca246b3549f..016cce9580e 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeTest.java @@ -1,9 +1,14 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.clustercontroller.core; -import com.yahoo.jrt.*; +import com.yahoo.jrt.Supervisor; +import com.yahoo.jrt.Transport; import com.yahoo.vdslib.distribution.ConfiguredNode; -import com.yahoo.vdslib.state.*; +import com.yahoo.vdslib.state.ClusterState; +import com.yahoo.vdslib.state.Node; +import com.yahoo.vdslib.state.NodeState; +import com.yahoo.vdslib.state.NodeType; +import com.yahoo.vdslib.state.State; import com.yahoo.vespa.clustercontroller.core.database.DatabaseHandler; import com.yahoo.vespa.clustercontroller.core.database.ZooKeeperDatabaseFactory; import com.yahoo.vespa.clustercontroller.core.testutils.StateWaiter; @@ -18,7 +23,10 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import java.time.Duration; -import java.util.*; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.logging.Logger; public class StateChangeTest extends FleetControllerTest { @@ -83,7 +91,6 @@ public class StateChangeTest extends FleetControllerTest { } assertEquals(correct, actual); - } private static List<ConfiguredNode> createNodes(int count) { @@ -184,7 +191,7 @@ public class StateChangeTest extends FleetControllerTest { ctrl.tick(); String desc = ctrl.getReportedNodeState(new Node(NodeType.DISTRIBUTOR, 0)).getDescription(); - assertTrue(desc, desc.indexOf("Closed at other end") != -1); + assertTrue(desc, desc.contains("Closed at other end")); assertEquals("version:4 distributor:10 .0.s:d storage:10", ctrl.getSystemState().toString()); @@ -201,7 +208,7 @@ public class StateChangeTest extends FleetControllerTest { assert(!ctrl.getReportedNodeState(new Node(NodeType.DISTRIBUTOR, 0)).hasDescription()); desc = ctrl.getReportedNodeState(new Node(NodeType.STORAGE, 0)).getDescription(); - assertTrue(desc, desc.indexOf("Closed at other end") != -1); + assertTrue(desc, desc.contains("Closed at other end")); timer.advanceTime(options.maxTransitionTime.get(NodeType.STORAGE) + 1); @@ -210,7 +217,7 @@ public class StateChangeTest extends FleetControllerTest { assertEquals("version:6 distributor:10 .0.t:12345678 storage:10 .0.s:d", ctrl.getSystemState().toString()); desc = ctrl.getReportedNodeState(new Node(NodeType.STORAGE, 0)).getDescription(); - assertTrue(desc, desc.indexOf("Closed at other end") != -1); + assertTrue(desc, desc.contains("Closed at other end")); timer.advanceTime(1000); @@ -249,7 +256,7 @@ public class StateChangeTest extends FleetControllerTest { assertEquals(1, ctrl.getCluster().getNodeInfo(new Node(NodeType.STORAGE, 0)).getPrematureCrashCount()); } - public void tick(int timeMs) throws Exception { + private void tick(int timeMs) throws Exception { timer.advanceTime(timeMs); ctrl.tick(); } @@ -272,8 +279,8 @@ public class StateChangeTest extends FleetControllerTest { ctrl.tick(); String desc = ctrl.getReportedNodeState(new Node(NodeType.DISTRIBUTOR, 0)).getDescription(); - assertTrue(desc, desc.indexOf("Received signal 15 (SIGTERM - Termination signal)") != -1 - || desc.indexOf("controlled shutdown") != -1); + assertTrue(desc, desc.contains("Received signal 15 (SIGTERM - Termination signal)") + || desc.contains("controlled shutdown")); tick(1000); @@ -286,15 +293,15 @@ public class StateChangeTest extends FleetControllerTest { assert(!ctrl.getReportedNodeState(new Node(NodeType.DISTRIBUTOR, 0)).hasDescription()); desc = ctrl.getReportedNodeState(new Node(NodeType.STORAGE, 0)).getDescription(); - assertTrue(desc, desc.indexOf("Received signal 15 (SIGTERM - Termination signal)") != -1 - || desc.indexOf("controlled shutdown") != -1); + assertTrue(desc, desc.contains("Received signal 15 (SIGTERM - Termination signal)") + || desc.contains("controlled shutdown")); tick(options.maxTransitionTime.get(NodeType.STORAGE) + 1); assertEquals("version:6 distributor:10 storage:10 .0.s:d", ctrl.getSystemState().toString()); desc = ctrl.getReportedNodeState(new Node(NodeType.STORAGE, 0)).getDescription(); - assertTrue(desc, desc.indexOf("Received signal 15 (SIGTERM - Termination signal)") != -1 - || desc.indexOf("controlled shutdown") != -1); + assertTrue(desc, desc.contains("Received signal 15 (SIGTERM - Termination signal)") + || desc.contains("controlled shutdown")); communicator.setNodeState(new Node(NodeType.STORAGE, 0), State.UP, ""); @@ -381,7 +388,7 @@ public class StateChangeTest extends FleetControllerTest { assertEquals("version:4 distributor:10 storage:10 .6.s:m", ctrl.getSystemState().toString()); NodeState ns = ctrl.getReportedNodeState(new Node(NodeType.STORAGE, 6)); - assertTrue(ns.toString(), ns.getDescription().indexOf("Connection error: Closed at other end") != -1); + assertTrue(ns.toString(), ns.getDescription().contains("Connection error: Closed at other end")); tick(1000); @@ -441,7 +448,7 @@ public class StateChangeTest extends FleetControllerTest { assertEquals("version:4 distributor:10 storage:10 .6.s:m", ctrl.getSystemState().toString()); NodeState ns = ctrl.getReportedNodeState(new Node(NodeType.STORAGE, 6)); - assertTrue(ns.toString(), ns.getDescription().indexOf("Connection error: Closed at other end") != -1); + assertTrue(ns.toString(), ns.getDescription().contains("Connection error: Closed at other end")); tick(1000); @@ -926,7 +933,7 @@ public class StateChangeTest extends FleetControllerTest { * Class for testing states of all nodes. Will fail in constructor with * debug message on non-expected results. */ - abstract class StateMessageChecker { + abstract static class StateMessageChecker { StateMessageChecker(final List<DummyVdsNode> nodes) { for (final DummyVdsNode node : nodes) { final List<ClusterState> states = node.getSystemStatesReceived(); @@ -992,8 +999,8 @@ public class StateChangeTest extends FleetControllerTest { setUpVdsNodes(true, new DummyVdsNodeOptions(), true); - for (int i=0; i<nodes.size(); ++i) { - nodes.get(i).connect(); + for (DummyVdsNode node : nodes) { + node.connect(); } // Marking one node as 'initializing' improves testing of state later on. nodes.get(3).setNodeState(State.INITIALIZING); @@ -1088,7 +1095,7 @@ public class StateChangeTest extends FleetControllerTest { assertEquals("version:6 bits:17 distributor:10 storage:10", ctrl.getSystemState().toString()); } - private void setMinUsedBitsForAllNodes(int bits) throws Exception { + private void setMinUsedBitsForAllNodes(int bits) { for (int i = 0; i < 10; ++i) { communicator.setNodeState(new Node(NodeType.STORAGE, i), new NodeState(NodeType.STORAGE, State.UP).setMinUsedBits(bits), ""); } @@ -1106,7 +1113,7 @@ public class StateChangeTest extends FleetControllerTest { fleetController.addSystemStateListener(waiter); // Simulate netsplit. Take node down without node booting - assertEquals(true, nodes.get(0).isDistributor()); + assertTrue(nodes.get(0).isDistributor()); nodes.get(0).disconnectImmediately(); waiter.waitForState("version:\\d+ distributor:10 .0.s:d storage:10", timeoutMS); @@ -1133,13 +1140,13 @@ public class StateChangeTest extends FleetControllerTest { } else { for (ConfiguredNode i : options.nodes) { Node nodeId = new Node(NodeType.STORAGE, i.index()); - assertTrue(nodeId.toString(), lastState.getNodeState(nodeId).getStartTimestamp() == 0); + assertEquals(nodeId.toString(), 0, lastState.getNodeState(nodeId).getStartTimestamp()); } } for (ConfiguredNode i : options.nodes) { Node nodeId = new Node(NodeType.DISTRIBUTOR, i.index()); - assertTrue(nodeId.toString(), lastState.getNodeState(nodeId).getStartTimestamp() == 0); + assertEquals(nodeId.toString(), 0, lastState.getNodeState(nodeId).getStartTimestamp()); } } } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateGatherTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateGatherTest.java index 007bda04f4e..398c1c16473 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateGatherTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateGatherTest.java @@ -13,7 +13,7 @@ public class StateGatherTest extends FleetControllerTest { public static Logger log = Logger.getLogger(StateGatherTest.class.getName()); - public String getGetNodeStateReplyCounts(DummyVdsNode node) { + private String getGetNodeStateReplyCounts(DummyVdsNode node) { StringBuilder sb = new StringBuilder(); sb.append("timedout ").append(node.timedOutStateReplies) .append(", outdated ").append(node.outdatedStateReplies) @@ -33,7 +33,7 @@ public class StateGatherTest extends FleetControllerTest { options.nodeStateRequestTimeoutEarliestPercentage = 80; options.nodeStateRequestTimeoutLatestPercentage = 80; setUpFleetController(true, options); - String connectionSpecs[] = new String[1]; + String[] connectionSpecs = new String[1]; connectionSpecs[0] = "tcp/localhost:" + slobrok.port(); DummyVdsNodeOptions dummyOptions = new DummyVdsNodeOptions(); DummyVdsNode dnode = new DummyVdsNode(timer, dummyOptions, connectionSpecs, this.options.clusterName, true, 0); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateVersionTrackerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateVersionTrackerTest.java index 95b699d4de4..83bdbfa0213 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateVersionTrackerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateVersionTrackerTest.java @@ -9,9 +9,7 @@ import com.yahoo.vdslib.state.State; import com.yahoo.vespa.clustercontroller.core.hostinfo.HostInfo; import org.junit.Test; -import java.text.ParseException; import java.util.Arrays; -import java.util.Optional; import static org.hamcrest.core.IsEqual.equalTo; import static org.hamcrest.core.Is.is; diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StatusPagesTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StatusPagesTest.java index fc18a1b0b8b..f761538cf1e 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StatusPagesTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StatusPagesTest.java @@ -19,8 +19,8 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStreamWriter; -import java.io.UnsupportedEncodingException; import java.net.Socket; +import java.nio.charset.StandardCharsets; import java.text.DateFormat; import java.text.SimpleDateFormat; import java.util.Date; @@ -225,7 +225,6 @@ public class StatusPagesTest extends FleetControllerTest { assertTrue(content, content.contains("REPORTED")); assertTrue(content, content.contains("Altered node state in cluster state from")); //System.err.println(sb.toString()); - } @Test @@ -283,7 +282,7 @@ public class StatusPagesTest extends FleetControllerTest { private static class DummyRequestHandler implements StatusPageServer.RequestHandler { private String returnData; - public DummyRequestHandler(String returnData) { + DummyRequestHandler(String returnData) { this.returnData = returnData; } @@ -301,11 +300,7 @@ public class StatusPagesTest extends FleetControllerTest { if (handler == null) { return null; } - try { - return handler.handle(httpRequest).getOutputStream().toString("UTF-8"); - } catch (UnsupportedEncodingException e) { - return "<ERROR>"; - } + return handler.handle(httpRequest).getOutputStream().toString(StandardCharsets.UTF_8); } @Test @@ -322,7 +317,7 @@ public class StatusPagesTest extends FleetControllerTest { assertNull(invokeHandler(router, "/blarg")); } - public String[] getResponseParts(String response) { + private String[] getResponseParts(String response) { int offset = response.indexOf("\r\n\r\n"); if (offset == -1) { throw new IllegalStateException("No HTTP header delimiter found"); @@ -333,18 +328,6 @@ public class StatusPagesTest extends FleetControllerTest { }; } - private String getHeaderValue(String header, String name) { - int offset = header.indexOf(name + ": "); - if (offset == -1) { - throw new IllegalStateException("No HTTP header found for " + name); - } - int end = header.indexOf("\r\n", offset); - if (end == -1) { - throw new IllegalStateException("No EOL found for " + name); - } - return header.substring(offset + name.length() + 2, end); - } - @Test public void testStateServing() throws Exception { startingTest("StatusPagesTest::testStateServing()"); @@ -369,7 +352,7 @@ public class StatusPagesTest extends FleetControllerTest { "}"; assertEquals(expected, body); // Check that it actually parses - JSONObject o = new JSONObject(expected); + new JSONObject(expected); } } 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 f99df6a25b2..182194fcc5a 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 @@ -1,7 +1,10 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.clustercontroller.core; -import com.yahoo.vdslib.state.*; +import com.yahoo.vdslib.state.Node; +import com.yahoo.vdslib.state.NodeState; +import com.yahoo.vdslib.state.NodeType; +import com.yahoo.vdslib.state.State; import com.yahoo.vespa.clustercontroller.core.database.DatabaseHandler; import com.yahoo.vespa.clustercontroller.core.listeners.NodeAddedOrRemovedListener; import com.yahoo.vespa.clustercontroller.core.listeners.NodeStateOrHostInfoChangeHandler; @@ -11,7 +14,6 @@ import org.mockito.ArgumentCaptor; import java.util.stream.Stream; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNull; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; @@ -143,7 +145,7 @@ public class SystemStateBroadcasterTest { } private static class MockSetClusterStateRequest extends SetClusterStateRequest { - public MockSetClusterStateRequest(NodeInfo nodeInfo, int clusterStateVersion) { + MockSetClusterStateRequest(NodeInfo nodeInfo, int clusterStateVersion) { super(nodeInfo, clusterStateVersion); } } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ZooKeeperTestServer.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ZooKeeperTestServer.java index 72b89dad85d..34c8fafa702 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ZooKeeperTestServer.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ZooKeeperTestServer.java @@ -20,7 +20,7 @@ public class ZooKeeperTestServer { private static final String DIR_PREFIX = "test_fltctrl_zk"; private static final String DIR_POSTFIX = "sdir"; - public ZooKeeperTestServer() throws IOException { + ZooKeeperTestServer() throws IOException { this(0); } @@ -42,15 +42,15 @@ public class ZooKeeperTestServer { } } - public static ZooKeeperTestServer createWithFixedPort(int port) throws IOException { + static ZooKeeperTestServer createWithFixedPort(int port) throws IOException { return new ZooKeeperTestServer(port); } - public int getPort() { + private int getPort() { return factory.getLocalPort(); } - public String getAddress() { + String getAddress() { return HostName.getLocalhost() + ":" + getPort(); } @@ -64,7 +64,7 @@ public class ZooKeeperTestServer { factory.shutdown(); } - public void delete(File f) { + private void delete(File f) { if (f.isDirectory()) { for (File file : f.listFiles()) { delete(file); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/hostinfo/HostInfoTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/hostinfo/HostInfoTest.java index b96f5282971..8804e9a9c96 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/hostinfo/HostInfoTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/hostinfo/HostInfoTest.java @@ -28,7 +28,7 @@ public class HostInfoTest { } @Test - public void testEmptyJson() throws IOException { + public void testEmptyJson() { HostInfo hostInfo = HostInfo.createHostInfo("{}"); assertThat(hostInfo.getVtag().getVersionOrNull(), is(nullValue())); assertThat(hostInfo.getDistributor().getStorageNodes().size(), is(0)); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/ClusterEventWithDescription.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/ClusterEventWithDescription.java index e91627e8479..f45120a2b71 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/ClusterEventWithDescription.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/ClusterEventWithDescription.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.clustercontroller.core.matchers; import com.yahoo.vespa.clustercontroller.core.ClusterEvent; -import com.yahoo.vespa.clustercontroller.core.NodeEvent; import org.hamcrest.Description; import org.hamcrest.Factory; import org.mockito.ArgumentMatcher; @@ -10,7 +9,7 @@ import org.mockito.ArgumentMatcher; public class ClusterEventWithDescription extends ArgumentMatcher<ClusterEvent> { private final String expected; - public ClusterEventWithDescription(String expected) { + private ClusterEventWithDescription(String expected) { this.expected = expected; } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/EventForNode.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/EventForNode.java index 7b806982313..6c8610db3ac 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/EventForNode.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/EventForNode.java @@ -10,7 +10,7 @@ import org.mockito.ArgumentMatcher; public class EventForNode extends ArgumentMatcher<NodeEvent> { private final Node expected; - EventForNode(Node expected) { + private EventForNode(Node expected) { this.expected = expected; } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/EventTimeIs.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/EventTimeIs.java index 2ffdfbca687..c35f3f95a5b 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/EventTimeIs.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/EventTimeIs.java @@ -9,7 +9,7 @@ import org.mockito.ArgumentMatcher; public class EventTimeIs extends ArgumentMatcher<Event> { private final long expected; - public EventTimeIs(long expected) { + private EventTimeIs(long expected) { this.expected = expected; } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/EventTypeIs.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/EventTypeIs.java index e54abfc99b0..cbb6dc47364 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/EventTypeIs.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/EventTypeIs.java @@ -8,7 +8,7 @@ import org.mockito.ArgumentMatcher; public class EventTypeIs extends ArgumentMatcher<NodeEvent> { private final NodeEvent.Type expected; - public EventTypeIs(NodeEvent.Type expected) { + private EventTypeIs(NodeEvent.Type expected) { this.expected = expected; } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/HasStateReasonForNode.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/HasStateReasonForNode.java index bbedba4114e..11166ae6ed1 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/HasStateReasonForNode.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/HasStateReasonForNode.java @@ -13,7 +13,7 @@ public class HasStateReasonForNode extends ArgumentMatcher<Map<Node, NodeStateRe private final Node node; private final NodeStateReason expected; - public HasStateReasonForNode(Node node, NodeStateReason expected) { + private HasStateReasonForNode(Node node, NodeStateReason expected) { this.node = node; this.expected = expected; } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/NodeEventForBucketSpace.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/NodeEventForBucketSpace.java index d13f91609f4..8e7f8b27c6a 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/NodeEventForBucketSpace.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/NodeEventForBucketSpace.java @@ -11,7 +11,7 @@ import java.util.Optional; public class NodeEventForBucketSpace extends ArgumentMatcher<NodeEvent> { private final Optional<String> bucketSpace; - public NodeEventForBucketSpace(Optional<String> bucketSpace) { + private NodeEventForBucketSpace(Optional<String> bucketSpace) { this.bucketSpace = bucketSpace; } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/NodeEventWithDescription.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/NodeEventWithDescription.java index 38d3d28ab62..00f043695da 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/NodeEventWithDescription.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/matchers/NodeEventWithDescription.java @@ -9,7 +9,7 @@ import org.mockito.ArgumentMatcher; public class NodeEventWithDescription extends ArgumentMatcher<NodeEvent> { private final String expected; - public NodeEventWithDescription(String expected) { + private NodeEventWithDescription(String expected) { this.expected = expected; } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/ClusterControllerMock.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/ClusterControllerMock.java index 472301e1dfc..a37b3abbd88 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/ClusterControllerMock.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/ClusterControllerMock.java @@ -11,13 +11,13 @@ import com.yahoo.vespa.clustercontroller.core.listeners.NodeStateOrHostInfoChang public class ClusterControllerMock implements RemoteClusterControllerTaskScheduler { public RemoteClusterControllerTask.Context context = new RemoteClusterControllerTask.Context(); - public int fleetControllerIndex; - public Integer fleetControllerMaster; - public StringBuilder events = new StringBuilder(); + private int fleetControllerIndex; + Integer fleetControllerMaster; + private StringBuilder events = new StringBuilder(); - public ClusterControllerMock(ContentCluster cluster, ClusterState state, - ClusterStateBundle publishedClusterStateBundle, - int fcIndex, Integer fcMaster) { + ClusterControllerMock(ContentCluster cluster, ClusterState state, + ClusterStateBundle publishedClusterStateBundle, + int fcIndex, Integer fcMaster) { this.fleetControllerIndex = fcIndex; this.fleetControllerMaster = fcMaster; context.cluster = cluster; diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/NotMasterTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/NotMasterTest.java index 6a2c14ae028..4ec5893c1e7 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/NotMasterTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/NotMasterTest.java @@ -9,6 +9,7 @@ import org.junit.Test; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; public class NotMasterTest extends StateRestApiTest { @@ -29,39 +30,39 @@ public class NotMasterTest extends StateRestApiTest { // Recursive cluster list does not work try{ restAPI.getState(new StateRequest("", 1)); - assertTrue(false); + fail(); } catch (UnknownMasterException e) { assertTrue(e.getMessage(), e.getMessage().contains("No known master cluster controller")); } // Other requests does not work either try{ restAPI.getState(new StateRequest("music", 0)); - assertTrue(false); + fail(); } catch (UnknownMasterException e) { assertTrue(e.getMessage(), e.getMessage().contains("No known master cluster controller")); } try{ restAPI.getState(new StateRequest("music/storage", 0)); - assertTrue(false); + fail(); } catch (UnknownMasterException e) { assertTrue(e.getMessage(), e.getMessage().contains("No known master cluster controller")); } try{ restAPI.getState(new StateRequest("music/storage/1", 0)); - assertTrue(false); + fail(); } catch (UnknownMasterException e) { assertTrue(e.getMessage(), e.getMessage().contains("No known master cluster controller")); } try{ restAPI.getState(new StateRequest("music/storage/1/0", 0)); - assertTrue(false); + fail(); } catch (UnknownMasterException e) { assertTrue(e.getMessage(), e.getMessage().contains("No known master cluster controller")); } try{ restAPI.setUnitState(new SetNodeStateTest.SetUnitStateRequestImpl("music/storage/1") .setNewState("user", "down", "test")); - assertTrue(false); + fail(); } catch (UnknownMasterException e) { assertTrue(e.getMessage(), e.getMessage().contains("No known master cluster controller")); } @@ -85,53 +86,53 @@ public class NotMasterTest extends StateRestApiTest { // Recursive cluster list does not work try{ restAPI.getState(new StateRequest("", 1)); - assertTrue(false); + fail(); } catch (OtherMasterException e) { assertTrue(e.getMessage(), e.getMessage().contains("Cluster controller not master. Use master at otherhost:" + getDefaults().vespaWebServicePort() + ".")); - assertTrue(e.getHost().equals("otherhost")); - assertTrue(e.getPort() == getDefaults().vespaWebServicePort()); + assertEquals("otherhost", e.getHost()); + assertEquals(e.getPort(), getDefaults().vespaWebServicePort()); } // Other requests does not work either try{ restAPI.getState(new StateRequest("music", 0)); - assertTrue(false); + fail(); } catch (OtherMasterException e) { assertTrue(e.getMessage(), e.getMessage().contains("Cluster controller not master. Use master at otherhost:" + getDefaults().vespaWebServicePort() + ".")); - assertTrue(e.getHost().equals("otherhost")); - assertTrue(e.getPort() == getDefaults().vespaWebServicePort()); + assertEquals("otherhost", e.getHost()); + assertEquals(e.getPort(), getDefaults().vespaWebServicePort()); } try{ restAPI.getState(new StateRequest("music/storage", 0)); - assertTrue(false); + fail(); } catch (OtherMasterException e) { assertTrue(e.getMessage(), e.getMessage().contains("Cluster controller not master. Use master at otherhost:" + getDefaults().vespaWebServicePort() + ".")); - assertTrue(e.getHost().equals("otherhost")); - assertTrue(e.getPort() == getDefaults().vespaWebServicePort()); + assertEquals("otherhost", e.getHost()); + assertEquals(e.getPort(), getDefaults().vespaWebServicePort()); } try{ restAPI.getState(new StateRequest("music/storage/1", 0)); - assertTrue(false); + fail(); } catch (OtherMasterException e) { assertTrue(e.getMessage(), e.getMessage().contains("Cluster controller not master. Use master at otherhost:" + getDefaults().vespaWebServicePort() + ".")); - assertTrue(e.getHost().equals("otherhost")); - assertTrue(e.getPort() == getDefaults().vespaWebServicePort()); + assertEquals("otherhost", e.getHost()); + assertEquals(e.getPort(), getDefaults().vespaWebServicePort()); } try{ restAPI.getState(new StateRequest("music/storage/1/0", 0)); - assertTrue(false); + fail(); } catch (OtherMasterException e) { assertTrue(e.getMessage(), e.getMessage().contains("Cluster controller not master. Use master at otherhost:" + getDefaults().vespaWebServicePort() + ".")); - assertTrue(e.getHost().equals("otherhost")); - assertTrue(e.getPort() == getDefaults().vespaWebServicePort()); + assertEquals("otherhost", e.getHost()); + assertEquals(e.getPort(), getDefaults().vespaWebServicePort()); } try{ restAPI.setUnitState(new SetNodeStateTest.SetUnitStateRequestImpl("music/storage/1") .setNewState("user", "down", "test")); - assertTrue(false); + fail(); } catch (OtherMasterException e) { assertTrue(e.getMessage(), e.getMessage().contains("Cluster controller not master. Use master at otherhost:" + getDefaults().vespaWebServicePort() + ".")); - assertTrue(e.getHost().equals("otherhost")); - assertTrue(e.getPort() == getDefaults().vespaWebServicePort()); + assertEquals("otherhost", e.getHost()); + assertEquals(e.getPort(), getDefaults().vespaWebServicePort()); } } } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/RequestTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/RequestTest.java index f76cfd13af1..78c5f3380be 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/RequestTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/RequestTest.java @@ -5,12 +5,13 @@ import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.InternalFailu import org.junit.Test; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; public class RequestTest { @Test public void testGetResultBeforeCompletion() { - Request<String> r = new Request<String>(Request.MasterState.MUST_BE_MASTER) { + Request<String> r = new Request<>(Request.MasterState.MUST_BE_MASTER) { @Override public String calculateResult(Context context) { return "foo"; @@ -18,18 +19,18 @@ public class RequestTest { }; try{ r.getResult(); - assertTrue(false); + fail(); } catch (InternalFailure e) { } catch (Exception e) { - assertTrue(false); + fail(); } r.notifyCompleted(); try{ r.getResult(); - assertTrue(false); + fail(); } catch (InternalFailure e) { } catch (Exception e) { - assertTrue(false); + fail(); } } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.java index 6cf4b7989e7..6882b083ec9 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.java @@ -32,6 +32,7 @@ import static org.hamcrest.core.StringContains.containsString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyBoolean; import static org.mockito.Mockito.mock; @@ -49,21 +50,21 @@ public class SetNodeStateTest extends StateRestApiTest { private TimeBudget timeBudget = TimeBudget.fromNow(Clock.systemUTC(), Duration.ofSeconds(10)); private boolean probe = false; - public SetUnitStateRequestImpl(String req) { + SetUnitStateRequestImpl(String req) { super(req, 0); } - public SetUnitStateRequestImpl setCondition(Condition condition) { + SetUnitStateRequestImpl setCondition(Condition condition) { this.condition = condition; return this; } - public SetUnitStateRequestImpl setResponseWait(ResponseWait responseWait) { + SetUnitStateRequestImpl setResponseWait(ResponseWait responseWait) { this.responseWait = responseWait; return this; } - public SetUnitStateRequestImpl setNewState( + SetUnitStateRequestImpl setNewState( final String type, final String state, final String reason) { @@ -246,7 +247,7 @@ public class SetNodeStateTest extends StateRestApiTest { try{ restAPI.setUnitState(new SetUnitStateRequestImpl( "").setNewState("user", "down", "testing")); - assertTrue(false); + fail(); } catch (OperationNotSupportedForUnitException e) { assertTrue(e.getMessage(), e.getMessage().contains(wrongUnitMessage)); } @@ -256,7 +257,7 @@ public class SetNodeStateTest extends StateRestApiTest { try{ restAPI.setUnitState(new SetUnitStateRequestImpl( "music/distributor").setNewState("user", "down", "testing")); - assertTrue(false); + fail(); } catch (OperationNotSupportedForUnitException e) { assertTrue(e.getMessage(), e.getMessage().contains(wrongUnitMessage)); } @@ -266,7 +267,7 @@ public class SetNodeStateTest extends StateRestApiTest { try{ restAPI.setUnitState(new SetUnitStateRequestImpl( "music/storage/1/0").setNewState("user", "down", "testing")); - assertTrue(false); + fail(); } catch (OperationNotSupportedForUnitException e) { assertTrue(e.getMessage(), e.getMessage().contains(wrongUnitMessage)); } @@ -278,37 +279,37 @@ public class SetNodeStateTest extends StateRestApiTest { try{ restAPI.setUnitState(new SetUnitStateRequestImpl( "foo").setNewState("user", "down", "testing")); - assertTrue(false); + fail(); } catch (MissingUnitException e) { } try{ restAPI.setUnitState(new SetUnitStateRequestImpl( "music/content").setNewState("user", "down", "testing")); - assertTrue(false); + fail(); } catch (MissingUnitException e) { } try{ restAPI.setUnitState(new SetUnitStateRequestImpl( "music/storage/bah").setNewState("user", "down", "testing")); - assertTrue(false); + fail(); } catch (MissingUnitException e) { } try{ restAPI.setUnitState(new SetUnitStateRequestImpl( "music/storage/10").setNewState("user", "down", "testing")); - assertTrue(false); + fail(); } catch (MissingUnitException e) { } try{ restAPI.setUnitState(new SetUnitStateRequestImpl( "music/storage/1/0/1").setNewState("user", "down", "testing")); - assertTrue(false); + fail(); } catch (MissingUnitException e) { } try{ restAPI.setUnitState(new SetUnitStateRequestImpl( "music/storage/1/bar").setNewState("user", "down", "testing")); - assertTrue(false); + fail(); } catch (MissingUnitException e) { } } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/StateRestApiTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/StateRestApiTest.java index 5160ecad8b3..faebbf8755d 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/StateRestApiTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/StateRestApiTest.java @@ -15,17 +15,17 @@ import java.util.*; // TODO: Author public abstract class StateRestApiTest { - protected ClusterControllerMock books; - protected ClusterControllerMock music; - protected StateRestAPI restAPI; - protected JsonWriter jsonWriter = new JsonWriter(); - protected Map<Integer, ClusterControllerStateRestAPI.Socket> ccSockets; + private ClusterControllerMock books; + ClusterControllerMock music; + StateRestAPI restAPI; + JsonWriter jsonWriter = new JsonWriter(); + Map<Integer, ClusterControllerStateRestAPI.Socket> ccSockets; public static class StateRequest implements UnitStateRequest { private String[] path; private int recursive; - public StateRequest(String req, int recursive) { + StateRequest(String req, int recursive) { path = req.isEmpty() ? new String[0] : req.split("/"); this.recursive = recursive; } @@ -83,7 +83,7 @@ public abstract class StateRestApiTest { }, ccSockets); } - protected void initializeCluster(ContentCluster cluster, Collection<ConfiguredNode> nodes) { + private void initializeCluster(ContentCluster cluster, Collection<ConfiguredNode> nodes) { for (ConfiguredNode configuredNode : nodes) { for (NodeType type : NodeType.getTypes()) { NodeState reported = new NodeState(type, State.UP); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/rpc/RPCCommunicatorTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/rpc/RPCCommunicatorTest.java index 9eb98f4f045..ac4e96c8c92 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/rpc/RPCCommunicatorTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/rpc/RPCCommunicatorTest.java @@ -6,7 +6,16 @@ import com.yahoo.vdslib.state.Node; import com.yahoo.vdslib.state.NodeState; import com.yahoo.vdslib.state.NodeType; import com.yahoo.vdslib.state.State; -import com.yahoo.vespa.clustercontroller.core.*; +import com.yahoo.vespa.clustercontroller.core.ActivateClusterStateVersionRequest; +import com.yahoo.vespa.clustercontroller.core.ClusterFixture; +import com.yahoo.vespa.clustercontroller.core.ClusterStateBundle; +import com.yahoo.vespa.clustercontroller.core.ClusterStateBundleUtil; +import com.yahoo.vespa.clustercontroller.core.Communicator; +import com.yahoo.vespa.clustercontroller.core.FakeTimer; +import com.yahoo.vespa.clustercontroller.core.FleetControllerOptions; +import com.yahoo.vespa.clustercontroller.core.NodeInfo; +import com.yahoo.vespa.clustercontroller.core.SetClusterStateRequest; +import com.yahoo.vespa.clustercontroller.core.Timer; import org.junit.Test; import org.mockito.Mockito; @@ -20,22 +29,24 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.core.Is.is; import static org.hamcrest.core.IsNot.not; import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.*; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyDouble; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; public class RPCCommunicatorTest { - public static final int NODE_STATE_REQUEST_TIMEOUT_INTERVAL_MAX_MS = 10000; - public static final int NODE_STATE_REQUEST_TIMEOUT_INTERVAL_START_PERCENTAGE = 80; - public static final int NODE_STATE_REQUEST_TIMEOUT_INTERVAL_STOP_PERCENTAGE = 95; - public static final int INDEX = 0; - public static final int TEST_ITERATIONS = 500; - public static final int ROUNDTRIP_LATENCY_SECONDS = 2000; + private static final int NODE_STATE_REQUEST_TIMEOUT_INTERVAL_MAX_MS = 10000; + private static final int NODE_STATE_REQUEST_TIMEOUT_INTERVAL_START_PERCENTAGE = 80; + private static final int NODE_STATE_REQUEST_TIMEOUT_INTERVAL_STOP_PERCENTAGE = 95; + private static final int INDEX = 0; + private static final int TEST_ITERATIONS = 500; + private static final int ROUNDTRIP_LATENCY_SECONDS = 2000; @Test - public void testGenerateNodeStateRequestTimeoutMs() throws Exception { + public void testGenerateNodeStateRequestTimeoutMs() { final RPCCommunicator communicator = new RPCCommunicator( RPCCommunicator.createRealSupervisor(), null /* Timer */, @@ -62,7 +73,7 @@ public class RPCCommunicatorTest { } @Test - public void testGenerateNodeStateRequestTimeoutMsWithUpdates() throws Exception { + public void testGenerateNodeStateRequestTimeoutMsWithUpdates() { final RPCCommunicator communicator = new RPCCommunicator(RPCCommunicator.createRealSupervisor(), null /* Timer */, INDEX, 1, 1, 100, 0); FleetControllerOptions fleetControllerOptions = new FleetControllerOptions(null /*clustername*/); fleetControllerOptions.nodeStateRequestTimeoutEarliestPercentage = 100; @@ -74,7 +85,7 @@ public class RPCCommunicatorTest { } @Test - public void testRoundtripLatency() throws Exception { + public void testRoundtripLatency() { final Timer timer = new FakeTimer(); final RPCCommunicator communicator = new RPCCommunicator( RPCCommunicator.createRealSupervisor(), @@ -94,9 +105,9 @@ public class RPCCommunicatorTest { when(nodeInfo.getReportedState()).thenReturn(new NodeState(NodeType.DISTRIBUTOR, State.UP)); communicator.getNodeState(nodeInfo, null); Mockito.verify(target).invokeAsync( - (Request)any(), + any(), eq(ROUNDTRIP_LATENCY_SECONDS + NODE_STATE_REQUEST_TIMEOUT_INTERVAL_MAX_MS/1000.0), - (RequestWaiter)any()); + any()); } private static class Fixture<RequestType> { diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/testutils/WaitCondition.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/testutils/WaitCondition.java index 9734156b13f..c24424d3bab 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/testutils/WaitCondition.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/testutils/WaitCondition.java @@ -29,7 +29,7 @@ public interface WaitCondition { abstract class StateWait implements WaitCondition { private final Object monitor; protected ClusterState currentState; - protected ClusterState convergedState; + ClusterState convergedState; private final SystemStateListener listener = new SystemStateListener() { @Override public void handleNewPublishedState(ClusterStateBundle state) { @@ -48,14 +48,14 @@ public interface WaitCondition { } }; - public StateWait(FleetController fc, Object monitor) { + protected StateWait(FleetController fc, Object monitor) { this.monitor = monitor; synchronized (this.monitor) { fc.addSystemStateListener(listener); } } - public ClusterState getCurrentState() { + ClusterState getCurrentState() { synchronized (monitor) { return currentState; } @@ -70,22 +70,22 @@ public interface WaitCondition { private boolean checkAllSpaces = false; private Set<String> checkSpaceSubset = Collections.emptySet(); - public RegexStateMatcher(String regex, FleetController fc, Object monitor) { + RegexStateMatcher(String regex, FleetController fc, Object monitor) { super(fc, monitor); pattern = Pattern.compile(regex); } - public RegexStateMatcher includeNotifyingNodes(Collection<DummyVdsNode> nodes) { + RegexStateMatcher includeNotifyingNodes(Collection<DummyVdsNode> nodes) { nodesToCheck = nodes; return this; } - public RegexStateMatcher checkAllSpaces(boolean checkAllSpaces) { + RegexStateMatcher checkAllSpaces(boolean checkAllSpaces) { this.checkAllSpaces = checkAllSpaces; return this; } - public RegexStateMatcher checkSpaceSubset(Set<String> spaces) { + RegexStateMatcher checkSpaceSubset(Set<String> spaces) { this.checkSpaceSubset = spaces; return this; } @@ -163,11 +163,11 @@ public interface WaitCondition { } } - public class InitProgressPassedMatcher extends StateWait { + class InitProgressPassedMatcher extends StateWait { private final Node node; private final double minProgress; - public InitProgressPassedMatcher(Node n, double minProgress, FleetController fc, Object monitor) { + InitProgressPassedMatcher(Node n, double minProgress, FleetController fc, Object monitor) { super(fc, monitor); this.node = n; this.minProgress = minProgress; @@ -195,7 +195,7 @@ public interface WaitCondition { private final int bitCount; private final int nodeCount; - public MinUsedBitsMatcher(int bitCount, int nodeCount, FleetController fc, Object monitor) { + MinUsedBitsMatcher(int bitCount, int nodeCount, FleetController fc, Object monitor) { super(fc, monitor); this.bitCount = bitCount; this.nodeCount = nodeCount; diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/testutils/WaitTask.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/testutils/WaitTask.java index 247f13d7cdb..92b06081645 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/testutils/WaitTask.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/testutils/WaitTask.java @@ -6,18 +6,18 @@ import com.yahoo.vespa.clustercontroller.core.NodeInfo; /** A wait task is something that is performed once in a while while waiting for something. */ public abstract class WaitTask { - public static final int defaultTaskFrequencyMillis = 1; + static final int defaultTaskFrequencyMillis = 1; public abstract boolean performWaitTask(); - public int getWaitTaskFrequencyInMillis() { + int getWaitTaskFrequencyInMillis() { return defaultTaskFrequencyMillis; } public static class StateResender extends WaitTask { public final FleetController fleetController; - public StateResender(FleetController fc) { + StateResender(FleetController fc) { fleetController = fc; } 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 fb49b1cbf47..deef3157eed 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 @@ -56,7 +56,7 @@ public interface Waiter { return swc.getCurrentState(); } - public ClusterState waitForState(String state) throws Exception { + public ClusterState waitForState(String state) { return waitForState(state, data.getTimeoutMS()); } public ClusterState waitForStateInAllSpaces(String state) { @@ -68,10 +68,10 @@ public interface Waiter { public ClusterState waitForState(String state, int timeoutMS) { return waitForState(state, timeoutMS, false, Collections.emptySet()); } - public ClusterState waitForStableSystem() throws Exception { + public ClusterState waitForStableSystem() { return waitForStableSystem(data.getDummyNodes().size() / 2); } - public ClusterState waitForStableSystem(int nodeCount) throws Exception { + public ClusterState waitForStableSystem(int nodeCount) { WaitCondition.StateWait swc = new WaitCondition.RegexStateMatcher("version:\\d+ distributor:"+nodeCount+" storage:"+nodeCount, data.getFleetController(), data.getMonitor()).includeNotifyingNodes(data.getDummyNodes()); wait(swc, new WaitTask.StateResender(data.getFleetController()), data.getTimeoutMS()); return swc.getCurrentState(); diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java index 56621550aa0..149943a89dc 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java @@ -59,7 +59,7 @@ public interface ModelContext { boolean useAdaptiveDispatch(); // TODO: Remove temporary default implementation default Optional<TlsSecrets> tlsSecrets() { return Optional.empty(); } - default boolean enableGroupingSessionCache() { return false; } + default boolean enableGroupingSessionCache() { return true; } // TODO Remove once no longer in use by old config models double defaultTermwiseLimit(); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainer.java index 405cfb47c72..af9c452da11 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainer.java @@ -1,18 +1,12 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.container; -import com.yahoo.config.model.api.TlsSecrets; import com.yahoo.config.model.api.container.ContainerServiceType; import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.container.bundle.BundleInstantiationSpecification; import com.yahoo.osgi.provider.model.ComponentModel; import com.yahoo.prelude.fastsearch.FS4ResourcePool; import com.yahoo.vespa.model.container.component.Component; -import com.yahoo.vespa.model.container.http.Http; -import com.yahoo.vespa.model.container.http.JettyHttpServer; -import com.yahoo.vespa.model.container.http.ssl.HostedSslConnectorFactory; - -import java.util.Optional; /** * A container that is typically used by container clusters set up from the user application. @@ -25,27 +19,14 @@ public final class ApplicationContainer extends Container { private final boolean isHostedVespa; - public ApplicationContainer(AbstractConfigProducer parent, String name, int index, boolean isHostedVespa, Optional<TlsSecrets> tlsSecrets, Optional<String> tlsCa) { - this(parent, name, false, index, isHostedVespa, tlsSecrets, tlsCa); + public ApplicationContainer(AbstractConfigProducer parent, String name, int index, boolean isHostedVespa) { + this(parent, name, false, index, isHostedVespa); } - public ApplicationContainer(AbstractConfigProducer parent, String name, boolean retired, int index, boolean isHostedVespa, Optional<TlsSecrets> tlsSecrets, Optional<String> tlsCa) { + public ApplicationContainer(AbstractConfigProducer parent, String name, boolean retired, int index, boolean isHostedVespa) { super(parent, name, retired, index); this.isHostedVespa = isHostedVespa; - if (isHostedVespa && tlsSecrets.isPresent()) { - - JettyHttpServer server = Optional.ofNullable(getHttp()) - .map(Http::getHttpServer) - .orElse(getDefaultHttpServer()); - if (server.getConnectorFactories().stream().noneMatch(connectorFactory -> connectorFactory instanceof HostedSslConnectorFactory)) { - String serverName = server.getComponentId().getName(); - var connectorFactory = tlsCa - .map(caCert -> new HostedSslConnectorFactory(serverName, tlsSecrets.get(), caCert)) - .orElseGet(() -> new HostedSslConnectorFactory(serverName, tlsSecrets.get())); - server.addConnector(connectorFactory); - } - } addComponent(getFS4ResourcePool()); // TODO Remove when FS4 based search protocol is gone } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java index 54318c0b7ea..414b127d6c4 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java @@ -56,8 +56,6 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat private Optional<TlsSecrets> tlsSecrets; private Optional<String> tlsClientAuthority; - private boolean useTlsClientAuthority = false; - private final boolean enableGroupingSessionCache; private MbusParams mbusParams; private boolean messageBusEnabled = true; @@ -67,7 +65,6 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat this.tlsSecrets = deployState.tlsSecrets(); this.tlsClientAuthority = deployState.tlsClientAuthority(); - this.enableGroupingSessionCache = deployState.getProperties().enableGroupingSessionCache(); restApiGroup = new ConfigProducerGroup<>(this, "rest-api"); servletGroup = new ConfigProducerGroup<>(this, "servlet"); @@ -189,10 +186,6 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat return tlsClientAuthority; } - public boolean enableGroupingSessionCache() { - return enableGroupingSessionCache; - } - public void setMbusParams(MbusParams mbusParams) { this.mbusParams = mbusParams; } @@ -211,10 +204,6 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat null)))); } - public void useTlsClientAuthority(boolean value) { - this.useTlsClientAuthority = value; - } - public static class MbusParams { // the amount of the maxpendingbytes to process concurrently, typically 0.2 (20%) final Double maxConcurrentFactor; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/search/ContainerSearch.java b/config-model/src/main/java/com/yahoo/vespa/model/container/search/ContainerSearch.java index 6e78ccf8daa..84e8cdf144f 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/search/ContainerSearch.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/search/ContainerSearch.java @@ -36,7 +36,6 @@ public class ContainerSearch extends ContainerSubsystem<SearchChains> private final List<AbstractSearchCluster> systems = new LinkedList<>(); private final Options options; - private final boolean enableGroupingSessionCache; private QueryProfiles queryProfiles; private SemanticRules semanticRules; @@ -45,8 +44,6 @@ public class ContainerSearch extends ContainerSubsystem<SearchChains> public ContainerSearch(ApplicationContainerCluster cluster, SearchChains chains, Options options) { super(chains); this.options = options; - - this.enableGroupingSessionCache = cluster.enableGroupingSessionCache(); } public void connectSearchClusters(Map<String, AbstractSearchCluster> searchClusters) { @@ -90,7 +87,6 @@ public class ContainerSearch extends ContainerSubsystem<SearchChains> if (queryProfiles!=null) { queryProfiles.getConfig(builder); } - builder.enableGroupingSessionCache(enableGroupingSessionCache); } @Override diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index 00aa5423087..e47c00eeea3 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.model.container.xml; import com.google.common.collect.ImmutableList; +import com.yahoo.component.ComponentId; import com.yahoo.component.Version; import com.yahoo.config.application.Xml; import com.yahoo.config.application.api.ApplicationPackage; @@ -57,7 +58,10 @@ import com.yahoo.vespa.model.container.component.Handler; import com.yahoo.vespa.model.container.component.chain.ProcessingHandler; import com.yahoo.vespa.model.container.docproc.ContainerDocproc; import com.yahoo.vespa.model.container.docproc.DocprocChains; +import com.yahoo.vespa.model.container.http.ConnectorFactory; import com.yahoo.vespa.model.container.http.Http; +import com.yahoo.vespa.model.container.http.JettyHttpServer; +import com.yahoo.vespa.model.container.http.ssl.HostedSslConnectorFactory; import com.yahoo.vespa.model.container.http.xml.HttpBuilder; import com.yahoo.vespa.model.container.jersey.xml.RestApiBuilder; import com.yahoo.vespa.model.container.processing.ProcessingChains; @@ -192,21 +196,10 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { addClientProviders(deployState, spec, cluster); addServerProviders(deployState, spec, cluster); - addTlsClientAuthority(deployState, spec, cluster); addAthensCopperArgos(cluster, context); // Must be added after nodes. } - private void addTlsClientAuthority(DeployState deployState, Element spec, ApplicationContainerCluster cluster) { - var clientAuthorized = XML.getChild(spec, "client-authorize"); - if (clientAuthorized != null) { - if (deployState.tlsClientAuthority().isEmpty()) { - throw new RuntimeException("client-authorize set, but security/clients.pem is missing"); - } - cluster.useTlsClientAuthority(true); - } - } - private void addSecretStore(ApplicationContainerCluster cluster, Element spec) { Element secretStoreElement = XML.getChild(spec, "secret-store"); if (secretStoreElement != null) { @@ -335,6 +328,33 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { if (httpElement != null) { cluster.setHttp(buildHttp(deployState, cluster, httpElement)); } + + // If the deployment contains certificate/private key reference, setup TLS port + if (deployState.tlsSecrets().isPresent()) { + boolean authorizeClient = XML.getChild(spec, "client-authorize") != null; + if (authorizeClient) { + if (deployState.tlsClientAuthority().isEmpty()) { + throw new RuntimeException("client-authorize set, but security/clients.pem is missing"); + } + } + + if(httpElement == null) { + cluster.setHttp(new Http(Collections.emptyList())); + } + if(cluster.getHttp().getHttpServer() == null) { + JettyHttpServer defaultHttpServer = new JettyHttpServer(new ComponentId("DefaultHttpServer")); + cluster.getHttp().setHttpServer(defaultHttpServer); + defaultHttpServer.addConnector(new ConnectorFactory("SearchServer", Defaults.getDefaults().vespaWebServicePort())); + + } + JettyHttpServer server = cluster.getHttp().getHttpServer(); + + String serverName = server.getComponentId().getName(); + HostedSslConnectorFactory connectorFactory = authorizeClient + ? new HostedSslConnectorFactory(serverName, deployState.tlsSecrets().get(), deployState.tlsClientAuthority().get()) + : new HostedSslConnectorFactory(serverName, deployState.tlsSecrets().get()); + server.addConnector(connectorFactory); + } } private Http buildHttp(DeployState deployState, ApplicationContainerCluster cluster, Element httpElement) { @@ -452,7 +472,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { } private void addStandaloneNode(ApplicationContainerCluster cluster) { - ApplicationContainer container = new ApplicationContainer(cluster, "standalone", cluster.getContainers().size(), cluster.isHostedVespa(), cluster.getTlsSecrets(), cluster.getTlsClientAuthority()); + ApplicationContainer container = new ApplicationContainer(cluster, "standalone", cluster.getContainers().size(), cluster.isHostedVespa()); cluster.addContainers(Collections.singleton(container)); } @@ -518,7 +538,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { Element nodesElement = XML.getChild(containerElement, "nodes"); Element rotationsElement = XML.getChild(containerElement, "rotations"); if (nodesElement == null) { // default single node on localhost - ApplicationContainer node = new ApplicationContainer(cluster, "container.0", 0, cluster.isHostedVespa(), cluster.getTlsSecrets(), cluster.getTlsClientAuthority()); + ApplicationContainer node = new ApplicationContainer(cluster, "container.0", 0, cluster.isHostedVespa()); HostResource host = allocateSingleNodeHost(cluster, log, containerElement, context); node.setHostResource(host); node.initService(context.getDeployLogger()); @@ -707,7 +727,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { List<ApplicationContainer> nodes = new ArrayList<>(); for (Map.Entry<HostResource, ClusterMembership> entry : hosts.entrySet()) { String id = "container." + entry.getValue().index(); - ApplicationContainer container = new ApplicationContainer(cluster, id, entry.getValue().retired(), entry.getValue().index(), cluster.isHostedVespa(), cluster.getTlsSecrets(), cluster.getTlsClientAuthority()); + ApplicationContainer container = new ApplicationContainer(cluster, id, entry.getValue().retired(), entry.getValue().index(), cluster.isHostedVespa()); container.setHostResource(entry.getKey()); container.initService(deployLogger); nodes.add(container); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerServiceBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerServiceBuilder.java index c976a7fb153..4d9ba2f920e 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerServiceBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerServiceBuilder.java @@ -26,9 +26,7 @@ public class ContainerServiceBuilder extends VespaDomBuilder.DomConfigProducerBu parent, id, index, - deployState.isHosted(), - deployState.tlsSecrets(), - deployState.tlsClientAuthority() + deployState.isHosted() ); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/Content.java b/config-model/src/main/java/com/yahoo/vespa/model/content/Content.java index f028f0ac0cc..74caf2d8026 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/Content.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/Content.java @@ -324,7 +324,7 @@ public class Content extends ConfigModel { if (!processedHosts.contains(host)) { String containerName = String.valueOf(searchNode.getDistributionKey()); ApplicationContainer docprocService = new ApplicationContainer(indexingCluster, containerName, index, - modelContext.getDeployState().isHosted(), modelContext.getDeployState().tlsSecrets(), Optional.empty()); + modelContext.getDeployState().isHosted()); index++; docprocService.useDynamicPorts(); docprocService.setHostResource(host); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java index 89169c44079..ec8a810c1a7 100755 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java @@ -5,7 +5,6 @@ import com.yahoo.cloud.config.ClusterInfoConfig; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.cloud.config.RoutingProviderConfig; import com.yahoo.config.application.api.DeployLogger; -import com.yahoo.config.model.api.TlsSecrets; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.deploy.TestProperties; import com.yahoo.config.model.test.MockRoot; @@ -14,7 +13,6 @@ import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.Zone; import com.yahoo.container.handler.ThreadpoolConfig; -import com.yahoo.jdisc.http.ConnectorConfig; import com.yahoo.search.config.QrStartConfig; import com.yahoo.vespa.model.Host; import com.yahoo.vespa.model.HostResource; @@ -22,22 +20,15 @@ import com.yahoo.vespa.model.admin.clustercontroller.ClusterControllerContainer; import com.yahoo.vespa.model.admin.clustercontroller.ClusterControllerContainerCluster; import com.yahoo.vespa.model.container.component.Component; import com.yahoo.vespa.model.container.docproc.ContainerDocproc; -import com.yahoo.vespa.model.container.http.ConnectorFactory; import com.yahoo.vespa.model.container.search.ContainerSearch; import com.yahoo.vespa.model.container.search.searchchain.SearchChains; -import org.hamcrest.Matchers; import org.junit.Test; import java.util.Collection; import java.util.Iterator; -import java.util.List; -import java.util.Optional; -import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; /** * @author Simon Thoresen Hult @@ -219,40 +210,8 @@ public class ContainerClusterTest { assertEquals(0, cluster.getAllComponents().stream().map(c -> c.getClassId().getName()).filter(c -> c.equals("com.yahoo.jdisc.http.filter.security.RoutingConfigProvider")).count()); } - @Test - public void requireThatProvidingTlsSecretOpensPort4443() { - DeployState state = new DeployState.Builder().properties(new TestProperties().setHostedVespa(true).setTlsSecrets(Optional.of(new TlsSecrets("CERT", "KEY")))).build(); - MockRoot root = new MockRoot("foo", state); - ApplicationContainerCluster cluster = new ApplicationContainerCluster(root, "container0", "container1", state); - - addContainer(state.getDeployLogger(), cluster, "c1", "host-c1"); - Optional<ApplicationContainer> container = cluster.getContainers().stream().findFirst(); - assertTrue(container.isPresent()); - - var httpServer = (container.get().getHttp() == null) ? container.get().getDefaultHttpServer() : container.get().getHttp().getHttpServer(); - - // Verify that there are two connectors - List<ConnectorFactory> connectorFactories = httpServer.getConnectorFactories(); - assertEquals(2, connectorFactories.size()); - List<Integer> ports = connectorFactories.stream() - .map(ConnectorFactory::getListenPort) - .collect(Collectors.toList()); - assertThat(ports, Matchers.containsInAnyOrder(8080, 4443)); - - ConnectorFactory tlsPort = connectorFactories.stream().filter(connectorFactory -> connectorFactory.getListenPort() == 4443).findFirst().orElseThrow(); - - ConnectorConfig.Builder builder = new ConnectorConfig.Builder(); - tlsPort.getConfig(builder); - - ConnectorConfig connectorConfig = new ConnectorConfig(builder); - assertTrue(connectorConfig.ssl().enabled()); - assertEquals("CERT", connectorConfig.ssl().certificate()); - assertEquals("KEY", connectorConfig.ssl().privateKey()); - assertEquals(4443, connectorConfig.listenPort()); - } - private static void addContainer(DeployLogger deployLogger, ApplicationContainerCluster cluster, String name, String hostName) { - ApplicationContainer container = new ApplicationContainer(cluster, name, 0, cluster.isHostedVespa(), cluster.getTlsSecrets(), cluster.getTlsClientAuthority()); + ApplicationContainer container = new ApplicationContainer(cluster, name, 0, cluster.isHostedVespa()); container.setHostResource(new HostResource(new Host(null, hostName))); container.initService(deployLogger); cluster.addContainer(container); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/empty.cfg b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/empty.cfg index 13153de1644..08b0e6809ce 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/empty.cfg +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/empty.cfg @@ -1 +1 @@ -enableGroupingSessionCache false
\ No newline at end of file +enableGroupingSessionCache true
\ No newline at end of file diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/explicit-reference-override.cfg b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/explicit-reference-override.cfg index b62e00a2ad5..fc77c5d82fa 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/explicit-reference-override.cfg +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/explicit-reference-override.cfg @@ -11,4 +11,4 @@ queryprofile[1].property[0].overridable "" queryprofile[1].reference[0].name "a" queryprofile[1].reference[0].value "a1" queryprofile[1].reference[0].overridable "" -enableGroupingSessionCache false
\ No newline at end of file +enableGroupingSessionCache true
\ No newline at end of file diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/newsbe-query-profiles-simple.cfg b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/newsbe-query-profiles-simple.cfg index 65b4258f174..d8092960ce8 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/newsbe-query-profiles-simple.cfg +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/newsbe-query-profiles-simple.cfg @@ -17,4 +17,4 @@ queryprofile[0].queryprofilevariant[0].fordimensionvalues[1] "uk" queryprofile[0].queryprofilevariant[0].fordimensionvalues[2] "sc" queryprofile[0].queryprofilevariant[0].property[0].name "scthumbnail.sourcecountry" queryprofile[0].queryprofilevariant[0].property[0].value "uk" -enableGroupingSessionCache false
\ No newline at end of file +enableGroupingSessionCache true
\ No newline at end of file diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/newsfe-query-profiles-simple.cfg b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/newsfe-query-profiles-simple.cfg index b416abf7c67..699f763b79c 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/newsfe-query-profiles-simple.cfg +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/newsfe-query-profiles-simple.cfg @@ -24,4 +24,4 @@ queryprofile[1].type "" queryprofile[1].reference[0].name "source.news" queryprofile[1].reference[0].value "backend/news" queryprofile[1].reference[0].overridable "" -enableGroupingSessionCache false
\ No newline at end of file +enableGroupingSessionCache true
\ No newline at end of file diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/query-profile-variants-configuration.cfg b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/query-profile-variants-configuration.cfg index 352966d0d8b..b86f55103bd 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/query-profile-variants-configuration.cfg +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/query-profile-variants-configuration.cfg @@ -39,4 +39,4 @@ queryprofile[2].type "" queryprofile[2].property[0].name "a" queryprofile[2].property[0].value "a1" queryprofile[2].property[0].overridable "" -enableGroupingSessionCache false
\ No newline at end of file +enableGroupingSessionCache true
\ No newline at end of file diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/query-profile-variants2-configuration.cfg b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/query-profile-variants2-configuration.cfg index 0f644cbb9eb..aff71104f7a 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/query-profile-variants2-configuration.cfg +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/query-profile-variants2-configuration.cfg @@ -59,4 +59,4 @@ queryprofile[3].queryprofilevariant[0].fordimensionvalues[0] "love" queryprofile[3].queryprofilevariant[0].fordimensionvalues[1] "default" queryprofile[3].queryprofilevariant[0].property[0].name "defaultIndex" queryprofile[3].queryprofilevariant[0].property[0].value "default" -enableGroupingSessionCache false
\ No newline at end of file +enableGroupingSessionCache true
\ No newline at end of file diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/query-profiles.cfg b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/query-profiles.cfg index 4a9dabdc6ca..54997e152f3 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/query-profiles.cfg +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/query-profiles.cfg @@ -103,4 +103,4 @@ queryprofiletype[2].field[0].type "string" queryprofiletype[2].field[0].overridable false queryprofiletype[2].field[0].mandatory false queryprofiletype[2].field[0].alias "" -enableGroupingSessionCache false
\ No newline at end of file +enableGroupingSessionCache true
\ No newline at end of file diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/variants-of-explicit-compound-with-reference.cfg b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/variants-of-explicit-compound-with-reference.cfg index 92605f504a0..0af30a9ad5c 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/variants-of-explicit-compound-with-reference.cfg +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/variants-of-explicit-compound-with-reference.cfg @@ -24,4 +24,4 @@ queryprofile[2].queryprofilevariant[0].reference[0].value "a2" queryprofile[2].queryprofilevariant[1].fordimensionvalues[0] "x2" queryprofile[2].queryprofilevariant[1].property[0].name "a.b" queryprofile[2].queryprofilevariant[1].property[0].value "a.b.x2" -enableGroupingSessionCache false
\ No newline at end of file +enableGroupingSessionCache true
\ No newline at end of file diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/variants-of-explicit-compound.cfg b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/variants-of-explicit-compound.cfg index 1e7739d9962..e5e2ff92510 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/variants-of-explicit-compound.cfg +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/variants-of-explicit-compound.cfg @@ -15,4 +15,4 @@ queryprofile[1].queryprofilevariant[0].property[0].value "a.b.x1" queryprofile[1].queryprofilevariant[1].fordimensionvalues[0] "x2" queryprofile[1].queryprofilevariant[1].property[0].name "a.b" queryprofile[1].queryprofilevariant[1].property[0].value "a.b.x2" -enableGroupingSessionCache false
\ No newline at end of file +enableGroupingSessionCache true
\ No newline at end of file diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java index 58947de398a..d2a9b8172b3 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java @@ -5,6 +5,7 @@ import com.yahoo.component.ComponentId; import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.model.NullConfigModelRegistry; import com.yahoo.config.model.api.ContainerEndpoint; +import com.yahoo.config.model.api.TlsSecrets; import com.yahoo.config.model.builder.xml.test.DomBuilderTest; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.deploy.TestProperties; @@ -23,6 +24,7 @@ import com.yahoo.container.handler.observability.ApplicationStatusHandler; import com.yahoo.container.jdisc.JdiscBindingsConfig; import com.yahoo.container.servlet.ServletConfigConfig; import com.yahoo.container.usability.BindingsOverviewHandler; +import com.yahoo.jdisc.http.ConnectorConfig; import com.yahoo.jdisc.http.ServletPathsConfig; import com.yahoo.net.HostName; import com.yahoo.path.Path; @@ -30,19 +32,21 @@ import com.yahoo.prelude.cluster.QrMonitorConfig; import com.yahoo.search.config.QrStartConfig; import com.yahoo.vespa.model.AbstractService; import com.yahoo.vespa.model.VespaModel; +import com.yahoo.vespa.model.container.ApplicationContainer; import com.yahoo.vespa.model.container.Container; import com.yahoo.vespa.model.container.ContainerCluster; import com.yahoo.vespa.model.container.SecretStore; import com.yahoo.vespa.model.container.component.Component; +import com.yahoo.vespa.model.container.http.ConnectorFactory; import com.yahoo.vespa.model.content.utils.ContentClusterUtils; import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithFilePkg; +import org.hamcrest.Matchers; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.w3c.dom.Element; import org.xml.sax.SAXException; -import java.io.File; import java.io.IOException; import java.io.StringReader; import java.util.List; @@ -659,7 +663,11 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { " <client-authorize />", "</container>"); try { - createModel(root, clusterElem); + DeployState state = new DeployState.Builder().properties( + new TestProperties() + .setHostedVespa(true) + .setTlsSecrets(Optional.of(new TlsSecrets("CERT", "KEY")))).build(); + createModel(root, state, null, clusterElem); } catch (RuntimeException e) { assertEquals(e.getMessage(), "client-authorize set, but security/clients.pem is missing"); return; @@ -706,6 +714,39 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { assertEquals("KMP_SETTING=1 KMP_AFFINITY=granularity=fine,verbose,compact,1,0 ", qrStartConfig.qrs().env()); } + @Test + public void requireThatProvidingTlsSecretOpensPort4443() { + Element clusterElem = DomBuilderTest.parse( + "<container version='1.0'>", + nodesXml, + "</container>" ); + + DeployState state = new DeployState.Builder().properties(new TestProperties().setHostedVespa(true).setTlsSecrets(Optional.of(new TlsSecrets("CERT", "KEY")))).build(); + createModel(root, state, null, clusterElem); + ApplicationContainer container = (ApplicationContainer)root.getProducer("container/container.0"); + + // Verify that there are two connectors + List<ConnectorFactory> connectorFactories = container.getHttp().getHttpServer().getConnectorFactories(); + assertEquals(2, connectorFactories.size()); + List<Integer> ports = connectorFactories.stream() + .map(ConnectorFactory::getListenPort) + .collect(Collectors.toList()); + assertThat(ports, Matchers.containsInAnyOrder(8080, 4443)); + + ConnectorFactory tlsPort = connectorFactories.stream().filter(connectorFactory -> connectorFactory.getListenPort() == 4443).findFirst().orElseThrow(); + + ConnectorConfig.Builder builder = new ConnectorConfig.Builder(); + tlsPort.getConfig(builder); + + + ConnectorConfig connectorConfig = new ConnectorConfig(builder); + assertTrue(connectorConfig.ssl().enabled()); + assertEquals("CERT", connectorConfig.ssl().certificate()); + assertEquals("KEY", connectorConfig.ssl().privateKey()); + assertEquals(4443, connectorConfig.listenPort()); + } + + private Element generateContainerElementWithRenderer(String rendererId) { return DomBuilderTest.parse( "<container id='default' version='1.0'>", diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java index b0fdc87c244..2b36f3496fe 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java @@ -136,7 +136,6 @@ public class ModelContextImpl implements ModelContext { private final boolean useAdaptiveDispatch; private final boolean dispatchWithProtobuf; private final Optional<TlsSecrets> tlsSecrets; - private final boolean enableGroupingSessionCache; private final double defaultTermwiseLimit; public Properties(ApplicationId applicationId, @@ -174,8 +173,6 @@ public class ModelContextImpl implements ModelContext { this.useAdaptiveDispatch = Flags.USE_ADAPTIVE_DISPATCH.bindTo(flagSource) .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); this.tlsSecrets = tlsSecrets; - this.enableGroupingSessionCache = Flags.ENABLE_GROUPING_SESSION_CACHE.bindTo(flagSource) - .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); defaultTermwiseLimit = Flags.DEFAULT_TERM_WISE_LIMIT.bindTo(flagSource) .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); } @@ -236,11 +233,6 @@ public class ModelContextImpl implements ModelContext { public Optional<TlsSecrets> tlsSecrets() { return tlsSecrets; } @Override - public boolean enableGroupingSessionCache() { - return enableGroupingSessionCache; - } - - @Override public double defaultTermwiseLimit() { return defaultTermwiseLimit; } } diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json index 164794243ec..cfb8d9dc1fb 100644 --- a/container-search/abi-spec.json +++ b/container-search/abi-spec.json @@ -4154,7 +4154,7 @@ ], "methods": [ "public void <init>(com.yahoo.statistics.Statistics, com.yahoo.jdisc.Metric, java.util.concurrent.Executor, com.yahoo.container.logging.AccessLog, com.yahoo.search.query.profile.config.QueryProfilesConfig, com.yahoo.container.core.ContainerHttpConfig, com.yahoo.search.searchchain.ExecutionFactory)", - "public void <init>(com.yahoo.statistics.Statistics, com.yahoo.jdisc.Metric, java.util.concurrent.Executor, com.yahoo.container.logging.AccessLog, com.yahoo.search.query.profile.compiled.CompiledQueryProfileRegistry, com.yahoo.search.searchchain.ExecutionFactory, boolean, java.util.Optional)", + "public void <init>(com.yahoo.statistics.Statistics, com.yahoo.jdisc.Metric, java.util.concurrent.Executor, com.yahoo.container.logging.AccessLog, com.yahoo.search.query.profile.compiled.CompiledQueryProfileRegistry, com.yahoo.search.searchchain.ExecutionFactory, java.util.Optional)", "public void <init>(com.yahoo.container.core.ChainsConfig, com.yahoo.search.config.IndexInfoConfig, com.yahoo.container.QrSearchersConfig, com.yahoo.vespa.configdefinition.SpecialtokensConfig, com.yahoo.statistics.Statistics, com.yahoo.language.Linguistics, com.yahoo.jdisc.Metric, com.yahoo.component.provider.ComponentRegistry, java.util.concurrent.Executor, com.yahoo.container.logging.AccessLog, com.yahoo.search.query.profile.config.QueryProfilesConfig, com.yahoo.component.provider.ComponentRegistry, com.yahoo.container.core.ContainerHttpConfig)", "public final com.yahoo.container.jdisc.HttpResponse handle(com.yahoo.container.jdisc.HttpRequest)", "public com.yahoo.search.Result searchAndFill(com.yahoo.search.Query, com.yahoo.component.chain.Chain)", diff --git a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java index eef4cb72e9b..57ecc468b48 100644 --- a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java +++ b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java @@ -98,8 +98,6 @@ public class SearchHandler extends LoggingRequestHandler { private final ExecutionFactory executionFactory; - private final boolean enableGroupingSessionCache; - private final class MeanConnections implements Callback { @Override @@ -127,7 +125,6 @@ public class SearchHandler extends LoggingRequestHandler { accessLog, QueryProfileConfigurer.createFromConfig(queryProfileConfig).compile(), executionFactory, - queryProfileConfig.enableGroupingSessionCache(), containerHttpConfig.hostResponseHeaderKey().equals("") ? Optional.empty() : Optional.of( containerHttpConfig.hostResponseHeaderKey())); } @@ -138,11 +135,9 @@ public class SearchHandler extends LoggingRequestHandler { AccessLog accessLog, CompiledQueryProfileRegistry queryProfileRegistry, ExecutionFactory executionFactory, - boolean enableGroupingSessionCache, Optional<String> hostResponseHeaderKey) { super(executor, accessLog, metric, true); log.log(LogLevel.DEBUG, "SearchHandler.init " + System.identityHashCode(this)); - this.enableGroupingSessionCache = enableGroupingSessionCache; this.queryProfileRegistry = queryProfileRegistry; this.executionFactory = executionFactory; @@ -251,9 +246,6 @@ public class SearchHandler extends LoggingRequestHandler { CompiledQueryProfile queryProfile = queryProfileRegistry.findQueryProfile(queryProfileName); Query query = new Query(request, requestMap, queryProfile); - if (!enableGroupingSessionCache) { - query.setGroupingSessionCache(false); - } boolean benchmarking = VespaHeaders.benchmarkOutput(request); boolean benchmarkCoverage = VespaHeaders.benchmarkCoverage(benchmarking, request.getJDiscRequest().headers()); diff --git a/container-search/src/main/resources/configdefinitions/query-profiles.def b/container-search/src/main/resources/configdefinitions/query-profiles.def index 11966cae8ce..3c6d11e2944 100644 --- a/container-search/src/main/resources/configdefinitions/query-profiles.def +++ b/container-search/src/main/resources/configdefinitions/query-profiles.def @@ -86,9 +86,8 @@ queryprofiletype[].field[].mandatory bool default=false # A space-separated list of aliases of this field name. Aliases are case insensitive queryprofiletype[].field[].alias string default="" -# Temporary feature flag for enabling grouping session cache by default -# TODO Remove me once grouping session cache rollout is complete and cache is enabled by default -enableGroupingSessionCache bool default=false +# TODO Remove once no longer used by old config models +enableGroupingSessionCache bool default=true diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Node.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Node.java index c2d39236b8f..4ffe221b555 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Node.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Node.java @@ -185,7 +185,8 @@ public class Node { inactive, dirty, failed, - parked + parked, + unknown, } /** Known node states with regards to service orchestration */ @@ -199,7 +200,8 @@ public class Node { public enum ClusterType { admin, container, - content + content, + unknown } } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java index 19486b6c2c5..9a44a76b145 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java @@ -6,6 +6,7 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeList; +import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeMembership; import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeRepositoryNode; import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeState; @@ -75,33 +76,37 @@ public interface NodeRepository { fromJacksonState(node.getState()), fromJacksonType(node.getType()), application, - Version.fromString(node.getVespaVersion()), - Version.fromString(node.getWantedVespaVersion()), - Version.fromString(node.getCurrentOsVersion()), - Version.fromString(node.getWantedOsVersion()), + versionFrom(node.getVespaVersion()), + versionFrom(node.getWantedVespaVersion()), + versionFrom(node.getCurrentOsVersion()), + versionFrom(node.getWantedOsVersion()), fromBoolean(node.getAllowedToBeDown()), - node.getCurrentRestartGeneration(), - node.getRestartGeneration(), - node.getCurrentRebootGeneration(), - node.getRebootGeneration(), - node.getMinCpuCores(), - node.getMinMainMemoryAvailableGb(), - node.getMinDiskAvailableGb(), - node.getBandwidth() / 1000, - node.getFastDisk(), - node.getCost() == null ? 0 : node.getCost(), + toInt(node.getCurrentRestartGeneration()), + toInt(node.getRestartGeneration()), + toInt(node.getCurrentRebootGeneration()), + toInt(node.getRebootGeneration()), + toDouble(node.getMinCpuCores()), + toDouble(node.getMinMainMemoryAvailableGb()), + toDouble(node.getMinDiskAvailableGb()), + toDouble(node.getBandwidth()) / 1000, + toBoolean(node.getFastDisk()), + toInt(node.getCost()), node.getCanonicalFlavor(), - node.getMembership().clusterid, - clusterTypeOf(node.getMembership().clustertype)); + clusterIdOf(node.getMembership()), + clusterTypeOf(node.getMembership())); } - private static Node.ClusterType clusterTypeOf(String type) { - switch (type) { + private static String clusterIdOf(NodeMembership nodeMembership) { + return nodeMembership == null ? "" : nodeMembership.clusterid; + } + + private static Node.ClusterType clusterTypeOf(NodeMembership nodeMembership) { + switch (nodeMembership.clustertype) { case "admin": return Node.ClusterType.admin; case "content": return Node.ClusterType.content; case "container": return Node.ClusterType.container; - default: throw new IllegalArgumentException("Unknown cluster type '" + type + "'."); } + return Node.ClusterType.unknown; } // Convert Jackson type to config.provision type @@ -127,8 +132,8 @@ public interface NodeRepository { case dirty: return Node.State.dirty; case failed: return Node.State.failed; case parked: return Node.State.parked; - default: throw new IllegalArgumentException("Unknown state: " + state); } + return Node.State.unknown; } private static Node.ServiceState fromBoolean(Boolean allowedDown) { @@ -136,4 +141,21 @@ public interface NodeRepository { ? Node.ServiceState.unorchestrated : allowedDown ? Node.ServiceState.allowedDown : Node.ServiceState.expectedUp; } + + private static boolean toBoolean(Boolean b) { + return b == null ? false : b; + } + + private static double toDouble(Double d) { + return d == null ? 0 : d; + } + + private static int toInt(Integer i) { + return i == null ? 0 : i; + } + + private static Version versionFrom(String s) { + return s == null ? Version.emptyVersion : Version.fromString(s); + } + } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/metrics/ConfigServerMetricsService.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/metrics/ConfigServerMetricsService.java index 2240afee804..4878c108bca 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/metrics/ConfigServerMetricsService.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/metrics/ConfigServerMetricsService.java @@ -1,10 +1,12 @@ package com.yahoo.vespa.hosted.controller.api.integration.metrics; +import com.google.inject.Inject; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; +import com.yahoo.vespa.hosted.controller.api.integration.ServiceRegistry; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServer; import com.yahoo.vespa.hosted.controller.api.integration.routing.RotationStatus; @@ -18,11 +20,18 @@ import java.util.function.Function; * * @author ogronnesby */ +// TODO: This module should not contain components. Move this to controller-server. public class ConfigServerMetricsService implements MetricsService { + private final ConfigServer configServerClient; - public ConfigServerMetricsService(ConfigServer configServerClient) { - this.configServerClient = configServerClient; + @Inject + public ConfigServerMetricsService(ServiceRegistry serviceRegistry) { + this(serviceRegistry.configServer()); + } + + ConfigServerMetricsService(ConfigServer configServer) { + this.configServerClient = configServer; } @Override diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 7dcdda38765..20ce7fe2578 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -145,10 +145,10 @@ public class Flags { "Takes effect on deployment through controller", APPLICATION_ID); - public static final UnboundBooleanFlag ENABLE_GROUPING_SESSION_CACHE = defineFeatureFlag( - "enable-grouping-session-cache", true, - "Enable grouping session cache", - "Takes effect at redeployment", + public static final UnboundBooleanFlag MULTIPLE_GLOBAL_ENDPOINTS = defineFeatureFlag( + "multiple-global-endpoints", false, + "Allow applications to use new endpoints syntax in deployment.xml", + "Takes effect on deployment through controller", APPLICATION_ID); public static final UnboundDoubleFlag DEFAULT_TERM_WISE_LIMIT = defineDoubleFlag( diff --git a/searchcore/src/vespa/searchcore/proton/matching/blueprintbuilder.cpp b/searchcore/src/vespa/searchcore/proton/matching/blueprintbuilder.cpp index 672e7f78784..656e49d7569 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/blueprintbuilder.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/blueprintbuilder.cpp @@ -103,9 +103,8 @@ private: void buildSameElement(ProtonSameElement &n) { SameElementBuilder builder(_requestContext, _context); - for (size_t i = 0; i < n.getChildren().size(); ++i) { - search::query::Node &node = *n.getChildren()[i]; - builder.add_child(node); + for (search::query::Node *node : n.getChildren()) { + builder.add_child(*node); } _result = builder.build(); } diff --git a/searchcore/src/vespa/searchcore/proton/server/proton.cpp b/searchcore/src/vespa/searchcore/proton/server/proton.cpp index 58313f17235..0a49f494406 100644 --- a/searchcore/src/vespa/searchcore/proton/server/proton.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/proton.cpp @@ -16,7 +16,6 @@ #include <vespa/searchcore/proton/flushengine/flushengine.h> #include <vespa/searchcore/proton/flushengine/flush_engine_explorer.h> -#include <vespa/searchcore/proton/flushengine/prepare_restart_flush_strategy.h> #include <vespa/searchcore/proton/flushengine/tls_stats_factory.h> #include <vespa/searchcore/proton/reference/document_db_reference_registry.h> #include <vespa/searchcore/proton/summaryengine/summaryengine.h> @@ -28,7 +27,6 @@ #include <vespa/document/datatype/documenttype.h> #include <vespa/document/repo/documenttyperepo.h> #include <vespa/vespalib/io/fileutil.h> -#include <vespa/vespalib/util/closuretask.h> #include <vespa/vespalib/util/lambdatask.h> #include <vespa/vespalib/util/host_name.h> #include <vespa/vespalib/util/random.h> @@ -382,7 +380,7 @@ Proton::addDocumentDB(const DocTypeName &docTypeName, try { const std::shared_ptr<const DocumentTypeRepo> repo = bootstrapConfig->getDocumentTypeRepoSP(); const document::DocumentType *docType = repo->getDocumentType(docTypeName.getName()); - if (docType != NULL) { + if (docType != nullptr) { LOG(info, "Add document database: doctypename(%s), configid(%s)", docTypeName.toString().c_str(), configId.c_str()); return addDocumentDB(*docType, bucketSpace, bootstrapConfig, documentDBConfig, initializeThreads); @@ -430,6 +428,7 @@ Proton::~Proton() } if (_rpcHooks) { _rpcHooks->close(); + _metricsEngine->removeExternalMetrics(_rpcHooks->proto_rpc_adapter_metrics()); } if (_memoryFlushConfigUpdater) { _diskMemUsageSampler->notifier().removeDiskMemUsageListener(_memoryFlushConfigUpdater.get()); @@ -453,7 +452,7 @@ Proton::~Proton() if (_fs4Server) { _fs4Server->shutDown(); } - if (_documentDBMap.size() > 0) { + if ( ! _documentDBMap.empty()) { size_t numCores = 4; const std::shared_ptr<proton::ProtonConfigSnapshot> pcsp = _protonConfigurer.getActiveConfigSnapshot(); if (pcsp) { @@ -568,7 +567,7 @@ Proton::addDocumentDB(const document::DocumentType &docType, std::lock_guard<std::shared_timed_mutex> guard(_mutex); DocTypeName docTypeName(docType.getName()); - DocumentDBMap::iterator it = _documentDBMap.find(docTypeName); + auto it = _documentDBMap.find(docTypeName); if (it != _documentDBMap.end()) { return it->second; } @@ -632,7 +631,7 @@ Proton::removeDocumentDB(const DocTypeName &docTypeName) DocumentDB::SP old; { std::lock_guard<std::shared_timed_mutex> guard(_mutex); - DocumentDBMap::iterator it = _documentDBMap.find(docTypeName); + auto it = _documentDBMap.find(docTypeName); if (it == _documentDBMap.end()) { return; } @@ -855,9 +854,9 @@ const vespalib::string RESOURCE_USAGE = "resourceusage"; struct StateExplorerProxy : vespalib::StateExplorer { const StateExplorer &explorer; explicit StateExplorerProxy(const StateExplorer &explorer_in) : explorer(explorer_in) {} - virtual void get_state(const vespalib::slime::Inserter &inserter, bool full) const override { explorer.get_state(inserter, full); } - virtual std::vector<vespalib::string> get_children_names() const override { return explorer.get_children_names(); } - virtual std::unique_ptr<vespalib::StateExplorer> get_child(vespalib::stringref name) const override { return explorer.get_child(name); } + void get_state(const vespalib::slime::Inserter &inserter, bool full) const override { explorer.get_state(inserter, full); } + std::vector<vespalib::string> get_children_names() const override { return explorer.get_children_names(); } + std::unique_ptr<vespalib::StateExplorer> get_child(vespalib::stringref name) const override { return explorer.get_child(name); } }; struct DocumentDBMapExplorer : vespalib::StateExplorer { diff --git a/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp b/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp index 60cad3e9c48..4d1d0b6fff5 100644 --- a/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp +++ b/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp @@ -280,7 +280,7 @@ EnumStoreTest::testHoldListAndGeneration() ses.decRefCount(idx); EXPECT_EQUAL(0u, ses.getRefCount(idx)); } - ses.freeUnusedEnums(true); + ses.freeUnusedEnums(); // check readers again checkReaders(ses, sesGen, readers); @@ -304,7 +304,7 @@ void decRefCount(NumericEnumStore& store, NumericEnumStore::Index idx) { store.decRefCount(idx); - store.freeUnusedEnums(false); + store.freeUnusedEnums(); generation_t gen = 5; store.transferHoldLists(gen); store.trimHoldLists(gen + 1); diff --git a/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.cpp b/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.cpp index 24c0eeabd2c..a080c91cb34 100644 --- a/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.cpp +++ b/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.cpp @@ -14,6 +14,10 @@ #include <vespa/log/log.h> LOG_SETUP(".searchlib.attribute.enum_store_dictionary"); +using search::datastore::EntryComparator; +using search::datastore::EntryRef; +using search::datastore::UniqueStoreAddResult; + namespace search { using btree::BTreeNode; @@ -76,44 +80,19 @@ EnumStoreDictionary<DictionaryT>::fixupRefCounts(const EnumVector& hist) template <typename DictionaryT> void EnumStoreDictionary<DictionaryT>::removeUnusedEnums(const IndexSet& unused, - const datastore::EntryComparator& cmp, - const datastore::EntryComparator* fcmp) + const datastore::EntryComparator& cmp) { - using Iterator = typename DictionaryT::Iterator; if (unused.empty()) { return; } - Iterator it(BTreeNode::Ref(), this->_dict.getAllocator()); - for (const auto& idx : unused) { - it.lower_bound(this->_dict.getRoot(), idx, cmp); - assert(it.valid() && !cmp(idx, it.getKey())); - if (Iterator::hasData() && fcmp != nullptr) { - typename DictionaryT::DataType pidx(it.getData()); - this->_dict.remove(it); - if (!it.valid() || (*fcmp)(idx, it.getKey())) { - continue; // Next entry does not use same posting list - } - --it; - if (it.valid() && !(*fcmp)(it.getKey(), idx)) { - continue; // Previous entry uses same posting list - } - if (it.valid()) { - ++it; - } else { - it.begin(); - } - this->_dict.thaw(it); - it.writeData(pidx); - } else { - this->_dict.remove(it); - } - } + for (const auto& ref : unused) { + this->remove(cmp, ref); + } } template <typename DictionaryT> void -EnumStoreDictionary<DictionaryT>::freeUnusedEnums(const datastore::EntryComparator& cmp, - const datastore::EntryComparator* fcmp) +EnumStoreDictionary<DictionaryT>::freeUnusedEnums(const datastore::EntryComparator& cmp) { IndexSet unused; @@ -121,20 +100,19 @@ EnumStoreDictionary<DictionaryT>::freeUnusedEnums(const datastore::EntryComparat for (auto iter = this->_dict.begin(); iter.valid(); ++iter) { _enumStore.freeUnusedEnum(iter.getKey(), unused); } - removeUnusedEnums(unused, cmp, fcmp); + removeUnusedEnums(unused, cmp); } template <typename DictionaryT> void EnumStoreDictionary<DictionaryT>::freeUnusedEnums(const IndexSet& toRemove, - const datastore::EntryComparator& cmp, - const datastore::EntryComparator* fcmp) + const datastore::EntryComparator& cmp) { IndexSet unused; for (const auto& index : toRemove) { _enumStore.freeUnusedEnum(index, unused); } - removeUnusedEnums(unused, cmp, fcmp); + removeUnusedEnums(unused, cmp); } template <typename DictionaryT> @@ -218,6 +196,51 @@ EnumStoreDictionary<DictionaryT>::hasData() const return DictionaryT::LeafNodeType::hasData(); } +EnumStoreFoldedDictionary::EnumStoreFoldedDictionary(IEnumStore& enumStore, std::unique_ptr<EntryComparator> folded_compare) + : EnumStoreDictionary<EnumPostingTree>(enumStore), + _folded_compare(std::move(folded_compare)) +{ +} + +EnumStoreFoldedDictionary::~EnumStoreFoldedDictionary() = default; + +UniqueStoreAddResult +EnumStoreFoldedDictionary::add(const EntryComparator &comp, std::function<EntryRef(void)> insertEntry) +{ + auto it = _dict.lowerBound(EntryRef(), comp); + if (it.valid() && !comp(EntryRef(), it.getKey())) { + // Entry already exists + return UniqueStoreAddResult(it.getKey(), false); + } + EntryRef newRef = insertEntry(); + _dict.insert(it, newRef, EntryRef()); + // Maybe move posting list reference from next entry + ++it; + if (it.valid() && it.getData().valid() && !(*_folded_compare)(newRef, it.getKey())) { + EntryRef posting_list_ref(it.getData()); + _dict.thaw(it); + it.writeData(EntryRef()); + --it; + assert(it.valid() && it.getKey() == newRef); + it.writeData(posting_list_ref); + } + return UniqueStoreAddResult(newRef, true); +} + +void +EnumStoreFoldedDictionary::remove(const EntryComparator &comp, EntryRef ref) +{ + assert(ref.valid()); + auto it = _dict.lowerBound(ref, comp); + assert(it.valid() && it.getKey() == ref); + EntryRef posting_list_ref(it.getData()); + _dict.remove(it); + // Maybe copy posting list reference to next entry + if (posting_list_ref.valid() && it.valid() && !it.getData().valid() && !(*_folded_compare)(ref, it.getKey())) { + this->_dict.thaw(it); + it.writeData(posting_list_ref); + } +} template class EnumStoreDictionary<EnumTree>; diff --git a/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.h b/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.h index 31f52e4e965..38a7d6b1985 100644 --- a/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.h +++ b/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.h @@ -38,15 +38,12 @@ public: void fixupRefCounts(const EnumVector& hist) override; void removeUnusedEnums(const IndexSet& unused, - const datastore::EntryComparator& cmp, - const datastore::EntryComparator* fcmp); + const datastore::EntryComparator& cmp); - void freeUnusedEnums(const datastore::EntryComparator& cmp, - const datastore::EntryComparator* fcmp) override; + void freeUnusedEnums(const datastore::EntryComparator& cmp) override; void freeUnusedEnums(const IndexSet& toRemove, - const datastore::EntryComparator& cmp, - const datastore::EntryComparator* fcmp) override; + const datastore::EntryComparator& cmp) override; bool findIndex(const datastore::EntryComparator& cmp, Index& idx) const override; bool findFrozenIndex(const datastore::EntryComparator& cmp, Index& idx) const override; @@ -62,6 +59,27 @@ public: bool hasData() const override; }; +/** + * Concrete dictionary for an enum store that extends the + * functionality of a unique store dictionary. + * + * Special handling of value (posting list reference) is added to + * ensure that entries with same folded key share a posting list + * (e.g. case insensitive search) and posting list reference is found + * for the first of these entries. + */ +class EnumStoreFoldedDictionary : public EnumStoreDictionary<EnumPostingTree> +{ +private: + std::unique_ptr<datastore::EntryComparator> _folded_compare; + +public: + EnumStoreFoldedDictionary(IEnumStore& enumStore, std::unique_ptr<datastore::EntryComparator> folded_compare); + ~EnumStoreFoldedDictionary() override; + datastore::UniqueStoreAddResult add(const datastore::EntryComparator& comp, std::function<datastore::EntryRef(void)> insertEntry) override; + void remove(const datastore::EntryComparator& comp, datastore::EntryRef ref) override; +}; + extern template class btree::BTreeNodeT<IEnumStore::Index, EnumTreeTraits::INTERNAL_SLOTS>; diff --git a/searchlib/src/vespa/searchlib/attribute/enumstore.cpp b/searchlib/src/vespa/searchlib/attribute/enumstore.cpp index 7ce65193c40..1d5b4348af8 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumstore.cpp +++ b/searchlib/src/vespa/searchlib/attribute/enumstore.cpp @@ -47,10 +47,14 @@ EnumStoreT<StringEntryType>::deserialize(const void* src, } std::unique_ptr<datastore::IUniqueStoreDictionary> -make_enum_store_dictionary(IEnumStore &store, bool has_postings) +make_enum_store_dictionary(IEnumStore &store, bool has_postings, std::unique_ptr<datastore::EntryComparator> folded_compare) { if (has_postings) { - return std::make_unique<EnumStoreDictionary<EnumPostingTree>>(store); + if (folded_compare) { + return std::make_unique<EnumStoreFoldedDictionary>(store, std::move(folded_compare)); + } else { + return std::make_unique<EnumStoreDictionary<EnumPostingTree>>(store); + } } else { return std::make_unique<EnumStoreDictionary<EnumTree>>(store); } diff --git a/searchlib/src/vespa/searchlib/attribute/enumstore.h b/searchlib/src/vespa/searchlib/attribute/enumstore.h index 94252239975..c422cf14cda 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumstore.h +++ b/searchlib/src/vespa/searchlib/attribute/enumstore.h @@ -243,20 +243,15 @@ public: std::vector<IEnumStore::EnumHandle> findFoldedEnums(DataType value) const; void addEnum(DataType value, Index &newIdx); bool findIndex(DataType value, Index &idx) const; - void freeUnusedEnums(bool movePostingidx) override; + void freeUnusedEnums() override; void freeUnusedEnums(const IndexSet& toRemove); vespalib::MemoryUsage update_stat() override; std::unique_ptr<EnumIndexRemapper> consider_compact(const CompactionStrategy& compaction_strategy) override; std::unique_ptr<EnumIndexRemapper> compact_worst(bool compact_memory, bool compact_address_space) override; - -private: - template <typename Dictionary> - void addEnum(DataType value, Index& newIdx, Dictionary& dict); - }; std::unique_ptr<datastore::IUniqueStoreDictionary> -make_enum_store_dictionary(IEnumStore &store, bool has_postings); +make_enum_store_dictionary(IEnumStore &store, bool has_postings, std::unique_ptr<datastore::EntryComparator> folded_compare); vespalib::asciistream & operator << (vespalib::asciistream & os, const IEnumStore::Index & idx); diff --git a/searchlib/src/vespa/searchlib/attribute/enumstore.hpp b/searchlib/src/vespa/searchlib/attribute/enumstore.hpp index 574712798c2..e107b6da787 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumstore.hpp +++ b/searchlib/src/vespa/searchlib/attribute/enumstore.hpp @@ -34,7 +34,7 @@ void EnumStoreT<EntryType>::freeUnusedEnum(Index idx, IndexSet& unused) template <typename EntryType> EnumStoreT<EntryType>::EnumStoreT(bool has_postings) - : _store(make_enum_store_dictionary(*this, has_postings)), + : _store(make_enum_store_dictionary(*this, has_postings, EntryType::hasFold() ? std::make_unique<FoldedComparatorType>(*this) : std::unique_ptr<datastore::EntryComparator>())), _dict(static_cast<IEnumStoreDictionary&>(_store.get_dictionary())), _cached_values_memory_usage(), _cached_values_address_space_usage(0, 0, (1ull << 32)) @@ -169,15 +169,10 @@ EnumStoreT<EntryType>::findIndex(DataType value, Index &idx) const template <typename EntryType> void -EnumStoreT<EntryType>::freeUnusedEnums(bool movePostingIdx) +EnumStoreT<EntryType>::freeUnusedEnums() { ComparatorType cmp(*this); - if (EntryType::hasFold() && movePostingIdx) { - FoldedComparatorType fcmp(*this); - _dict.freeUnusedEnums(cmp, &fcmp); - } else { - _dict.freeUnusedEnums(cmp, nullptr); - } + _dict.freeUnusedEnums(cmp); } template <typename EntryType> @@ -185,75 +180,16 @@ void EnumStoreT<EntryType>::freeUnusedEnums(const IndexSet& toRemove) { ComparatorType cmp(*this); - if (EntryType::hasFold()) { - FoldedComparatorType fcmp(*this); - _dict.freeUnusedEnums(toRemove, cmp, &fcmp); - } else { - _dict.freeUnusedEnums(toRemove, cmp, nullptr); - } -} - -template <typename EntryType> -template <typename Dictionary> -void -EnumStoreT<EntryType>::addEnum(DataType value, Index& newIdx, Dictionary& dict) -{ - typedef typename Dictionary::Iterator DictionaryIterator; - - // check if already present - ComparatorType cmp(*this, value); - DictionaryIterator it(btree::BTreeNode::Ref(), dict.getAllocator()); - it.lower_bound(dict.getRoot(), Index(), cmp); - if (it.valid() && !cmp(Index(), it.getKey())) { - newIdx = it.getKey(); - return; - } - - newIdx = _store.get_allocator().allocate(value); - - // TODO: Move this logic to "add/insert" on the dictionary - // update tree with new index - dict.insert(it, newIdx, typename Dictionary::DataType()); - - // Copy posting list idx from next entry if same folded value. - // Only for string posting list attributes, i.e. dictionary has - // data and entry type has folded compare. - if (DictionaryIterator::hasData() && EntryType::hasFold()) { - FoldedComparatorType foldCmp(*this); - ++it; - if (!it.valid() || foldCmp(newIdx, it.getKey())) { - return; // Next entry does not use same posting list - } - --it; - --it; - if (it.valid() && !foldCmp(it.getKey(), newIdx)) { - return; // Previous entry uses same posting list - } - if (it.valid()) { - ++it; - } else { - it.begin(); - } - assert(it.valid() && it.getKey() == newIdx); - ++it; - typename Dictionary::DataType pidx(it.getData()); - dict.thaw(it); - it.writeData(typename Dictionary::DataType()); - --it; - assert(it.valid() && it.getKey() == newIdx); - it.writeData(pidx); - } + _dict.freeUnusedEnums(toRemove, cmp); } template <typename EntryType> void EnumStoreT<EntryType>::addEnum(DataType value, Index& newIdx) { - if (_dict.hasData()) { - addEnum(value, newIdx, static_cast<EnumStoreDictionary<EnumPostingTree> &>(_dict).getDictionary()); - } else { - addEnum(value, newIdx, static_cast<EnumStoreDictionary<EnumTree> &>(_dict).getDictionary()); - } + ComparatorType cmp(*this, value); + auto add_result = _dict.add(cmp, [this, &value]() -> EntryRef { return _store.get_allocator().allocate(value); }); + newIdx = add_result.ref(); } template <typename EntryType> diff --git a/searchlib/src/vespa/searchlib/attribute/i_enum_store.h b/searchlib/src/vespa/searchlib/attribute/i_enum_store.h index 2a9842075e6..8a9058d8766 100644 --- a/searchlib/src/vespa/searchlib/attribute/i_enum_store.h +++ b/searchlib/src/vespa/searchlib/attribute/i_enum_store.h @@ -51,7 +51,7 @@ public: virtual ssize_t deserialize0(const void* src, size_t available, IndexVector& idx) = 0; virtual void fixupRefCount(Index idx, uint32_t refCount) = 0; virtual void freeUnusedEnum(Index idx, IndexSet& unused) = 0; - virtual void freeUnusedEnums(bool movePostingIdx) = 0; + virtual void freeUnusedEnums() = 0; virtual bool foldedChange(const Index& idx1, const Index& idx2) = 0; virtual IEnumStoreDictionary& getEnumStoreDict() = 0; virtual const IEnumStoreDictionary& getEnumStoreDict() const = 0; @@ -94,7 +94,7 @@ public: fixupRefCount(ti.getKey(), *hi); } assert(!ti.valid()); - freeUnusedEnums(false); + freeUnusedEnums(); } }; diff --git a/searchlib/src/vespa/searchlib/attribute/i_enum_store_dictionary.h b/searchlib/src/vespa/searchlib/attribute/i_enum_store_dictionary.h index a4c14256835..5c2510346c8 100644 --- a/searchlib/src/vespa/searchlib/attribute/i_enum_store_dictionary.h +++ b/searchlib/src/vespa/searchlib/attribute/i_enum_store_dictionary.h @@ -42,11 +42,9 @@ public: virtual ssize_t deserialize(const void* src, size_t available, IndexVector& idx) = 0; virtual void fixupRefCounts(const EnumVector& hist) = 0; - virtual void freeUnusedEnums(const datastore::EntryComparator& cmp, - const datastore::EntryComparator* fcmp) = 0; + virtual void freeUnusedEnums(const datastore::EntryComparator& cmp) = 0; virtual void freeUnusedEnums(const IndexSet& toRemove, - const datastore::EntryComparator& cmp, - const datastore::EntryComparator* fcmp) = 0; + const datastore::EntryComparator& cmp) = 0; virtual bool findIndex(const datastore::EntryComparator& cmp, Index& idx) const = 0; virtual bool findFrozenIndex(const datastore::EntryComparator& cmp, Index& idx) const = 0; virtual std::vector<attribute::IAttributeVector::EnumHandle> diff --git a/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp b/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp index c8ee6ab50b2..e6e390ef3a9 100644 --- a/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp @@ -108,7 +108,7 @@ PostingListAttributeBase<P>::fillPostingsFixupEnumBase(const LoadedEnumAttribute &postings._removals[0], &postings._removals[0] + postings._removals.size()); posting_itr.writeData(newIndex); - enumStore.freeUnusedEnums(false); + enumStore.freeUnusedEnums(); } template <typename P> diff --git a/searchlib/src/vespa/searchlib/queryeval/create_blueprint_visitor_helper.cpp b/searchlib/src/vespa/searchlib/queryeval/create_blueprint_visitor_helper.cpp index 5cd74229fce..e3588d88ccb 100644 --- a/searchlib/src/vespa/searchlib/queryeval/create_blueprint_visitor_helper.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/create_blueprint_visitor_helper.cpp @@ -32,10 +32,10 @@ CreateBlueprintVisitorHelper::getResult() void CreateBlueprintVisitorHelper::visitPhrase(query::Phrase &n) { auto phrase = std::make_unique<SimplePhraseBlueprint>(_field, _requestContext); - for (size_t i = 0; i < n.getChildren().size(); ++i) { + for (const query::Node * child : n.getChildren()) { FieldSpecList fields; fields.add(phrase->getNextChildField(_field)); - phrase->addTerm(_searchable.createBlueprint(_requestContext, fields, *n.getChildren()[i])); + phrase->addTerm(_searchable.createBlueprint(_requestContext, fields, *child)); } setResult(std::move(phrase)); } @@ -64,8 +64,7 @@ CreateBlueprintVisitorHelper::handleNumberTermAsText(query::NumberTerm &n) template <typename WS, typename NODE> void -CreateBlueprintVisitorHelper::createWeightedSet(WS *bp, NODE &n) { - Blueprint::UP result(bp); +CreateBlueprintVisitorHelper::createWeightedSet(std::unique_ptr<WS> bp, NODE &n) { FieldSpecList fields; for (size_t i = 0; i < n.getChildren().size(); ++i) { fields.clear(); @@ -74,25 +73,21 @@ CreateBlueprintVisitorHelper::createWeightedSet(WS *bp, NODE &n) { uint32_t weight = getWeightFromNode(node).percent(); bp->addTerm(_searchable.createBlueprint(_requestContext, fields, node), weight); } - setResult(std::move(result)); + setResult(std::move(bp)); } void CreateBlueprintVisitorHelper::visitWeightedSetTerm(query::WeightedSetTerm &n) { - WeightedSetTermBlueprint *bp = new WeightedSetTermBlueprint(_field); - createWeightedSet(bp, n); + createWeightedSet(std::make_unique<WeightedSetTermBlueprint>(_field), n); } void CreateBlueprintVisitorHelper::visitDotProduct(query::DotProduct &n) { - DotProductBlueprint *bp = new DotProductBlueprint(_field); - createWeightedSet(bp, n); + createWeightedSet(std::make_unique<DotProductBlueprint>(_field), n); } void CreateBlueprintVisitorHelper::visitWandTerm(query::WandTerm &n) { - ParallelWeakAndBlueprint *bp = new ParallelWeakAndBlueprint(_field, - n.getTargetNumHits(), - n.getScoreThreshold(), - n.getThresholdBoostFactor()); - createWeightedSet(bp, n); + createWeightedSet(std::make_unique<ParallelWeakAndBlueprint>(_field, n.getTargetNumHits(), + n.getScoreThreshold(), n.getThresholdBoostFactor()), + n); } } diff --git a/searchlib/src/vespa/searchlib/queryeval/create_blueprint_visitor_helper.h b/searchlib/src/vespa/searchlib/queryeval/create_blueprint_visitor_helper.h index 5bcc4f4c4c5..84830111fde 100644 --- a/searchlib/src/vespa/searchlib/queryeval/create_blueprint_visitor_helper.h +++ b/searchlib/src/vespa/searchlib/queryeval/create_blueprint_visitor_helper.h @@ -25,10 +25,7 @@ protected: public: CreateBlueprintVisitorHelper(Searchable &searchable, const FieldSpec &field, const IRequestContext & requestContext); - ~CreateBlueprintVisitorHelper(); - - template <typename T> - std::unique_ptr<T> make_UP(T *p) { return std::unique_ptr<T>(p); } + ~CreateBlueprintVisitorHelper() override; template <typename T> void setResult(std::unique_ptr<T> result) { _result = std::move(result); } @@ -40,7 +37,7 @@ public: void visitPhrase(query::Phrase &n); template <typename WS, typename NODE> - void createWeightedSet(WS *bp, NODE &n); + void createWeightedSet(std::unique_ptr<WS> bp, NODE &n); void visitWeightedSetTerm(query::WeightedSetTerm &n); void visitDotProduct(query::DotProduct &n); void visitWandTerm(query::WandTerm &n); diff --git a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/QueueBoundsTest.java b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/QueueBoundsTest.java index 528d6a71c2d..1f875e0dd72 100644 --- a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/QueueBoundsTest.java +++ b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/QueueBoundsTest.java @@ -28,7 +28,6 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; /** - * Only runs on screwdriver to save time! * * @author Einar M R Rosenvinge */ diff --git a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPIMultiClusterTest.java b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPIMultiClusterTest.java index 4edd731136b..5fbc367c620 100644 --- a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPIMultiClusterTest.java +++ b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPIMultiClusterTest.java @@ -19,10 +19,8 @@ import static com.yahoo.vespa.http.client.TestUtils.getResults; import static com.yahoo.vespa.http.client.V3HttpAPITest.documents; import static org.hamcrest.CoreMatchers.*; import static org.junit.Assert.assertThat; -import static org.junit.Assert.fail; /** - * Only runs on screwdriver to save time! * * @author Einar M R Rosenvinge */ diff --git a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPITest.java b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPITest.java index cd8d9f26b07..1d70ce953e4 100644 --- a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPITest.java +++ b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPITest.java @@ -24,7 +24,6 @@ import static org.hamcrest.CoreMatchers.nullValue; import static org.junit.Assert.assertThat; /** - * Only runs on screwdriver to save time! * * @author Einar M R Rosenvinge */ diff --git a/vespalog/src/logger/runserver.cpp b/vespalog/src/logger/runserver.cpp index 8ea669697ee..e58565f78b9 100644 --- a/vespalog/src/logger/runserver.cpp +++ b/vespalog/src/logger/runserver.cpp @@ -5,6 +5,7 @@ #include <fcntl.h> #include <cerrno> #include <unistd.h> +#include <csignal> #include <sys/select.h> #include <sys/types.h> |