diff options
13 files changed, 300 insertions, 445 deletions
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStateView.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStateView.java index a0e93c2a9ad..ea638010ab7 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStateView.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStateView.java @@ -29,31 +29,25 @@ public class ClusterStateView { private static Logger log = Logger.getLogger(ClusterStateView.class.getName()); private final ClusterState clusterState; private final ClusterStatsAggregator statsAggregator; - private final MetricUpdater metricUpdater; - /** - * @param metricUpdater may be null, in which case no stats will be reported. - */ - public static ClusterStateView create(String serializedClusterState, MetricUpdater metricUpdater) - throws ParseException { + public static ClusterStateView create(String serializedClusterState) throws ParseException { ClusterState clusterState = new ClusterState(serializedClusterState); - return new ClusterStateView(clusterState, createNewAggregator(clusterState, metricUpdater), metricUpdater); + return new ClusterStateView(clusterState, createNewAggregator(clusterState)); } - public static ClusterStateView create(final ClusterState clusterState, final MetricUpdater metricUpdater) { - return new ClusterStateView(clusterState, createNewAggregator(clusterState, metricUpdater), metricUpdater); + public static ClusterStateView create(final ClusterState clusterState) { + return new ClusterStateView(clusterState, createNewAggregator(clusterState)); } - private static ClusterStatsAggregator createNewAggregator(ClusterState clusterState, MetricUpdater metricUpdater) { + private static ClusterStatsAggregator createNewAggregator(ClusterState clusterState) { Set<Integer> upDistributors = getIndicesOfUpNodes(clusterState, NodeType.DISTRIBUTOR); Set<Integer> upStorageNodes = getIndicesOfUpNodes(clusterState, NodeType.STORAGE); - return new ClusterStatsAggregator(upDistributors, upStorageNodes, metricUpdater); + return new ClusterStatsAggregator(upDistributors, upStorageNodes); } - ClusterStateView(ClusterState clusterState, ClusterStatsAggregator statsAggregator, MetricUpdater metricUpdater) { + ClusterStateView(ClusterState clusterState, ClusterStatsAggregator statsAggregator) { this.clusterState = clusterState; this.statsAggregator = statsAggregator; - this.metricUpdater = metricUpdater; } /** @@ -85,8 +79,7 @@ public class ClusterStateView { ClusterState clonedClusterState = clusterState.clone(); return new ClusterStateView( clonedClusterState, - createNewAggregator(clonedClusterState, metricUpdater), - metricUpdater); + createNewAggregator(clonedClusterState)); } public ClusterState getClusterState() { return clusterState; } @@ -115,8 +108,8 @@ public class ClusterStateView { return; } - statsAggregator.updateForDistributor( - hostnames, node.getNodeIndex(), StorageNodeStatsBridge.generate(hostInfo.getDistributor())); + statsAggregator.updateForDistributor(node.getNodeIndex(), + StorageNodeStatsBridge.generate(hostInfo.getDistributor())); } public String toString() { diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregator.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregator.java index e1e626a7a71..3f7cd129fc1 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregator.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregator.java @@ -4,16 +4,13 @@ package com.yahoo.vespa.clustercontroller.core; import java.util.HashMap; import java.util.Map; import java.util.Set; -import java.util.logging.Logger; - -import com.yahoo.log.LogLevel; /** - * A class that stores stats about outstanding merge operations for - * the current cluster state version, and exports metrics about these. + * Class that stores content cluster stats (with bucket space stats per node) for + * the current cluster state version. * - * Each distributor reports outstanding merge operations for the different - * storage nodes. These reports arrive with getnodestate RPC calls, + * Each distributor reports bucket space stats for the different content nodes. + * These reports arrive with getnodestate RPC calls, * and eventually ends up as calls to updateForDistributor(). * No assumptions are made on the sequence of getnodestate calls. * For instance, it's perfectly fine for the calls to arrive in the @@ -25,95 +22,53 @@ import com.yahoo.log.LogLevel; * distributor 2 * ... etc * - * Whereas the metrics we want, is how many merge operations are outstanding - * for a given storage nodes. So we need to keep track of the latest info - * from each distributor. - * * @author hakonhall */ public class ClusterStatsAggregator { - private static Logger log = Logger.getLogger(ClusterStatsAggregator.class.getName()); - private final Set<Integer> distributors; - private final MetricUpdater updater; - // Maps the distributor node index to a map of storage node index to the - // storage node's merge stats. + // Maps the distributor node index to a map of content node index to the + // content node's stats. private final Map<Integer, ContentClusterStats> distributorToStats = new HashMap<>(); - // This is only needed as an optimization. should just be the sum of distributorToStats' ContentClusterStats. - // Maps the storage node index to the aggregate merge stats for that storage node. + // This is only needed as an optimization. Is just the sum of distributorToStats' ContentClusterStats. + // Maps the content node index to the content node stats for that node. // This MUST be kept up-to-date with distributorToStats; private final ContentClusterStats aggregatedStats; - private int hostToStatsMapHashCode = 0; - - ClusterStatsAggregator(Set<Integer> distributors, Set<Integer> storageNodes, MetricUpdater updater) { + ClusterStatsAggregator(Set<Integer> distributors, Set<Integer> storageNodes) { this.distributors = distributors; aggregatedStats = new ContentClusterStats(storageNodes); - this.updater = updater; + } + + ContentClusterStats getAggregatedStats() { + return aggregatedStats; } /** * Update the aggregator with the newest available stats from a distributor. - * Will update metrics if necessary. */ - void updateForDistributor(Map<Integer, String> hostnames, int distributorIndex, ContentClusterStats clusterStats) { + void updateForDistributor(int distributorIndex, ContentClusterStats clusterStats) { if (!distributors.contains(distributorIndex)) { return; } - addStatsFromDistributor(distributorIndex, clusterStats); - - if (distributorToStats.size() < distributors.size()) { - // Not all distributors have reported their merge stats through getnodestate yet. - return; - } - - Map<String, ContentNodeStats> hostToStatsMap = getHostToStatsMap(hostnames); - if (hostToStatsMap == null) { - return; - } - - if (hostToStatsMapHashCode == 0 || hostToStatsMapHashCode != hostToStatsMap.hashCode()) { - updater.updateMergeOpMetrics(hostToStatsMap); - hostToStatsMapHashCode = hostToStatsMap.hashCode(); - } - } - - private Map<String, ContentNodeStats> getHostToStatsMap(Map<Integer, String> hostnames) { - Map<String, ContentNodeStats> hostToStatsMap = new HashMap<>(aggregatedStats.size()); - for (ContentNodeStats nodeStats : aggregatedStats) { - // The hosts names are kept up-to-date from Slobrok, and MAY therefore be arbitrarily - // different from the node set used by aggregatedStats (and typically tied to a cluster state). - // If so, we will not pretend the returned map is complete, and will return null. - String host = hostnames.get(nodeStats.getNodeIndex()); - if (host == null) { - log.log(LogLevel.DEBUG, "Failed to find the host name of storage node " + nodeStats.getNodeIndex() + - ". Skipping the report from " + ClusterStatsAggregator.class.getName()); - return null; - } - - hostToStatsMap.put(host, nodeStats); - } - - return hostToStatsMap; } private void addStatsFromDistributor(int distributorIndex, ContentClusterStats clusterStats) { - ContentClusterStats previousStorageStats = distributorToStats.put(distributorIndex, clusterStats); + ContentClusterStats prevClusterStats = distributorToStats.put(distributorIndex, clusterStats); for (ContentNodeStats contentNode : aggregatedStats) { Integer nodeIndex = contentNode.getNodeIndex(); - ContentNodeStats statsToAdd = clusterStats.getStorageNode(nodeIndex); + ContentNodeStats statsToAdd = clusterStats.getContentNode(nodeIndex); if (statsToAdd != null) { contentNode.add(statsToAdd); } - if (previousStorageStats != null) { - ContentNodeStats statsToSubtract = clusterStats.getStorageNode(nodeIndex); + if (prevClusterStats != null) { + ContentNodeStats statsToSubtract = prevClusterStats.getContentNode(nodeIndex); if (statsToSubtract != null) { contentNode.subtract(statsToSubtract); } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStats.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStats.java index 154113b95c0..ec64b2e8147 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStats.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStats.java @@ -1,19 +1,16 @@ // 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 java.util.HashMap; -import java.util.Iterator; -import java.util.Map; -import java.util.Set; +import java.util.*; /** - * Class for storing the pending merge operation stats for all the content nodes. + * Class for storing pending content node stats for all content nodes in the cluster. * * @author hakonhall */ public class ContentClusterStats implements Iterable<ContentNodeStats> { - // Maps a storage node index to the storage node's pending merges stats. + // Maps a content node index to the content node's stats. private final Map<Integer, ContentNodeStats> mapToNodeStats; public ContentClusterStats(Set<Integer> storageNodes) { @@ -32,7 +29,7 @@ public class ContentClusterStats implements Iterable<ContentNodeStats> { return mapToNodeStats.values().iterator(); } - ContentNodeStats getStorageNode(Integer index) { + ContentNodeStats getContentNode(Integer index) { return mapToNodeStats.get(index); } @@ -43,21 +40,19 @@ public class ContentClusterStats implements Iterable<ContentNodeStats> { @Override public boolean equals(Object o) { if (this == o) return true; - if (!(o instanceof ContentClusterStats)) { - return false; - } - + if (o == null || getClass() != o.getClass()) return false; ContentClusterStats that = (ContentClusterStats) o; - - if (mapToNodeStats != null ? !mapToNodeStats.equals(that.mapToNodeStats) : that.mapToNodeStats != null) { - return false; - } - return true; + return Objects.equals(mapToNodeStats, that.mapToNodeStats); } @Override public int hashCode() { - return mapToNodeStats != null ? mapToNodeStats.hashCode() : 0; + return Objects.hash(mapToNodeStats); } + @Override + public String toString() { + return String.format("{mapToNodeStats=[%s]}", + Arrays.toString(mapToNodeStats.entrySet().toArray())); + } } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStats.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStats.java index 11e05a74cf7..f589468ccdf 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStats.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStats.java @@ -3,149 +3,130 @@ package com.yahoo.vespa.clustercontroller.core; import com.yahoo.vespa.clustercontroller.core.hostinfo.StorageNode; +import java.util.*; + /** * @author hakonhall */ public class ContentNodeStats { - /** - * Constructor that sets values to zero if not present. - */ - public ContentNodeStats(StorageNode storageNodePojo) { - this.nodeIndex = storageNodePojo.getIndex(); + private int nodeIndex; + private Map<String, BucketSpaceStats> bucketSpaces = new HashMap<>(); - StorageNode.OutstandingMergeOps mergeOps = storageNodePojo.getOutstandingMergeOpsOrNull(); - if (mergeOps == null) { - mergeOps = new StorageNode.OutstandingMergeOps(); - } - syncing = createAmount(mergeOps.getSyncingOrNull()); - copyingIn = createAmount(mergeOps.getCopyingInOrNull()); - movingOut = createAmount(mergeOps.getMovingOutOrNull()); - copyingOut = createAmount(mergeOps.getCopyingOutOrNull()); - } + public static class BucketSpaceStats { + private long bucketsTotal; + private long bucketsPending; - private static Amount createAmount(StorageNode.Buckets bucketOrNull) { - if (bucketOrNull == null) { - return new Amount(); + public BucketSpaceStats() { + this.bucketsTotal = 0; + this.bucketsPending = 0; } - return new Amount(bucketOrNull.getBuckets()); - } - - static public class Amount { - private long buckets; - Amount() { this(0); } - Amount(long buckets) { this.buckets = buckets; } + public BucketSpaceStats(long bucketsTotal, long bucketsPending) { + this.bucketsTotal = bucketsTotal; + this.bucketsPending = bucketsPending; + } - public void set(Amount other) { - buckets = other.buckets; + public long getBucketsTotal() { + return bucketsTotal; } - public long getBuckets() { - return buckets; + public long getBucketsPending() { + return bucketsPending; } - /** - * Logically, add (factor * amount) to this object. - */ - void scaledAdd(int factor, Amount amount) { - buckets += factor * amount.buckets; + public void merge(BucketSpaceStats rhs, int factor) { + this.bucketsTotal += (factor * rhs.bucketsTotal); + this.bucketsPending += (factor * rhs.bucketsPending); } - public boolean equals(Object other) { - if (!(other instanceof Amount)) { - return false; - } - Amount otherAmount = (Amount) other; - return buckets == otherAmount.buckets; + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + BucketSpaceStats that = (BucketSpaceStats) o; + return bucketsTotal == that.bucketsTotal && + bucketsPending == that.bucketsPending; } + @Override public int hashCode() { - return (int)buckets; + return Objects.hash(bucketsTotal, bucketsPending); } + @Override public String toString() { - return String.format("{buckets = %d}", buckets); + return "{bucketsTotal=" + bucketsTotal + ", bucketsPending=" + bucketsPending + "}"; } } - private final Amount syncing; - private final Amount copyingIn; - private final Amount movingOut; - private final Amount copyingOut; - private int nodeIndex; + public ContentNodeStats(StorageNode storageNode) { + this.nodeIndex = storageNode.getIndex(); + for (StorageNode.BucketSpaceStats stats : storageNode.getBucketSpacesStats()) { + if (stats.valid()) { + this.bucketSpaces.put(stats.getName(), + new BucketSpaceStats(stats.getBucketStats().getTotal(), + stats.getBucketStats().getPending())); + } else { + // TODO: better handling of invalid bucket space stats + this.bucketSpaces.put(stats.getName(), new BucketSpaceStats()); + } + } + } - /** - * An instance with all 0 amounts. - */ public ContentNodeStats(int index) { - this(index, new Amount(), new Amount(), new Amount(), new Amount()); + this(index, new HashMap<>()); } - ContentNodeStats(int index, Amount syncing, Amount copyingIn, Amount movingOut, Amount copyingOut) { + public ContentNodeStats(int index, Map<String, BucketSpaceStats> bucketSpaces) { this.nodeIndex = index; - this.syncing = syncing; - this.copyingIn = copyingIn; - this.movingOut = movingOut; - this.copyingOut = copyingOut; + this.bucketSpaces = bucketSpaces; } - public void set(ContentNodeStats stats) { - nodeIndex = stats.nodeIndex; - syncing.set(stats.syncing); - copyingIn.set(stats.copyingIn); - movingOut.set(stats.movingOut); - copyingOut.set(stats.copyingOut); - } + public int getNodeIndex() { return nodeIndex; } - int getNodeIndex() { return nodeIndex; } - public Amount getSyncing() { return syncing; } - public Amount getCopyingIn() { return copyingIn; } - public Amount getMovingOut() { return movingOut; } - public Amount getCopyingOut() { return copyingOut; } + public void add(ContentNodeStats stats) { + merge(stats, 1); + } - void add(ContentNodeStats stats) { - scaledAdd(1, stats); + public void subtract(ContentNodeStats stats) { + merge(stats, -1); } - void subtract(ContentNodeStats stats) { - scaledAdd(-1, stats); + private void merge(ContentNodeStats stats, int factor) { + for (Map.Entry<String, BucketSpaceStats> entry : stats.bucketSpaces.entrySet()) { + BucketSpaceStats statsToUpdate = bucketSpaces.get(entry.getKey()); + if (statsToUpdate == null && factor == 1) { + statsToUpdate = new BucketSpaceStats(); + bucketSpaces.put(entry.getKey(), statsToUpdate); + } + if (statsToUpdate != null) { + statsToUpdate.merge(entry.getValue(), factor); + } + } } - /** - * Logically, adds (factor * stats) to this object. factor of 1 is normal add, -1 is subtraction. - */ - private void scaledAdd(int factor, ContentNodeStats stats) { - syncing.scaledAdd(factor, stats.syncing); - copyingIn.scaledAdd(factor, stats.copyingIn); - movingOut.scaledAdd(factor, stats.movingOut); - copyingOut.scaledAdd(factor, stats.copyingOut); + public Map<String, BucketSpaceStats> getBucketSpaces() { + return bucketSpaces; } @Override - public int hashCode() { - return (int) (syncing.buckets + - copyingIn.buckets * 31 + - movingOut.buckets * 17 + - copyingOut.buckets * 7); + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ContentNodeStats that = (ContentNodeStats) o; + return nodeIndex == that.nodeIndex && + Objects.equals(bucketSpaces, that.bucketSpaces); } @Override - public boolean equals(Object other) { - if (!(other instanceof ContentNodeStats)) { - return false; - } - - ContentNodeStats otherStats = (ContentNodeStats) other; - return nodeIndex == otherStats.nodeIndex && - syncing.equals(otherStats.syncing) && - copyingIn.equals(otherStats.copyingIn) && - movingOut.equals(otherStats.movingOut) && - copyingOut.equals(otherStats.copyingOut); + public int hashCode() { + return Objects.hash(nodeIndex, bucketSpaces); } + @Override public String toString() { - return String.format("{index = %d, syncing = %s, copyingIn = %s, movingOut = %s, copyingOut = %s}", - nodeIndex, syncing, copyingIn, movingOut, copyingOut); + return String.format("{index=%d, bucketSpaces=[%s]}", + nodeIndex, Arrays.toString(bucketSpaces.entrySet().toArray())); } } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java index a5419c64818..ba56d75ab4c 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java @@ -122,7 +122,7 @@ public class FleetController implements NodeStateOrHostInfoChangeHandler, NodeAd this.stateGatherer = nodeStateGatherer; this.stateChangeHandler = stateChangeHandler; this.systemStateBroadcaster = systemStateBroadcaster; - this.stateVersionTracker = new StateVersionTracker(metricUpdater); + this.stateVersionTracker = new StateVersionTracker(); this.metricUpdater = metricUpdater; this.statusPageServer = statusPage; diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MetricUpdater.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MetricUpdater.java index 7c921be958b..199e9a3169b 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MetricUpdater.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MetricUpdater.java @@ -85,7 +85,4 @@ public class MetricUpdater { metricReporter.add("node-event", 1); } - public void updateMergeOpMetrics(Map<String, ContentNodeStats> contentNodeStats) { - // TODO(hakonhall): Remove this method once we figure out how to propagate metrics to state HTTP API. - } } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/StateVersionTracker.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/StateVersionTracker.java index 518361f18fc..902189fca8b 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/StateVersionTracker.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/StateVersionTracker.java @@ -32,15 +32,13 @@ public class StateVersionTracker { private AnnotatedClusterState latestCandidateState = AnnotatedClusterState.emptyState(); private AnnotatedClusterState currentClusterState = latestCandidateState; - private final MetricUpdater metricUpdater; private ClusterStateView clusterStateView; private final LinkedList<ClusterStateHistoryEntry> clusterStateHistory = new LinkedList<>(); private int maxHistoryEntryCount = 50; - StateVersionTracker(final MetricUpdater metricUpdater) { - this.metricUpdater = metricUpdater; - clusterStateView = ClusterStateView.create(currentUnversionedState, metricUpdater); + StateVersionTracker() { + clusterStateView = ClusterStateView.create(currentUnversionedState); } void setVersionRetrievedFromZooKeeper(final int version) { @@ -121,7 +119,7 @@ public class StateVersionTracker { lowestObservedDistributionBits, newState.getClusterState().getDistributionBitCount()); // TODO should this take place in updateLatestCandidateState instead? I.e. does it require a consolidated state? - clusterStateView = ClusterStateView.create(currentClusterState.getClusterState(), metricUpdater); + clusterStateView = ClusterStateView.create(currentClusterState.getClusterState()); } private void recordCurrentStateInHistoryAtTime(final long currentTimeMs) { diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/hostinfo/StorageNode.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/hostinfo/StorageNode.java index 9e951236c45..bf7c1fad6ca 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/hostinfo/StorageNode.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/hostinfo/StorageNode.java @@ -4,6 +4,9 @@ package com.yahoo.vespa.clustercontroller.core.hostinfo; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import java.util.ArrayList; +import java.util.List; + /** * Keeping information about a storage node seen from the distributor. * @@ -36,31 +39,43 @@ public class StorageNode { public Put getPut() { return put; } } - static public class Buckets { - private final long buckets; + static public class BucketStats { + private final long total; + private final long pending; @JsonCreator - public Buckets(@JsonProperty("buckets") Long buckets) { - this.buckets = buckets; + public BucketStats(@JsonProperty("total") Long total, @JsonProperty("pending") Long pending) { + this.total = total; + this.pending = pending; } - public long getBuckets() { return buckets; } + public long getTotal() { + return total; + } + public long getPending() { + return pending; + } } - static public class OutstandingMergeOps { - @JsonProperty("syncing") - private Buckets syncing; - @JsonProperty("copying-in") - private Buckets copyingIn; - @JsonProperty("moving-out") - private Buckets movingOut; - @JsonProperty("copying-out") - private Buckets copyingOut; - - public Buckets getSyncingOrNull() { return syncing; } - public Buckets getCopyingInOrNull() { return copyingIn; } - public Buckets getMovingOutOrNull() { return movingOut; } - public Buckets getCopyingOutOrNull() { return copyingOut; } + static public class BucketSpaceStats { + private final String name; + @JsonProperty("buckets") + private BucketStats bucketStats = null; + + @JsonCreator + public BucketSpaceStats(@JsonProperty("name") String name) { + this.name = name; + } + + public String getName() { + return name; + } + public boolean valid() { + return bucketStats != null; + } + public BucketStats getBucketStats() { + return bucketStats; + } } private final Integer index; @@ -74,8 +89,8 @@ public class StorageNode { @JsonProperty("min-current-replication-factor") private Integer minCurrentReplicationFactor; - @JsonProperty("outstanding-merge-ops") - private OutstandingMergeOps outstandingMergeOps; + @JsonProperty("bucket-spaces") + private List<BucketSpaceStats> bucketSpacesStats = new ArrayList<>(); @JsonCreator public StorageNode(@JsonProperty("node-index") Integer index) { @@ -95,8 +110,7 @@ public class StorageNode { return minCurrentReplicationFactor; } - public OutstandingMergeOps getOutstandingMergeOpsOrNull() { - return outstandingMergeOps; + public List<BucketSpaceStats> getBucketSpacesStats() { + return bucketSpacesStats; } - } 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 6487a55b554..dc8a4a0d441 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 @@ -23,8 +23,7 @@ public class ClusterStateViewTest { final Node node = mock(Node.class); final ClusterStatsAggregator statsAggregator = mock(ClusterStatsAggregator.class); final ClusterState clusterState = mock(ClusterState.class); - final MetricUpdater metricUpdater = mock(MetricUpdater.class); - final ClusterStateView clusterStateView = new ClusterStateView(clusterState, statsAggregator, metricUpdater); + final ClusterStateView clusterStateView = new ClusterStateView(clusterState, statsAggregator); HostInfo createHostInfo(String version) { return HostInfo.createHostInfo("{ \"cluster-state-version\": " + version + " }"); @@ -36,7 +35,7 @@ public class ClusterStateViewTest { clusterStateView.handleUpdatedHostInfo(hostnames, nodeInfo, createHostInfo("101")); - verify(statsAggregator, never()).updateForDistributor(any(), anyInt(), any()); + verify(statsAggregator, never()).updateForDistributor(anyInt(), any()); } @@ -48,7 +47,7 @@ public class ClusterStateViewTest { clusterStateView.handleUpdatedHostInfo(hostnames, nodeInfo, createHostInfo("22")); - verify(statsAggregator, never()).updateForDistributor(any(), anyInt(), any()); + verify(statsAggregator, never()).updateForDistributor(anyInt(), any()); } @Test @@ -58,7 +57,7 @@ public class ClusterStateViewTest { clusterStateView.handleUpdatedHostInfo(hostnames, nodeInfo, createHostInfo("22")); - verify(statsAggregator, never()).updateForDistributor(any(), anyInt(), any()); + verify(statsAggregator, never()).updateForDistributor(anyInt(), any()); } @Test @@ -80,8 +79,7 @@ public class ClusterStateViewTest { clusterStateView.handleUpdatedHostInfo(hostnames, nodeInfo, hostInfo); - verify(statsAggregator).updateForDistributor( - hostnames, 3, StorageNodeStatsBridge.generate(hostInfo.getDistributor())); + verify(statsAggregator).updateForDistributor(3, StorageNodeStatsBridge.generate(hostInfo.getDistributor())); } @Test 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 e5bda010faf..6035ae9d8ce 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 @@ -1,217 +1,142 @@ // 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.google.common.collect.Sets; import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.runners.MockitoJUnitRunner; import java.util.*; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.any; -import static org.mockito.Mockito.*; /** * @author hakonhall * @since 5.34 */ -@RunWith(MockitoJUnitRunner.class) public class ClusterStatsAggregatorTest { - final Set<Integer> distributors = new HashSet<>(); - final Set<Integer> storageNodes = new HashSet<>(); - final Map<Integer, String> hostnames = new HashMap<>(); - final MetricUpdater updater = mock(MetricUpdater.class); - ContentClusterStats clusterStats; + private static class StatsBuilder { + private final Map<Integer, Map<String, ContentNodeStats.BucketSpaceStats> > stats = new HashMap<>(); - private void addDistributors(Integer... indices) { - for (Integer i : indices) { - distributors.add(i); + public StatsBuilder add(int nodeIndex, String bucketSpace, long bucketsTotal, long bucketsPending) { + return add(nodeIndex, bucketSpace, new ContentNodeStats.BucketSpaceStats(bucketsTotal, bucketsPending)); } - } - - private static class StorageNodeSpec { - public StorageNodeSpec(Integer index, String hostname) { - this.index = index; - this.hostname = hostname; + public StatsBuilder add(int nodeIndex, String bucketSpace) { + return add(nodeIndex, bucketSpace, new ContentNodeStats.BucketSpaceStats()); + } + public StatsBuilder add(int nodeIndex, String bucketSpace, ContentNodeStats.BucketSpaceStats bucketSpaceStats) { + Map<String, ContentNodeStats.BucketSpaceStats> contentNodeStats = stats.get(nodeIndex); + if (contentNodeStats == null) { + contentNodeStats = new HashMap<>(); + stats.put(nodeIndex, contentNodeStats); + } + contentNodeStats.put(bucketSpace, bucketSpaceStats); + return this; + } + public StatsBuilder add(int nodeIndex) { + stats.put(nodeIndex, new HashMap<>()); + return this; + } + public ContentClusterStats build() { + Map<Integer, ContentNodeStats> nodeToStatsMap = new HashMap<>(); + stats.forEach((nodeIndex, bucketSpaces) -> + nodeToStatsMap.put(nodeIndex, new ContentNodeStats(nodeIndex, bucketSpaces))); + return new ContentClusterStats(nodeToStatsMap); } - public Integer index; - public String hostname; } - private void addStorageNodes(StorageNodeSpec... specs) { - for (StorageNodeSpec spec : specs) { - storageNodes.add(spec.index); - hostnames.put(spec.index, spec.hostname); + private static class Fixture { + private ClusterStatsAggregator aggregator; + public Fixture(Set<Integer> distributorNodes, + Set<Integer> contentNodes) { + aggregator = new ClusterStatsAggregator(distributorNodes, contentNodes); + } + public void update(int distributorIndex, StatsBuilder clusterStats) { + aggregator.updateForDistributor(distributorIndex, clusterStats.build()); + } + public void verify(StatsBuilder expectedStats) { + assertEquals(expectedStats.build(), aggregator.getAggregatedStats()); } - clusterStats = new ContentClusterStats(storageNodes); } - private void putStorageStats(int index, int syncing, int copyingIn, int movingOut, int copyingOut) { - clusterStats.getStorageNode(index).set(createStats(index, syncing, copyingIn, movingOut, copyingOut)); + private static Set<Integer> distributorNodes(Integer... indices) { + return Sets.newHashSet(indices); } - private static ContentNodeStats createStats(int index, int syncing, int copyingIn, int movingOut, int copyingOut) { - return new ContentNodeStats( - index, - new ContentNodeStats.Amount(syncing), - new ContentNodeStats.Amount(copyingIn), - new ContentNodeStats.Amount(movingOut), - new ContentNodeStats.Amount(copyingOut)); + private static Set<Integer> contentNodes(Integer... indices) { + return Sets.newHashSet(indices); } @Test - public void testSimple() { - final int distributorIndex = 1; - addDistributors(distributorIndex); - - final int storageNodeIndex = 11; - addStorageNodes(new StorageNodeSpec(storageNodeIndex, "storage-node")); - - putStorageStats(storageNodeIndex, 5, 6, 7, 8); - - ClusterStatsAggregator aggregator = new ClusterStatsAggregator(distributors, storageNodes, updater); - aggregator.updateForDistributor(hostnames, distributorIndex, clusterStats); - - Map<String, ContentNodeStats> expectedContentNodeStats = new HashMap<>(); - expectedContentNodeStats.put("storage-node", createStats(storageNodeIndex, 5, 6, 7, 8)); - - verify(updater).updateMergeOpMetrics(expectedContentNodeStats); + public void aggregator_handles_updates_to_single_distributor_and_content_node() { + Fixture f = new Fixture(distributorNodes(1), contentNodes(3)); + StatsBuilder stats = new StatsBuilder() + .add(3, "default", 10, 1) + .add(3, "global", 11, 2); + f.update(1, stats); + f.verify(stats); } @Test - public void testComplex() { - final int distributor1 = 1; - final int distributor2 = 2; - addDistributors(distributor1, distributor2); - - final int storageNode1 = 11; - final int storageNode2 = 12; - addStorageNodes( - new StorageNodeSpec(storageNode1, "storage-node-1"), - new StorageNodeSpec(storageNode2, "storage-node-2")); - - ClusterStatsAggregator aggregator = new ClusterStatsAggregator(distributors, storageNodes, updater); - - // Distributor 1. - putStorageStats(storageNode1, 0, 1, 2, 3); - putStorageStats(storageNode2, 20, 21, 22, 23); - aggregator.updateForDistributor(hostnames, distributor1, clusterStats); - - // Distributor 2. - putStorageStats(storageNode1, 10, 11, 12, 13); - putStorageStats(storageNode2, 30, 31, 32, 33); - aggregator.updateForDistributor(hostnames, distributor2, clusterStats); - - Map<String, ContentNodeStats> expectedContentNodeStats = new HashMap<>(); - expectedContentNodeStats.put("storage-node-1", createStats(storageNode1, 0 + 10, 1 + 11, 2 + 12, 3 + 13)); - expectedContentNodeStats.put("storage-node-2", createStats(storageNode2, 20 + 30, 21 + 31, 22 + 32, 23 + 33)); - - verify(updater, times(1)).updateMergeOpMetrics(expectedContentNodeStats); + public void aggregator_handles_updates_to_multiple_distributors_and_content_nodes() { + Fixture f = new Fixture(distributorNodes(1, 2), contentNodes(3, 4)); + + f.update(1, new StatsBuilder() + .add(3, "default", 10, 1) + .add(3, "global", 11, 2) + .add(4, "default", 12, 3) + .add(4, "global", 13, 4)); + f.update(2, new StatsBuilder() + .add(3, "default", 14, 5) + .add(3, "global", 15, 6) + .add(4, "default", 16, 7) + .add(4, "global", 17, 8)); + f.verify(new StatsBuilder() + .add(3, "default", 10 + 14, 1 + 5) + .add(3, "global", 11 + 15, 2 + 6) + .add(4, "default", 12 + 16, 3 + 7) + .add(4, "global", 13 + 17, 4 + 8)); } @Test - public void testHashCodeCache() { - final int distributor1 = 1; - final int distributor2 = 2; - addDistributors(distributor1, distributor2); + public void aggregator_handles_multiple_updates_from_same_distributor() { + Fixture f = new Fixture(distributorNodes(1, 2), contentNodes(3)); - final int storageNode1 = 11; - final int storageNode2 = 12; - addStorageNodes( - new StorageNodeSpec(storageNode1, "storage-node-1"), - new StorageNodeSpec(storageNode2, "storage-node-2")); + f.update(1, new StatsBuilder().add(3, "default")); + f.verify(new StatsBuilder().add(3, "default")); - ClusterStatsAggregator aggregator = new ClusterStatsAggregator(distributors, storageNodes, updater); + f.update(2, new StatsBuilder().add(3, "default", 10, 1)); + f.verify(new StatsBuilder().add(3, "default", 10, 1)); - // Distributor 1. - putStorageStats(storageNode1, 0, 1, 2, 3); - putStorageStats(storageNode2, 20, 21, 22, 23); - aggregator.updateForDistributor(hostnames, distributor1, clusterStats); + f.update(1, new StatsBuilder().add(3, "default", 11, 2)); + f.verify(new StatsBuilder().add(3, "default", 10 + 11, 1 + 2)); - // Distributor 2. - putStorageStats(storageNode1, 10, 11, 12, 13); - putStorageStats(storageNode2, 30, 31, 32, 33); - aggregator.updateForDistributor(hostnames, distributor2, clusterStats); + f.update(2, new StatsBuilder().add(3, "default", 15, 6)); + f.verify(new StatsBuilder().add(3, "default", 11 + 15, 2 + 6)); - // If we add call another updateForDistributor with the same arguments, updateMergeOpMetrics() should not be called. - // See times(1) below. - aggregator.updateForDistributor(hostnames, distributor2, clusterStats); + f.update(1, new StatsBuilder().add(3, "default", 16, 7)); + f.verify(new StatsBuilder().add(3, "default", 15 + 16, 6 + 7)); - Map<String, ContentNodeStats> expectedContentNodeStats = new HashMap<>(); - expectedContentNodeStats.put("storage-node-1", createStats(storageNode1, 0 + 10, 1 + 11, 2 + 12, 3 + 13)); - expectedContentNodeStats.put("storage-node-2", createStats(storageNode2, 20 + 30, 21 + 31, 22 + 32, 23 + 33)); - - - verify(updater, times(1)).updateMergeOpMetrics(expectedContentNodeStats); + f.update(2, new StatsBuilder().add(3, "default", 12, 3)); + f.verify(new StatsBuilder().add(3, "default", 16 + 12, 7 + 3)); } @Test - public void testUnknownDistributor() { - final int upDistributor = 1; - final int DownDistributorIndex = 2; - addDistributors(upDistributor); - - final int storageNodeIndex = 11; - addStorageNodes(new StorageNodeSpec(storageNodeIndex, "storage-node")); - - putStorageStats(storageNodeIndex, 5, 6, 7, 8); - - ClusterStatsAggregator aggregator = new ClusterStatsAggregator(distributors, storageNodes, updater); - aggregator.updateForDistributor(hostnames, DownDistributorIndex, clusterStats); - - verify(updater, never()).updateMergeOpMetrics(any()); + public void aggregator_handles_more_content_nodes_that_distributors() { + Fixture f = new Fixture(distributorNodes(1), contentNodes(3, 4)); + StatsBuilder stats = new StatsBuilder() + .add(3, "default", 10, 1) + .add(4, "default", 11, 2); + f.update(1, stats); + f.verify(stats); } @Test - public void testMoreStorageNodesThanDistributors() { - final int distributor1 = 1; - addDistributors(distributor1); - - final int storageNode1 = 11; - final int storageNode2 = 12; - addStorageNodes( - new StorageNodeSpec(storageNode1, "storage-node-1"), - new StorageNodeSpec(storageNode2, "storage-node-2")); - - ClusterStatsAggregator aggregator = new ClusterStatsAggregator(distributors, storageNodes, updater); - - // Distributor 1. - putStorageStats(storageNode1, 0, 1, 2, 3); - putStorageStats(storageNode2, 20, 21, 22, 23); - aggregator.updateForDistributor(hostnames, distributor1, clusterStats); - - Map<String, ContentNodeStats> expectedContentNodeStats = new HashMap<>(); - expectedContentNodeStats.put("storage-node-1", createStats(storageNode1, 0, 1, 2, 3)); - expectedContentNodeStats.put("storage-node-2", createStats(storageNode2, 20, 21, 22, 23)); - - verify(updater, times(1)).updateMergeOpMetrics(expectedContentNodeStats); + public void aggregator_ignores_updates_to_unknown_distributor() { + Fixture f = new Fixture(distributorNodes(1), contentNodes(3)); + final int downDistributorIndex = 2; + f.update(downDistributorIndex, new StatsBuilder() + .add(3, "default", 7, 3)); + f.verify(new StatsBuilder().add(3)); } - @Test - public void testMoreDistributorsThanStorageNodes() { - final int distributor1 = 1; - final int distributor2 = 2; - addDistributors(distributor1, distributor2); - - final int storageNode1 = 11; - addStorageNodes(new StorageNodeSpec(storageNode1, "storage-node-1")); - - ClusterStatsAggregator aggregator = new ClusterStatsAggregator(distributors, storageNodes, updater); - - // Distributor 1. - putStorageStats(storageNode1, 0, 1, 2, 3); - aggregator.updateForDistributor(hostnames, distributor1, clusterStats); - - // Distributor 2. - putStorageStats(storageNode1, 10, 11, 12, 13); - aggregator.updateForDistributor(hostnames, distributor2, clusterStats); - - Map<String, ContentNodeStats> expectedContentNodeStats = new HashMap<>(); - expectedContentNodeStats.put("storage-node-1", createStats(storageNode1, 0 + 10, 1 + 11, 2 + 12, 3 + 13)); - - verify(updater, times(1)).updateMergeOpMetrics(expectedContentNodeStats); - } } 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 0af7bcbfeaf..d27acf8bc17 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 @@ -11,14 +11,11 @@ import org.junit.Test; import java.util.Arrays; import java.util.Optional; -import static org.hamcrest.CoreMatchers.hasItems; import static org.hamcrest.core.IsEqual.equalTo; import static org.hamcrest.core.Is.is; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; -import static org.mockito.Mockito.mock; public class StateVersionTrackerTest { @@ -28,7 +25,7 @@ public class StateVersionTrackerTest { } private static StateVersionTracker createWithMockedMetrics() { - return new StateVersionTracker(mock(MetricUpdater.class)); + return new StateVersionTracker(); } private static void updateAndPromote(final StateVersionTracker versionTracker, diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/hostinfo/StorageNodeStatsBridgeTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/hostinfo/StorageNodeStatsBridgeTest.java index c9ce54ae3be..5319d741503 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/hostinfo/StorageNodeStatsBridgeTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/hostinfo/StorageNodeStatsBridgeTest.java @@ -12,11 +12,11 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Iterator; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.core.Is.is; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.*; /** * @author hakonhall @@ -49,19 +49,30 @@ public class StorageNodeStatsBridgeTest { } @Test - public void testStorageMergeStats() throws IOException { + public void testContentNodeStats() throws IOException { String data = getJsonString(); HostInfo hostInfo = HostInfo.createHostInfo(data); ContentClusterStats clusterStats = StorageNodeStatsBridge.generate(hostInfo.getDistributor()); - int size = 0; - for (ContentNodeStats nodeStats : clusterStats) { - assertThat(nodeStats.getCopyingIn().getBuckets(), is(2L)); - assertThat(nodeStats.getCopyingOut().getBuckets(), is(4L)); - assertThat(nodeStats.getSyncing().getBuckets(), is(1L)); - assertThat(nodeStats.getMovingOut().getBuckets(), is(3L)); - size++; + Iterator<ContentNodeStats> itr = clusterStats.iterator(); + { // content node 0 + ContentNodeStats stats = itr.next(); + assertThat(stats.getNodeIndex(), is(0)); + assertThat(stats.getBucketSpaces().size(), is(2)); + assertBucketSpaceStats(11, 3, stats.getBucketSpaces().get("default")); + assertBucketSpaceStats(13, 5, stats.getBucketSpaces().get("global")); } - assertThat(size, is(2)); + { // content node 1 + ContentNodeStats stats = itr.next(); + assertThat(stats.getNodeIndex(), is(1)); + assertThat(stats.getBucketSpaces().size(), is(1)); + assertBucketSpaceStats(0, 0, stats.getBucketSpaces().get("default")); + } + assertFalse(itr.hasNext()); + } + + private static void assertBucketSpaceStats(long expBucketsTotal, long expBucketsPending, ContentNodeStats.BucketSpaceStats stats) { + assertThat(stats.getBucketsTotal(), is(expBucketsTotal)); + assertThat(stats.getBucketsPending(), is(expBucketsPending)); } } diff --git a/protocols/getnodestate/host_info.json b/protocols/getnodestate/host_info.json index 5ef62d1a84f..305f279c847 100644 --- a/protocols/getnodestate/host_info.json +++ b/protocols/getnodestate/host_info.json @@ -48,20 +48,22 @@ "count": 16 } }, - "outstanding-merge-ops": { - "syncing": { - "buckets": 1 + "bucket-spaces": [ + { + "name": "default", + "buckets": { + "total": 11, + "pending": 3 + } }, - "copying-in": { - "buckets": 2 - }, - "moving-out": { - "buckets": 3 - }, - "copying-out": { - "buckets": 4 + { + "name": "global", + "buckets": { + "total": 13, + "pending": 5 + } } - } + ] }, { "node-index": 1, @@ -71,24 +73,13 @@ "latency-ms-sum": 17, "count": 18 } - }, - "outstanding-merge-ops": { - "syncing": { - "buckets": 1 - }, - "copying-in": { - "buckets": 2 - }, - "moving-out": { - "buckets": 3 - }, - "copying-out": { - "buckets": 4 + }, + "bucket-spaces": [ + { + "name": "default" } - } - + ] } - ] } } |