From 162cbebd066dd0205b13d1b2041b23747b58bde1 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Wed, 17 Aug 2022 16:31:48 +0530 Subject: [PATCH 01/31] Weighted round-robin scheduling policy for shard coordination traffic routing Signed-off-by: Anshu Agarwal --- .../metadata/WeightedRoundRobinMetadata.java | 157 ++++++++++++++ .../routing/IndexShardRoutingTable.java | 60 +++++- .../cluster/routing/OperationRouting.java | 53 ++++- .../opensearch/cluster/routing/WRRWeight.java | 77 +++++++ .../cluster/routing/WeightedRoundRobin.java | 134 ++++++++++++ .../common/settings/ClusterSettings.java | 1 + .../routing/OperationRoutingTests.java | 198 ++++++++++++++++++ 7 files changed, 671 insertions(+), 9 deletions(-) create mode 100644 server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinMetadata.java create mode 100644 server/src/main/java/org/opensearch/cluster/routing/WRRWeight.java create mode 100644 server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java diff --git a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinMetadata.java new file mode 100644 index 0000000000000..f863c47e66758 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinMetadata.java @@ -0,0 +1,157 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.metadata; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.OpenSearchParseException; +import org.opensearch.Version; +import org.opensearch.cluster.AbstractNamedDiffable; +import org.opensearch.cluster.NamedDiff; +import org.opensearch.cluster.routing.WRRWeight; +import org.opensearch.common.Strings; +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.io.stream.StreamOutput; +import org.opensearch.common.xcontent.ToXContent; +import org.opensearch.common.xcontent.XContentBuilder; +import org.opensearch.common.xcontent.XContentParser; + +import java.io.IOException; +import java.util.EnumSet; +import java.util.HashMap; +import java.util.Map; + +/** + * Contains metadata for weighted round-robin shard routing weights + * + * @opensearch.internal + */ +public class WeightedRoundRobinMetadata extends AbstractNamedDiffable implements Metadata.Custom { + private static final Logger logger = LogManager.getLogger(WeightedRoundRobinMetadata.class); + public static final String TYPE = "wrr_shard_routing"; + private WRRWeight wrrWeight; + + public WRRWeight getWrrWeight() { + return wrrWeight; + } + + public WeightedRoundRobinMetadata setWrrWeight(WRRWeight wrrWeight) { + this.wrrWeight = wrrWeight; + return this; + } + + public WeightedRoundRobinMetadata(StreamInput in) throws IOException { + this.wrrWeight = new WRRWeight(in); + } + + public WeightedRoundRobinMetadata(WRRWeight wrrWeight) { + this.wrrWeight = wrrWeight; + } + + @Override + public EnumSet context() { + return Metadata.API_AND_GATEWAY; + } + + @Override + public String getWriteableName() { + return TYPE; + } + + @Override + public Version getMinimalSupportedVersion() { + // TODO: Check if this needs to be changed + return Version.CURRENT.minimumCompatibilityVersion(); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + wrrWeight.writeTo(out); + } + + public static NamedDiff readDiffFrom(StreamInput in) throws IOException { + return readDiffFrom(Metadata.Custom.class, TYPE, in); + } + + public static WeightedRoundRobinMetadata fromXContent(XContentParser parser) throws IOException { + String attrKey = null; + Object attrValue; + String attributeName = null; + Map weights = new HashMap<>(); + WRRWeight wrrWeight = null; + XContentParser.Token token; + // move to the first alias + parser.nextToken(); + String awarenessField = null; + + while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { + if (token == XContentParser.Token.FIELD_NAME) { + awarenessField = parser.currentName(); + if (parser.nextToken() != XContentParser.Token.START_OBJECT) { + throw new OpenSearchParseException("failed to parse wrr metadata [{}], expected object", awarenessField); + } + while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { + attributeName = parser.currentName(); + if (parser.nextToken() != XContentParser.Token.START_OBJECT) { + throw new OpenSearchParseException("failed to parse wrr metadata [{}], expected object", attributeName); + } + while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { + if (token == XContentParser.Token.FIELD_NAME) { + attrKey = parser.currentName(); + } else if (token == XContentParser.Token.VALUE_STRING) { + attrValue = parser.text(); + weights.put(attrKey, attrValue); + } else { + throw new OpenSearchParseException("failed to parse wrr metadata attribute [{}], unknown type", attributeName); + } + } + } + } else { + throw new OpenSearchParseException("failed to parse wrr metadata attribute [{}]", attributeName); + } + } + wrrWeight = new WRRWeight(attributeName, weights); + return new WeightedRoundRobinMetadata(wrrWeight); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + WeightedRoundRobinMetadata that = (WeightedRoundRobinMetadata) o; + return wrrWeight.equals(that.wrrWeight); + } + + @Override + public int hashCode() { + return wrrWeight.hashCode(); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException { + toXContent(wrrWeight, builder); + return builder; + } + + public static void toXContent(WRRWeight wrrWeight, XContentBuilder builder) throws IOException { + builder.startObject("awareness"); + builder.startObject(wrrWeight.attributeName()); + for (Map.Entry entry : wrrWeight.weights().entrySet()) { + builder.field(entry.getKey(), entry.getValue()); + } + builder.endObject(); + builder.endObject(); + } + + @Override + public String toString() { + return Strings.toString(this); + } + +} diff --git a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java index d4597f47d9a6c..2006f1bd3ef08 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java +++ b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java @@ -43,7 +43,6 @@ import org.opensearch.index.Index; import org.opensearch.index.shard.ShardId; import org.opensearch.node.ResponseCollectorService; - import java.io.IOException; import java.util.ArrayList; import java.util.Collections; @@ -292,6 +291,65 @@ public ShardIterator activeInitializingShardsRankedIt( return new PlainShardIterator(shardId, ordered); } + /** + * * + * @param wrrWeight Weighted round-robin weight entity + * @param nodes discovered nodes in the cluster + * @return an interator over active and initializing shards, ordered by weighted round-robin + * scheduling policy. Making sure that initializing shards are the last to iterate through. + */ + public ShardIterator activeInitializingShardsWRR(WRRWeight wrrWeight, DiscoveryNodes nodes) { + final int seed = shuffler.nextSeed(); + ArrayList ordered = new ArrayList<>(activeShards.size() + allInitializingShards.size()); + ArrayList orderedActiveShards = getShardsWRR(activeShards, wrrWeight, nodes); + ordered.addAll(shuffler.shuffle(orderedActiveShards, seed)); + if (!allInitializingShards.isEmpty()) { + ArrayList orderedInitializingShards = getShardsWRR(allInitializingShards, wrrWeight, nodes); + ordered.addAll(shuffler.shuffle(orderedInitializingShards, seed)); + } + return new PlainShardIterator(shardId, ordered); + } + + /** + * + * @param shards shards to be ordered using weighted round-robin scheduling policy + * @param wrrWeight weights to be considered for routing + * @param nodes discovered nodes in the cluster + * @return list of shards ordered using weighted round-robin scheduling. + */ + private ArrayList getShardsWRR(List shards, WRRWeight wrrWeight, DiscoveryNodes nodes) { + List> weightedShards = calculateShardWeight(shards, wrrWeight, nodes); + WeightedRoundRobin wrr = new WeightedRoundRobin<>(weightedShards); + List> wrrOrderedActiveShards = wrr.orderEntities(); + ArrayList orderedActiveShards = new ArrayList<>(activeShards.size()); + for (WeightedRoundRobin.Entity shardRouting : wrrOrderedActiveShards) { + orderedActiveShards.add(shardRouting.getTarget()); + } + return orderedActiveShards; + } + + /** + * * + * @param shards associate weights to shards + * @param wrrWeight weights to be used for association + * @param nodes + * @return list of entity containing shard routing and associated weight. + */ + private List> calculateShardWeight( + List shards, + WRRWeight wrrWeight, + DiscoveryNodes nodes + ) { + List> weightedShards = new ArrayList<>(); + for (ShardRouting shard : shards) { + shard.currentNodeId(); + DiscoveryNode node = nodes.get(shard.currentNodeId()); + String attVal = node.getAttributes().get(wrrWeight.attributeName()); + weightedShards.add(new WeightedRoundRobin.Entity<>(Double.parseDouble(wrrWeight.weights().get(attVal).toString()), shard)); + } + return weightedShards; + } + private static Set getAllNodeIds(final List shards) { final Set nodeIds = new HashSet<>(); for (ShardRouting shard : shards) { diff --git a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java index 30f6408c19783..19209330eb46b 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java +++ b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java @@ -34,6 +34,7 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.cluster.metadata.WeightedRoundRobinMetadata; import org.opensearch.cluster.node.DiscoveryNodes; import org.opensearch.cluster.routing.allocation.decider.AwarenessAllocationDecider; import org.opensearch.common.Nullable; @@ -68,6 +69,12 @@ public class OperationRouting { Setting.Property.NodeScope ); + public static final Setting USE_WEIGHTED_ROUND_ROBIN = Setting.boolSetting( + "cluster.routing.use_weighted_round_robin", + true, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); public static final String IGNORE_AWARENESS_ATTRIBUTES = "cluster.search.ignore_awareness_attributes"; public static final Setting IGNORE_AWARENESS_ATTRIBUTES_SETTING = Setting.boolSetting( IGNORE_AWARENESS_ATTRIBUTES, @@ -79,6 +86,8 @@ public class OperationRouting { private volatile boolean useAdaptiveReplicaSelection; private volatile boolean ignoreAwarenessAttr; + private volatile boolean useWeightedRoundRobin; + public OperationRouting(Settings settings, ClusterSettings clusterSettings) { // whether to ignore awareness attributes when routing requests this.ignoreAwarenessAttr = clusterSettings.get(IGNORE_AWARENESS_ATTRIBUTES_SETTING); @@ -88,8 +97,11 @@ public OperationRouting(Settings settings, ClusterSettings clusterSettings) { this::setAwarenessAttributes ); this.useAdaptiveReplicaSelection = USE_ADAPTIVE_REPLICA_SELECTION_SETTING.get(settings); + this.useWeightedRoundRobin = USE_WEIGHTED_ROUND_ROBIN.get(settings); clusterSettings.addSettingsUpdateConsumer(USE_ADAPTIVE_REPLICA_SELECTION_SETTING, this::setUseAdaptiveReplicaSelection); clusterSettings.addSettingsUpdateConsumer(IGNORE_AWARENESS_ATTRIBUTES_SETTING, this::setIgnoreAwarenessAttributes); + clusterSettings.addSettingsUpdateConsumer(USE_WEIGHTED_ROUND_ROBIN, this::setUseWeightedRoundRobin); + } void setUseAdaptiveReplicaSelection(boolean useAdaptiveReplicaSelection) { @@ -100,6 +112,14 @@ void setIgnoreAwarenessAttributes(boolean ignoreAwarenessAttributes) { this.ignoreAwarenessAttr = ignoreAwarenessAttributes; } + public boolean isUseWeightedRoundRobin() { + return useWeightedRoundRobin; + } + + public void setUseWeightedRoundRobin(boolean useWeightedRoundRobin) { + this.useWeightedRoundRobin = useWeightedRoundRobin; + } + public boolean isIgnoreAwarenessAttr() { return ignoreAwarenessAttr; } @@ -169,14 +189,22 @@ public GroupShardsIterator searchShards( final Set shards = computeTargetedShards(clusterState, concreteIndices, routing); final Set set = new HashSet<>(shards.size()); for (IndexShardRoutingTable shard : shards) { - ShardIterator iterator = preferenceActiveShardIterator( - shard, - clusterState.nodes().getLocalNodeId(), - clusterState.nodes(), - preference, - collectorService, - nodeCounts - ); + ShardIterator iterator = null; + // TODO: Do we need similar changes in getShards call?? + if (isWeightedRoundRobinEnabled(clusterState)) { + WeightedRoundRobinMetadata weightedRoundRobinMetadata = clusterState.metadata().custom(WeightedRoundRobinMetadata.TYPE); + iterator = shard.activeInitializingShardsWRR(weightedRoundRobinMetadata.getWrrWeight(), clusterState.nodes()); + } else { + iterator = preferenceActiveShardIterator( + shard, + clusterState.nodes().getLocalNodeId(), + clusterState.nodes(), + preference, + collectorService, + nodeCounts + ); + } + if (iterator != null) { set.add(iterator); } @@ -184,6 +212,14 @@ public GroupShardsIterator searchShards( return GroupShardsIterator.sortAndCreate(new ArrayList<>(set)); } + private boolean isWeightedRoundRobinEnabled(ClusterState clusterState) { + WeightedRoundRobinMetadata weightedRoundRobinMetadata = clusterState.metadata().custom(WeightedRoundRobinMetadata.TYPE); + if (useWeightedRoundRobin && weightedRoundRobinMetadata != null) { + return true; + } + return false; + } + public static ShardIterator getShards(ClusterState clusterState, ShardId shardId) { final IndexShardRoutingTable shard = clusterState.routingTable().shardRoutingTable(shardId); return shard.activeInitializingShardsRandomIt(); @@ -227,6 +263,7 @@ private ShardIterator preferenceActiveShardIterator( @Nullable ResponseCollectorService collectorService, @Nullable Map nodeCounts ) { + if (preference == null || preference.isEmpty()) { return shardRoutings(indexShard, nodes, collectorService, nodeCounts); } diff --git a/server/src/main/java/org/opensearch/cluster/routing/WRRWeight.java b/server/src/main/java/org/opensearch/cluster/routing/WRRWeight.java new file mode 100644 index 0000000000000..f27f7c88c7f55 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/routing/WRRWeight.java @@ -0,0 +1,77 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.routing; + +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.io.stream.StreamOutput; +import org.opensearch.common.io.stream.Writeable; + +import java.io.IOException; +import java.util.Map; +import java.util.Objects; + +/** + * Entity for Weighted Round Robin weights + * + * @opensearch.internal + */ +public class WRRWeight implements Writeable { + private String attributeName; + private Map weights; + + public WRRWeight(String attributeName, Map weights) { + this.attributeName = attributeName; + this.weights = weights; + } + + public WRRWeight(WRRWeight wrrWeight) { + this.attributeName = wrrWeight.attributeName(); + this.weights = wrrWeight.weights; + } + + public WRRWeight(StreamInput in) throws IOException { + attributeName = in.readString(); + weights = in.readMap(); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(attributeName); + out.writeMap(weights); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + WRRWeight that = (WRRWeight) o; + + if (!attributeName.equals(that.attributeName)) return false; + return weights.equals(that.weights); + } + + @Override + public int hashCode() { + return Objects.hash(attributeName, weights); + } + + @Override + public String toString() { + return "WRRWeightsDefinition{" + attributeName + "}{" + weights().toString() + "}"; + } + + public Map weights() { + return this.weights; + } + + public String attributeName() { + return this.attributeName; + } +} diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java new file mode 100644 index 0000000000000..0e00b1aa5cedd --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java @@ -0,0 +1,134 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.routing; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; + +public class WeightedRoundRobin implements Iterator, Iterable { + + private List> entities; + private int turn; + private int lastSelectedEntity; + private double currentWeight = 0; + + public WeightedRoundRobin(List> entities) { + this.entities = entities; + this.turn = 0; + this.lastSelectedEntity = -1; + } + + /* (non-Javadoc) + * @see java.util.Iterator#hasNext() + */ + @Override + public boolean hasNext() { + return entities.size() > 0; + } + + /* (non-Javadoc) + * @see java.util.Iterator#next() + */ + @Override + public T next() { + Entity entity = entities.get(turn++); + return entity.getTarget(); + } + + /* (non-Javadoc) + * @see java.lang.Iterable#iterator() + */ + @Override + public Iterator iterator() { + return this; + } + + /** + * * + * @return list of entities that is ordered using weighted round-robin scheduling + * http://kb.linuxvirtualserver.org/wiki/Weighted_Round-Robin_Scheduling + */ + public List> orderEntities() { + int size = entities.size(); + List> orderedWeight = new ArrayList<>(); + if (size <= 0) { + return null; + } + if (size == 1) { + return entities; + } + // Find maximum weight and greatest common divisor of weight across all entities + double maxWeight = 0; + double sumWeight = 0; + Double gcd = null; + for (WeightedRoundRobin.Entity entity : entities) { + maxWeight = Math.max(maxWeight, entity.getWeight()); + gcd = (gcd == null) ? entity.getWeight() : gcd(gcd, entity.getWeight()); + sumWeight += entity.getWeight(); + } + int count = 0; + while (count < sumWeight) { + lastSelectedEntity = (lastSelectedEntity + 1) % size; + if (lastSelectedEntity == 0) { + currentWeight = currentWeight - gcd; + if (currentWeight <= 0) { + currentWeight = maxWeight; + if (currentWeight == 0) { + return orderedWeight; + } + } + } + if (entities.get(lastSelectedEntity).getWeight() >= currentWeight) { + orderedWeight.add(entities.get(lastSelectedEntity)); + count++; + } + } + return orderedWeight; + } + + /** + * Return greatest common divisor for two integers + * https://en.wikipedia.org/wiki/Greatest_common_divisor#Using_Euclid.27s_algorithm + * + * @param a + * @param b + * @return greatest common divisor + */ + private double gcd(double a, double b) { + return (b == 0) ? a : gcd(b, a % b); + } + + static final class Entity { + private double weight; + private T target; + + public Entity(double weight, T target) { + this.weight = weight; + this.target = target; + } + + public T getTarget() { + return this.target; + } + + public void setTarget(T target) { + this.target = target; + } + + public double getWeight() { + return this.weight; + } + + public void setWeight(double weight) { + this.weight = weight; + } + } + +} diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index 971fb518ff1da..bc06987624918 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -529,6 +529,7 @@ public void apply(Settings value, Settings current, Settings previous) { Node.BREAKER_TYPE_KEY, OperationRouting.USE_ADAPTIVE_REPLICA_SELECTION_SETTING, OperationRouting.IGNORE_AWARENESS_ATTRIBUTES_SETTING, + OperationRouting.USE_WEIGHTED_ROUND_ROBIN, IndexGraveyard.SETTING_MAX_TOMBSTONES, PersistentTasksClusterService.CLUSTER_TASKS_ALLOCATION_RECHECK_INTERVAL_SETTING, EnableAssignmentDecider.CLUSTER_TASKS_ALLOCATION_ENABLE_SETTING, diff --git a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java index 8bf2b1626292a..dae65adb4baa9 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java @@ -36,6 +36,7 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.Metadata; +import org.opensearch.cluster.metadata.WeightedRoundRobinMetadata; import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.node.DiscoveryNodeRole; import org.opensearch.cluster.routing.allocation.decider.AwarenessAllocationDecider; @@ -102,6 +103,7 @@ public void testGenerateShardId() { .settings(settings(Version.CURRENT)) .numberOfShards(shardSplits[2]) .numberOfReplicas(1) + .setRoutingNumShards(shardSplits[0]) .build(); shrunkShard = OperationRouting.generateShardId(shrunk, term, null); @@ -759,6 +761,111 @@ public void testAdaptiveReplicaSelectionWithZoneAwarenessIgnored() throws Except terminate(threadPool); } + public void testWRR() throws Exception { + final int numIndices = 2; + final int numShards = 3; + final int numReplicas = 2; + final String[] indexNames = new String[numIndices]; + for (int i = 0; i < numIndices; i++) { + indexNames[i] = "test" + i; + } + DiscoveryNode[] allNodes = setUpNodesWRR(); + ClusterState state = ClusterStateCreationUtils.state(allNodes[0], allNodes[6], allNodes); + + Map> discoveryNodeMap = new HashMap<>(); + List nodesZoneA = new ArrayList<>(); + nodesZoneA.add(allNodes[0]); + nodesZoneA.add(allNodes[1]); + + List nodesZoneB = new ArrayList<>(); + nodesZoneB.add(allNodes[2]); + nodesZoneB.add(allNodes[3]); + + List nodesZoneC = new ArrayList<>(); + nodesZoneC.add(allNodes[4]); + nodesZoneC.add(allNodes[5]); + discoveryNodeMap.put("a", nodesZoneA); + discoveryNodeMap.put("b", nodesZoneB); + discoveryNodeMap.put("c", nodesZoneC); + + state = updateStatetoTestWRR(indexNames, numShards, numReplicas, state, discoveryNodeMap); + + Settings setting = Settings.builder() + .put("cluster.routing.allocation.awareness.attributes", "zone") + .put("cluster.routing.use_weighted_round_robin", "true") + .build(); + + TestThreadPool threadPool = new TestThreadPool("testThatOnlyNodesSupport"); + ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool); + + OperationRouting opRouting = new OperationRouting( + setting, + new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); + + assertTrue(opRouting.ignoreAwarenessAttributes()); + Set selectedNodes = new HashSet<>(); + ResponseCollectorService collector = new ResponseCollectorService(clusterService); + Map outstandingRequests = new HashMap<>(); + + Map weights = Map.of("a", "1", "b", "1", "c", "0"); + WRRWeight wrrWeight = new WRRWeight("zone", weights); + WeightedRoundRobinMetadata wrrMetadata = new WeightedRoundRobinMetadata(wrrWeight); + Metadata.Builder metadataBuilder = Metadata.builder(state.metadata()); + metadataBuilder.putCustom(WeightedRoundRobinMetadata.TYPE, wrrMetadata); + state = ClusterState.builder(state).metadata(metadataBuilder).build(); + + GroupShardsIterator groupIterator = opRouting.searchShards( + state, + indexNames, + null, + null, + collector, + outstandingRequests + ); + + for (ShardIterator it : groupIterator) { + List shardRoutings = Collections.singletonList(it.nextOrNull()); + for (ShardRouting shardRouting : shardRoutings) { + selectedNodes.add(shardRouting.currentNodeId()); + } + } + for (String nodeID : selectedNodes) { + // No shards are assigned to nodes in zone c since its weight is 0 + assertFalse(nodeID.contains("c")); + } + + selectedNodes = new HashSet<>(); + setting = Settings.builder() + .put("cluster.routing.allocation.awareness.attributes", "zone") + .put("cluster.routing.use_weighted_round_robin", "true") + .build(); + + weights = Map.of("a", "1", "b", "0", "c", "1"); + wrrWeight = new WRRWeight("zone", weights); + wrrMetadata = new WeightedRoundRobinMetadata(wrrWeight); + metadataBuilder = Metadata.builder(state.metadata()); + metadataBuilder.putCustom(WeightedRoundRobinMetadata.TYPE, wrrMetadata); + state = ClusterState.builder(state).metadata(metadataBuilder).build(); + + opRouting = new OperationRouting(setting, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)); + + groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); + + for (ShardIterator it : groupIterator) { + List shardRoutings = Collections.singletonList(it.nextOrNull()); + for (ShardRouting shardRouting : shardRoutings) { + selectedNodes.add(shardRouting.currentNodeId()); + } + } + for (String nodeID : selectedNodes) { + // No shards are assigned to nodes in zone b since its weight is 0 + assertFalse(nodeID.contains("b")); + } + IOUtils.close(clusterService); + terminate(threadPool); + } + private DiscoveryNode[] setupNodes() { // Sets up two data nodes in zone-a and one data node in zone-b List zones = Arrays.asList("a", "a", "b"); @@ -785,6 +892,32 @@ private DiscoveryNode[] setupNodes() { return allNodes; } + private DiscoveryNode[] setUpNodesWRR() { + List zones = Arrays.asList("a", "a", "b", "b", "c", "c"); + DiscoveryNode[] allNodes = new DiscoveryNode[7]; + int i = 0; + for (String zone : zones) { + DiscoveryNode node = new DiscoveryNode( + "node_" + zone + "_" + i, + buildNewFakeTransportAddress(), + singletonMap("zone", zone), + Collections.singleton(DiscoveryNodeRole.DATA_ROLE), + Version.CURRENT + ); + allNodes[i++] = node; + } + + DiscoveryNode clusterManager = new DiscoveryNode( + "cluster-manager", + buildNewFakeTransportAddress(), + Collections.emptyMap(), + Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), + Version.CURRENT + ); + allNodes[i] = clusterManager; + return allNodes; + } + public void testAllocationAwarenessDeprecation() { OperationRouting routing = new OperationRouting( Settings.builder() @@ -841,4 +974,69 @@ private ClusterState updateStatetoTestARS( clusterState.routingTable(routingTableBuilder.build()); return clusterState.build(); } + + private ClusterState updateStatetoTestWRR( + String[] indices, + int numberOfShards, + int numberOfReplicas, + ClusterState state, + Map> discoveryNodeMap + ) { + RoutingTable.Builder routingTableBuilder = RoutingTable.builder(); + Metadata.Builder metadataBuilder = Metadata.builder(); + ClusterState.Builder clusterState = ClusterState.builder(state); + List nodesZoneA = discoveryNodeMap.get("a"); + List nodesZoneB = discoveryNodeMap.get("b"); + List nodesZoneC = discoveryNodeMap.get("c"); + for (String index : indices) { + IndexMetadata indexMetadata = IndexMetadata.builder(index) + .settings( + Settings.builder() + .put(SETTING_VERSION_CREATED, Version.CURRENT) + .put(SETTING_NUMBER_OF_SHARDS, numberOfShards) + .put(SETTING_NUMBER_OF_REPLICAS, numberOfReplicas) + .put(SETTING_CREATION_DATE, System.currentTimeMillis()) + ) + .build(); + metadataBuilder.put(indexMetadata, false).generateClusterUuidIfNeeded(); + IndexRoutingTable.Builder indexRoutingTableBuilder = IndexRoutingTable.builder(indexMetadata.getIndex()); + for (int i = 0; i < numberOfShards; i++) { + final ShardId shardId = new ShardId(index, "_na_", i); + IndexShardRoutingTable.Builder indexShardRoutingBuilder = new IndexShardRoutingTable.Builder(shardId); + // Assign all the primary shards on nodes in zone-a (node_a0 or node_a1) + indexShardRoutingBuilder.addShard( + TestShardRouting.newShardRouting( + index, + i, + nodesZoneA.get(randomInt(nodesZoneA.size() - 1)).getId(), + null, + true, + ShardRoutingState.STARTED + ) + ); + for (int replica = 0; replica < numberOfReplicas; replica++) { + // Assign all the replicas on nodes in zone-b (node_b2) + String nodeId = ""; + if (replica == 0) { + nodeId = nodesZoneB.get(randomInt(nodesZoneB.size() - 1)).getId(); + } else { + nodeId = nodesZoneC.get(randomInt(nodesZoneC.size() - 1)).getId(); + } + indexShardRoutingBuilder.addShard( + TestShardRouting.newShardRouting(index, i, nodeId, null, false, ShardRoutingState.STARTED) + ); + } + indexRoutingTableBuilder.addIndexShard(indexShardRoutingBuilder.build()); + } + routingTableBuilder.add(indexRoutingTableBuilder.build()); + } + // add wrr weights in metadata + Map weights = Map.of("a", "1", "b", "1", "c", "0"); + WRRWeight wrrWeight = new WRRWeight("zone", weights); + WeightedRoundRobinMetadata wrrMetadata = new WeightedRoundRobinMetadata(wrrWeight); + metadataBuilder.putCustom(WeightedRoundRobinMetadata.TYPE, wrrMetadata); + clusterState.metadata(metadataBuilder); + clusterState.routingTable(routingTableBuilder.build()); + return clusterState.build(); + } } From 8513c4f45edd922b93deba3d82daccec53817292 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Fri, 26 Aug 2022 10:01:05 +0530 Subject: [PATCH 02/31] Add caching layer for wrr shard routing and moved wrr routing call to shardRoutings Signed-off-by: Anshu Agarwal --- .../metadata/WeightedRoundRobinMetadata.java | 18 +-- .../routing/IndexShardRoutingTable.java | 17 ++- .../cluster/routing/OperationRouting.java | 78 ++++++++----- .../cluster/routing/WRRShardsCache.java | 84 ++++++++++++++ .../{WRRWeight.java => WRRWeights.java} | 10 +- .../cluster/routing/WeightedRoundRobin.java | 2 +- .../cluster/service/ClusterService.java | 1 + .../routing/OperationRoutingTests.java | 106 +++++++++++++++++- 8 files changed, 266 insertions(+), 50 deletions(-) create mode 100644 server/src/main/java/org/opensearch/cluster/routing/WRRShardsCache.java rename server/src/main/java/org/opensearch/cluster/routing/{WRRWeight.java => WRRWeights.java} (86%) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinMetadata.java index f863c47e66758..ed89f885bdb4a 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinMetadata.java @@ -14,7 +14,7 @@ import org.opensearch.Version; import org.opensearch.cluster.AbstractNamedDiffable; import org.opensearch.cluster.NamedDiff; -import org.opensearch.cluster.routing.WRRWeight; +import org.opensearch.cluster.routing.WRRWeights; import org.opensearch.common.Strings; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; @@ -35,22 +35,22 @@ public class WeightedRoundRobinMetadata extends AbstractNamedDiffable implements Metadata.Custom { private static final Logger logger = LogManager.getLogger(WeightedRoundRobinMetadata.class); public static final String TYPE = "wrr_shard_routing"; - private WRRWeight wrrWeight; + private WRRWeights wrrWeight; - public WRRWeight getWrrWeight() { + public WRRWeights getWrrWeight() { return wrrWeight; } - public WeightedRoundRobinMetadata setWrrWeight(WRRWeight wrrWeight) { + public WeightedRoundRobinMetadata setWrrWeight(WRRWeights wrrWeight) { this.wrrWeight = wrrWeight; return this; } public WeightedRoundRobinMetadata(StreamInput in) throws IOException { - this.wrrWeight = new WRRWeight(in); + this.wrrWeight = new WRRWeights(in); } - public WeightedRoundRobinMetadata(WRRWeight wrrWeight) { + public WeightedRoundRobinMetadata(WRRWeights wrrWeight) { this.wrrWeight = wrrWeight; } @@ -84,7 +84,7 @@ public static WeightedRoundRobinMetadata fromXContent(XContentParser parser) thr Object attrValue; String attributeName = null; Map weights = new HashMap<>(); - WRRWeight wrrWeight = null; + WRRWeights wrrWeight = null; XContentParser.Token token; // move to the first alias parser.nextToken(); @@ -116,7 +116,7 @@ public static WeightedRoundRobinMetadata fromXContent(XContentParser parser) thr throw new OpenSearchParseException("failed to parse wrr metadata attribute [{}]", attributeName); } } - wrrWeight = new WRRWeight(attributeName, weights); + wrrWeight = new WRRWeights(attributeName, weights); return new WeightedRoundRobinMetadata(wrrWeight); } @@ -139,7 +139,7 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params par return builder; } - public static void toXContent(WRRWeight wrrWeight, XContentBuilder builder) throws IOException { + public static void toXContent(WRRWeights wrrWeight, XContentBuilder builder) throws IOException { builder.startObject("awareness"); builder.startObject(wrrWeight.attributeName()); for (Map.Entry entry : wrrWeight.weights().entrySet()) { diff --git a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java index 2006f1bd3ef08..56abf80af1cdc 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java +++ b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java @@ -298,14 +298,21 @@ public ShardIterator activeInitializingShardsRankedIt( * @return an interator over active and initializing shards, ordered by weighted round-robin * scheduling policy. Making sure that initializing shards are the last to iterate through. */ - public ShardIterator activeInitializingShardsWRR(WRRWeight wrrWeight, DiscoveryNodes nodes) { + public ShardIterator activeInitializingShardsWRR(WRRWeights wrrWeight, DiscoveryNodes nodes, WRRShardsCache cache) { final int seed = shuffler.nextSeed(); ArrayList ordered = new ArrayList<>(activeShards.size() + allInitializingShards.size()); - ArrayList orderedActiveShards = getShardsWRR(activeShards, wrrWeight, nodes); + ArrayList orderedActiveShards; + if (cache.getCache().get(new WRRShardsCache.Key(shardId)) != null) { + orderedActiveShards = cache.getCache().get(new WRRShardsCache.Key(shardId)); + } else { + orderedActiveShards = getShardsWRR(activeShards, wrrWeight, nodes); + cache.getCache().put(new WRRShardsCache.Key(shardId), orderedActiveShards); + } + ordered.addAll(shuffler.shuffle(orderedActiveShards, seed)); if (!allInitializingShards.isEmpty()) { ArrayList orderedInitializingShards = getShardsWRR(allInitializingShards, wrrWeight, nodes); - ordered.addAll(shuffler.shuffle(orderedInitializingShards, seed)); + ordered.addAll(orderedInitializingShards); } return new PlainShardIterator(shardId, ordered); } @@ -317,7 +324,7 @@ public ShardIterator activeInitializingShardsWRR(WRRWeight wrrWeight, DiscoveryN * @param nodes discovered nodes in the cluster * @return list of shards ordered using weighted round-robin scheduling. */ - private ArrayList getShardsWRR(List shards, WRRWeight wrrWeight, DiscoveryNodes nodes) { + private ArrayList getShardsWRR(List shards, WRRWeights wrrWeight, DiscoveryNodes nodes) { List> weightedShards = calculateShardWeight(shards, wrrWeight, nodes); WeightedRoundRobin wrr = new WeightedRoundRobin<>(weightedShards); List> wrrOrderedActiveShards = wrr.orderEntities(); @@ -337,7 +344,7 @@ private ArrayList getShardsWRR(List shards, WRRWeigh */ private List> calculateShardWeight( List shards, - WRRWeight wrrWeight, + WRRWeights wrrWeight, DiscoveryNodes nodes ) { List> weightedShards = new ArrayList<>(); diff --git a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java index 19209330eb46b..d33b42bc69620 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java +++ b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java @@ -37,6 +37,7 @@ import org.opensearch.cluster.metadata.WeightedRoundRobinMetadata; import org.opensearch.cluster.node.DiscoveryNodes; import org.opensearch.cluster.routing.allocation.decider.AwarenessAllocationDecider; +import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.Nullable; import org.opensearch.common.Strings; import org.opensearch.common.settings.ClusterSettings; @@ -86,7 +87,32 @@ public class OperationRouting { private volatile boolean useAdaptiveReplicaSelection; private volatile boolean ignoreAwarenessAttr; + // reads value from cluster setting private volatile boolean useWeightedRoundRobin; + /** + * Reads value from cluster setting and cluster state to determine if weighted round-robin + * search routing is enabled + * This is true if useWeightedRoundRobin=true and weights are set in cluster metadata. + */ + private volatile boolean isWeightedRoundRobinEnabled; + + private volatile WRRWeights wrrWeights; + + public WRRShardsCache getWrrShardsCache() { + return wrrShardsCache; + } + + private WRRShardsCache wrrShardsCache; + + public ClusterService getClusterService() { + return clusterService; + } + + public void setClusterService(ClusterService clusterService) { + this.clusterService = clusterService; + } + + private ClusterService clusterService; public OperationRouting(Settings settings, ClusterSettings clusterSettings) { // whether to ignore awareness attributes when routing requests @@ -112,10 +138,6 @@ void setIgnoreAwarenessAttributes(boolean ignoreAwarenessAttributes) { this.ignoreAwarenessAttr = ignoreAwarenessAttributes; } - public boolean isUseWeightedRoundRobin() { - return useWeightedRoundRobin; - } - public void setUseWeightedRoundRobin(boolean useWeightedRoundRobin) { this.useWeightedRoundRobin = useWeightedRoundRobin; } @@ -136,6 +158,10 @@ public boolean ignoreAwarenessAttributes() { return this.awarenessAttributes.isEmpty() || this.ignoreAwarenessAttr; } + public WRRWeights getWrrWeights() { + return wrrWeights; + } + public ShardIterator indexShards(ClusterState clusterState, String index, String id, @Nullable String routing) { return shards(clusterState, index, id, routing).shardsIt(); } @@ -159,6 +185,7 @@ public ShardIterator getShards( public ShardIterator getShards(ClusterState clusterState, String index, int shardId, @Nullable String preference) { final IndexShardRoutingTable indexShard = clusterState.getRoutingTable().shardRoutingTable(index, shardId); + setWeightedRoundRobinAttributes(clusterState, getClusterService()); return preferenceActiveShardIterator( indexShard, clusterState.nodes().getLocalNodeId(), @@ -188,23 +215,16 @@ public GroupShardsIterator searchShards( ) { final Set shards = computeTargetedShards(clusterState, concreteIndices, routing); final Set set = new HashSet<>(shards.size()); + setWeightedRoundRobinAttributes(clusterState, getClusterService()); for (IndexShardRoutingTable shard : shards) { - ShardIterator iterator = null; - // TODO: Do we need similar changes in getShards call?? - if (isWeightedRoundRobinEnabled(clusterState)) { - WeightedRoundRobinMetadata weightedRoundRobinMetadata = clusterState.metadata().custom(WeightedRoundRobinMetadata.TYPE); - iterator = shard.activeInitializingShardsWRR(weightedRoundRobinMetadata.getWrrWeight(), clusterState.nodes()); - } else { - iterator = preferenceActiveShardIterator( - shard, - clusterState.nodes().getLocalNodeId(), - clusterState.nodes(), - preference, - collectorService, - nodeCounts - ); - } - + ShardIterator iterator = preferenceActiveShardIterator( + shard, + clusterState.nodes().getLocalNodeId(), + clusterState.nodes(), + preference, + collectorService, + nodeCounts + ); if (iterator != null) { set.add(iterator); } @@ -212,12 +232,18 @@ public GroupShardsIterator searchShards( return GroupShardsIterator.sortAndCreate(new ArrayList<>(set)); } - private boolean isWeightedRoundRobinEnabled(ClusterState clusterState) { + private void setWeightedRoundRobinAttributes(ClusterState clusterState, ClusterService clusterService) { WeightedRoundRobinMetadata weightedRoundRobinMetadata = clusterState.metadata().custom(WeightedRoundRobinMetadata.TYPE); - if (useWeightedRoundRobin && weightedRoundRobinMetadata != null) { - return true; + this.isWeightedRoundRobinEnabled = useWeightedRoundRobin && weightedRoundRobinMetadata != null ? true : false; + if (this.isWeightedRoundRobinEnabled) { + this.wrrWeights = weightedRoundRobinMetadata.getWrrWeight(); + this.wrrShardsCache = getWrrShardsCache() != null ? getWrrShardsCache() : new WRRShardsCache(clusterService); } - return false; + + } + + private boolean isWeightedRoundRobinEnabled() { + return isWeightedRoundRobinEnabled; } public static ShardIterator getShards(ClusterState clusterState, ShardId shardId) { @@ -337,7 +363,9 @@ private ShardIterator shardRoutings( @Nullable ResponseCollectorService collectorService, @Nullable Map nodeCounts ) { - if (ignoreAwarenessAttributes()) { + if (isWeightedRoundRobinEnabled()) { + return indexShard.activeInitializingShardsWRR(getWrrWeights(), nodes, wrrShardsCache); + } else if (ignoreAwarenessAttributes()) { if (useAdaptiveReplicaSelection) { return indexShard.activeInitializingShardsRankedIt(collectorService, nodeCounts); } else { diff --git a/server/src/main/java/org/opensearch/cluster/routing/WRRShardsCache.java b/server/src/main/java/org/opensearch/cluster/routing/WRRShardsCache.java new file mode 100644 index 0000000000000..966f185a29630 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/routing/WRRShardsCache.java @@ -0,0 +1,84 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.routing; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.cluster.ClusterChangedEvent; +import org.opensearch.cluster.ClusterStateListener; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.cache.Cache; +import org.opensearch.common.cache.CacheBuilder; +import org.opensearch.common.lease.Releasable; +import org.opensearch.index.shard.ShardId; + +import java.util.ArrayList; + +public class WRRShardsCache implements Releasable, ClusterStateListener { + private static final Logger logger = LogManager.getLogger(WRRShardsCache.class); + + private final Cache> cache; + + public WRRShardsCache(ClusterService clusterService) { + + final long sizeInBytes = 2000000; + CacheBuilder> cacheBuilder = CacheBuilder.>builder() + .removalListener(notification -> logger.info("Object" + " {} removed from cache", notification.getKey().shardId)) + .setMaximumWeight(sizeInBytes); + cache = cacheBuilder.build(); + clusterService.addListener(this); + } + + @Override + public void close() { + logger.info("Invalidating WRRShardsCache on close"); + cache.invalidateAll(); + } + + public Cache> getCache() { + return cache; + } + + @Override + public void clusterChanged(ClusterChangedEvent event) { + logger.info("Invalidating WRRShardsCache on ClusterChangedEvent"); + cache.invalidateAll(); + } + + /** + * Key for the WRRShardsCache + * + * @opensearch.internal + */ + public static class Key { + public final ShardId shardId; + + Key(ShardId shardId) { + + this.shardId = shardId; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + WRRShardsCache.Key key = (WRRShardsCache.Key) o; + if (!shardId.equals(key.shardId)) return false; + return true; + } + + @Override + public int hashCode() { + int result = shardId.hashCode(); + + return result; + } + + } +} diff --git a/server/src/main/java/org/opensearch/cluster/routing/WRRWeight.java b/server/src/main/java/org/opensearch/cluster/routing/WRRWeights.java similarity index 86% rename from server/src/main/java/org/opensearch/cluster/routing/WRRWeight.java rename to server/src/main/java/org/opensearch/cluster/routing/WRRWeights.java index f27f7c88c7f55..daf2986ea8b71 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WRRWeight.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WRRWeights.java @@ -21,21 +21,21 @@ * * @opensearch.internal */ -public class WRRWeight implements Writeable { +public class WRRWeights implements Writeable { private String attributeName; private Map weights; - public WRRWeight(String attributeName, Map weights) { + public WRRWeights(String attributeName, Map weights) { this.attributeName = attributeName; this.weights = weights; } - public WRRWeight(WRRWeight wrrWeight) { + public WRRWeights(WRRWeights wrrWeight) { this.attributeName = wrrWeight.attributeName(); this.weights = wrrWeight.weights; } - public WRRWeight(StreamInput in) throws IOException { + public WRRWeights(StreamInput in) throws IOException { attributeName = in.readString(); weights = in.readMap(); } @@ -51,7 +51,7 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - WRRWeight that = (WRRWeight) o; + WRRWeights that = (WRRWeights) o; if (!attributeName.equals(that.attributeName)) return false; return weights.equals(that.weights); diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java index 0e00b1aa5cedd..ad9702544a1e4 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java @@ -58,7 +58,7 @@ public Iterator iterator() { public List> orderEntities() { int size = entities.size(); List> orderedWeight = new ArrayList<>(); - if (size <= 0) { + if (size == 0) { return null; } if (size == 1) { diff --git a/server/src/main/java/org/opensearch/cluster/service/ClusterService.java b/server/src/main/java/org/opensearch/cluster/service/ClusterService.java index d393613118af8..4a8bf9275389a 100644 --- a/server/src/main/java/org/opensearch/cluster/service/ClusterService.java +++ b/server/src/main/java/org/opensearch/cluster/service/ClusterService.java @@ -108,6 +108,7 @@ public ClusterService( this.nodeName = Node.NODE_NAME_SETTING.get(settings); this.clusterManagerService = clusterManagerService; this.operationRouting = new OperationRouting(settings, clusterSettings); + this.operationRouting.setClusterService(this); this.clusterSettings = clusterSettings; this.clusterName = ClusterName.CLUSTER_NAME_SETTING.get(settings); // Add a no-op update consumer so changes are logged diff --git a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java index dae65adb4baa9..e5c07f1b3daf1 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java @@ -33,6 +33,7 @@ import org.opensearch.Version; import org.opensearch.action.support.replication.ClusterStateCreationUtils; +import org.opensearch.cluster.ClusterChangedEvent; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.Metadata; @@ -802,14 +803,14 @@ public void testWRR() throws Exception { setting, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) ); - + opRouting.setClusterService(clusterService); assertTrue(opRouting.ignoreAwarenessAttributes()); Set selectedNodes = new HashSet<>(); ResponseCollectorService collector = new ResponseCollectorService(clusterService); Map outstandingRequests = new HashMap<>(); Map weights = Map.of("a", "1", "b", "1", "c", "0"); - WRRWeight wrrWeight = new WRRWeight("zone", weights); + WRRWeights wrrWeight = new WRRWeights("zone", weights); WeightedRoundRobinMetadata wrrMetadata = new WeightedRoundRobinMetadata(wrrWeight); Metadata.Builder metadataBuilder = Metadata.builder(state.metadata()); metadataBuilder.putCustom(WeightedRoundRobinMetadata.TYPE, wrrMetadata); @@ -822,6 +823,7 @@ public void testWRR() throws Exception { null, collector, outstandingRequests + ); for (ShardIterator it : groupIterator) { @@ -842,13 +844,14 @@ public void testWRR() throws Exception { .build(); weights = Map.of("a", "1", "b", "0", "c", "1"); - wrrWeight = new WRRWeight("zone", weights); + wrrWeight = new WRRWeights("zone", weights); wrrMetadata = new WeightedRoundRobinMetadata(wrrWeight); metadataBuilder = Metadata.builder(state.metadata()); metadataBuilder.putCustom(WeightedRoundRobinMetadata.TYPE, wrrMetadata); state = ClusterState.builder(state).metadata(metadataBuilder).build(); opRouting = new OperationRouting(setting, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)); + opRouting.setClusterService(clusterService); groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); @@ -866,6 +869,99 @@ public void testWRR() throws Exception { terminate(threadPool); } + public void testWRRShardsCaching() throws Exception { + final int numIndices = 2; + final int numShards = 3; + final int numReplicas = 2; + final String[] indexNames = new String[numIndices]; + for (int i = 0; i < numIndices; i++) { + indexNames[i] = "test" + i; + } + DiscoveryNode[] allNodes = setUpNodesWRR(); + ClusterState state = ClusterStateCreationUtils.state(allNodes[0], allNodes[6], allNodes); + + Map> discoveryNodeMap = new HashMap<>(); + List nodesZoneA = new ArrayList<>(); + nodesZoneA.add(allNodes[0]); + nodesZoneA.add(allNodes[1]); + + List nodesZoneB = new ArrayList<>(); + nodesZoneB.add(allNodes[2]); + nodesZoneB.add(allNodes[3]); + + List nodesZoneC = new ArrayList<>(); + nodesZoneC.add(allNodes[4]); + nodesZoneC.add(allNodes[5]); + discoveryNodeMap.put("a", nodesZoneA); + discoveryNodeMap.put("b", nodesZoneB); + discoveryNodeMap.put("c", nodesZoneC); + + state = updateStatetoTestWRR(indexNames, numShards, numReplicas, state, discoveryNodeMap); + + Settings setting = Settings.builder() + .put("cluster.routing.allocation.awareness.attributes", "zone") + .put("cluster.routing.use_weighted_round_robin", "true") + .build(); + + TestThreadPool threadPool = new TestThreadPool("testThatOnlyNodesSupport"); + ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool); + + OperationRouting opRouting = new OperationRouting( + setting, + new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); + opRouting.setClusterService(clusterService); + + assertTrue(opRouting.ignoreAwarenessAttributes()); + ResponseCollectorService collector = new ResponseCollectorService(clusterService); + Map outstandingRequests = new HashMap<>(); + + Map weights = Map.of("a", "1", "b", "1", "c", "0"); + WRRWeights wrrWeight = new WRRWeights("zone", weights); + WeightedRoundRobinMetadata wrrMetadata = new WeightedRoundRobinMetadata(wrrWeight); + Metadata.Builder metadataBuilder = Metadata.builder(state.metadata()); + metadataBuilder.putCustom(WeightedRoundRobinMetadata.TYPE, wrrMetadata); + state = ClusterState.builder(state).metadata(metadataBuilder).build(); + + GroupShardsIterator groupIterator = opRouting.searchShards( + state, + indexNames, + null, + null, + collector, + outstandingRequests + ); + + assertEquals(6, opRouting.getWrrShardsCache().getCache().count()); + + // Calling operation routing without any cluster state change + groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); + + assertEquals(12, opRouting.getWrrShardsCache().getCache().stats().getHits()); + assertEquals(6, opRouting.getWrrShardsCache().getCache().count()); + + // Calling Operation Routing after a change in cluster metadata + weights = Map.of("a", "1", "b", "0", "c", "1"); + wrrWeight = new WRRWeights("zone", weights); + wrrMetadata = new WeightedRoundRobinMetadata(wrrWeight); + metadataBuilder = Metadata.builder(state.metadata()); + metadataBuilder.putCustom(WeightedRoundRobinMetadata.TYPE, wrrMetadata); + ClusterState state2 = ClusterState.builder(state).metadata(metadataBuilder).build(); + + ClusterChangedEvent event = new ClusterChangedEvent("test", state2, state); + + opRouting.getWrrShardsCache().clusterChanged(event); + + groupIterator = opRouting.searchShards(state2, indexNames, null, null, collector, outstandingRequests); + + assertEquals(12, opRouting.getWrrShardsCache().getCache().stats().getHits()); + assertEquals(12, opRouting.getWrrShardsCache().getCache().stats().getMisses()); + assertEquals(6, opRouting.getWrrShardsCache().getCache().count()); + + IOUtils.close(clusterService); + terminate(threadPool); + } + private DiscoveryNode[] setupNodes() { // Sets up two data nodes in zone-a and one data node in zone-b List zones = Arrays.asList("a", "a", "b"); @@ -1032,8 +1128,8 @@ private ClusterState updateStatetoTestWRR( } // add wrr weights in metadata Map weights = Map.of("a", "1", "b", "1", "c", "0"); - WRRWeight wrrWeight = new WRRWeight("zone", weights); - WeightedRoundRobinMetadata wrrMetadata = new WeightedRoundRobinMetadata(wrrWeight); + WRRWeights wrrWeights = new WRRWeights("zone", weights); + WeightedRoundRobinMetadata wrrMetadata = new WeightedRoundRobinMetadata(wrrWeights); metadataBuilder.putCustom(WeightedRoundRobinMetadata.TYPE, wrrMetadata); clusterState.metadata(metadataBuilder); clusterState.routingTable(routingTableBuilder.build()); From 4cdbee303db934d7afa202dd1b796731291fdcbc Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Sat, 27 Aug 2022 12:29:13 +0530 Subject: [PATCH 03/31] Integrate ARS with weighted round robin, Remove Cluster Settings for WRR Add Code refactoring and Java docs Signed-off-by: Anshu Agarwal --- .../metadata/WeightedRoundRobinMetadata.java | 4 +- .../routing/IndexShardRoutingTable.java | 50 ++++++++++++----- .../cluster/routing/OperationRouting.java | 17 +----- .../cluster/routing/WRRShardsCache.java | 19 +++++-- .../cluster/routing/WeightedRoundRobin.java | 38 ++----------- .../common/settings/ClusterSettings.java | 1 - .../routing/OperationRoutingTests.java | 53 +++++++++++++------ .../routing/WeightedRoundRobinTests.java | 37 +++++++++++++ 8 files changed, 137 insertions(+), 82 deletions(-) create mode 100644 server/src/test/java/org/opensearch/cluster/routing/WeightedRoundRobinTests.java diff --git a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinMetadata.java index ed89f885bdb4a..34a5a15114ce7 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinMetadata.java @@ -66,8 +66,8 @@ public String getWriteableName() { @Override public Version getMinimalSupportedVersion() { - // TODO: Check if this needs to be changed - return Version.CURRENT.minimumCompatibilityVersion(); + return Version.V_2_3_0; + } @Override diff --git a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java index 56abf80af1cdc..16d8dbddf75fd 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java +++ b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java @@ -43,6 +43,7 @@ import org.opensearch.index.Index; import org.opensearch.index.shard.ShardId; import org.opensearch.node.ResponseCollectorService; + import java.io.IOException; import java.util.ArrayList; import java.util.Collections; @@ -292,16 +293,24 @@ public ShardIterator activeInitializingShardsRankedIt( } /** - * * + * Returns an iterator over active and initializing shards, shards are ordered by weighted round-robin scheduling + * policy with adaptive replica selection. The output from weighted round-robin is ordered using adaptive replica + * selection to select eligible nodes for better performance. * @param wrrWeight Weighted round-robin weight entity * @param nodes discovered nodes in the cluster - * @return an interator over active and initializing shards, ordered by weighted round-robin + * @return an iterator over active and initializing shards, ordered by weighted round-robin * scheduling policy. Making sure that initializing shards are the last to iterate through. */ - public ShardIterator activeInitializingShardsWRR(WRRWeights wrrWeight, DiscoveryNodes nodes, WRRShardsCache cache) { + public ShardIterator activeInitializingShardsWRR( + WRRWeights wrrWeight, + DiscoveryNodes nodes, + WRRShardsCache cache, + @Nullable ResponseCollectorService collector, + @Nullable Map nodeSearchCounts + ) { final int seed = shuffler.nextSeed(); - ArrayList ordered = new ArrayList<>(activeShards.size() + allInitializingShards.size()); - ArrayList orderedActiveShards; + List ordered = new ArrayList<>(activeShards.size() + allInitializingShards.size()); + List orderedActiveShards; if (cache.getCache().get(new WRRShardsCache.Key(shardId)) != null) { orderedActiveShards = cache.getCache().get(new WRRShardsCache.Key(shardId)); } else { @@ -309,9 +318,24 @@ public ShardIterator activeInitializingShardsWRR(WRRWeights wrrWeight, Discovery cache.getCache().put(new WRRShardsCache.Key(shardId), orderedActiveShards); } - ordered.addAll(shuffler.shuffle(orderedActiveShards, seed)); + // In case the shardRouting list returned by weighted round-robin is empty, we fail open and consider all + // activeShards + orderedActiveShards = orderedActiveShards == null || orderedActiveShards.isEmpty() ? activeShards : orderedActiveShards; + + // output from weighted round-robin is ordered using adaptive replica selection + orderedActiveShards = rankShardsAndUpdateStats(shuffler.shuffle(orderedActiveShards, seed), collector, nodeSearchCounts); + + ordered.addAll(orderedActiveShards); + if (!allInitializingShards.isEmpty()) { - ArrayList orderedInitializingShards = getShardsWRR(allInitializingShards, wrrWeight, nodes); + List orderedInitializingShards = getShardsWRR(allInitializingShards, wrrWeight, nodes); + // In case the shardRouting list returned by weighted round-robin is empty, we fail open and consider all + // initializing shards + orderedInitializingShards = orderedInitializingShards == null || orderedInitializingShards.isEmpty() + ? allInitializingShards + : orderedInitializingShards; + // output from weighted round-robin is ordered using adaptive replica selection + orderedInitializingShards = rankShardsAndUpdateStats(orderedInitializingShards, collector, nodeSearchCounts); ordered.addAll(orderedInitializingShards); } return new PlainShardIterator(shardId, ordered); @@ -324,11 +348,11 @@ public ShardIterator activeInitializingShardsWRR(WRRWeights wrrWeight, Discovery * @param nodes discovered nodes in the cluster * @return list of shards ordered using weighted round-robin scheduling. */ - private ArrayList getShardsWRR(List shards, WRRWeights wrrWeight, DiscoveryNodes nodes) { + private List getShardsWRR(List shards, WRRWeights wrrWeight, DiscoveryNodes nodes) { List> weightedShards = calculateShardWeight(shards, wrrWeight, nodes); WeightedRoundRobin wrr = new WeightedRoundRobin<>(weightedShards); List> wrrOrderedActiveShards = wrr.orderEntities(); - ArrayList orderedActiveShards = new ArrayList<>(activeShards.size()); + List orderedActiveShards = new ArrayList<>(activeShards.size()); for (WeightedRoundRobin.Entity shardRouting : wrrOrderedActiveShards) { orderedActiveShards.add(shardRouting.getTarget()); } @@ -336,10 +360,10 @@ private ArrayList getShardsWRR(List shards, WRRWeigh } /** - * * + * * @param shards associate weights to shards * @param wrrWeight weights to be used for association - * @param nodes + * @param nodes discovered nodes in the cluster * @return list of entity containing shard routing and associated weight. */ private List> calculateShardWeight( @@ -352,7 +376,9 @@ private List> calculateShardWeight( shard.currentNodeId(); DiscoveryNode node = nodes.get(shard.currentNodeId()); String attVal = node.getAttributes().get(wrrWeight.attributeName()); - weightedShards.add(new WeightedRoundRobin.Entity<>(Double.parseDouble(wrrWeight.weights().get(attVal).toString()), shard)); + // If weight for a zone is not defined, considering it as 1 by default + Double weight = Double.parseDouble(wrrWeight.weights().getOrDefault(attVal, 1).toString()); + weightedShards.add(new WeightedRoundRobin.Entity<>(weight, shard)); } return weightedShards; } diff --git a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java index d33b42bc69620..c48450f72a244 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java +++ b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java @@ -70,12 +70,6 @@ public class OperationRouting { Setting.Property.NodeScope ); - public static final Setting USE_WEIGHTED_ROUND_ROBIN = Setting.boolSetting( - "cluster.routing.use_weighted_round_robin", - true, - Setting.Property.Dynamic, - Setting.Property.NodeScope - ); public static final String IGNORE_AWARENESS_ATTRIBUTES = "cluster.search.ignore_awareness_attributes"; public static final Setting IGNORE_AWARENESS_ATTRIBUTES_SETTING = Setting.boolSetting( IGNORE_AWARENESS_ATTRIBUTES, @@ -123,10 +117,8 @@ public OperationRouting(Settings settings, ClusterSettings clusterSettings) { this::setAwarenessAttributes ); this.useAdaptiveReplicaSelection = USE_ADAPTIVE_REPLICA_SELECTION_SETTING.get(settings); - this.useWeightedRoundRobin = USE_WEIGHTED_ROUND_ROBIN.get(settings); clusterSettings.addSettingsUpdateConsumer(USE_ADAPTIVE_REPLICA_SELECTION_SETTING, this::setUseAdaptiveReplicaSelection); clusterSettings.addSettingsUpdateConsumer(IGNORE_AWARENESS_ATTRIBUTES_SETTING, this::setIgnoreAwarenessAttributes); - clusterSettings.addSettingsUpdateConsumer(USE_WEIGHTED_ROUND_ROBIN, this::setUseWeightedRoundRobin); } @@ -138,10 +130,6 @@ void setIgnoreAwarenessAttributes(boolean ignoreAwarenessAttributes) { this.ignoreAwarenessAttr = ignoreAwarenessAttributes; } - public void setUseWeightedRoundRobin(boolean useWeightedRoundRobin) { - this.useWeightedRoundRobin = useWeightedRoundRobin; - } - public boolean isIgnoreAwarenessAttr() { return ignoreAwarenessAttr; } @@ -234,12 +222,11 @@ public GroupShardsIterator searchShards( private void setWeightedRoundRobinAttributes(ClusterState clusterState, ClusterService clusterService) { WeightedRoundRobinMetadata weightedRoundRobinMetadata = clusterState.metadata().custom(WeightedRoundRobinMetadata.TYPE); - this.isWeightedRoundRobinEnabled = useWeightedRoundRobin && weightedRoundRobinMetadata != null ? true : false; + this.isWeightedRoundRobinEnabled = weightedRoundRobinMetadata != null ? true : false; if (this.isWeightedRoundRobinEnabled) { this.wrrWeights = weightedRoundRobinMetadata.getWrrWeight(); this.wrrShardsCache = getWrrShardsCache() != null ? getWrrShardsCache() : new WRRShardsCache(clusterService); } - } private boolean isWeightedRoundRobinEnabled() { @@ -364,7 +351,7 @@ private ShardIterator shardRoutings( @Nullable Map nodeCounts ) { if (isWeightedRoundRobinEnabled()) { - return indexShard.activeInitializingShardsWRR(getWrrWeights(), nodes, wrrShardsCache); + return indexShard.activeInitializingShardsWRR(getWrrWeights(), nodes, wrrShardsCache, collectorService, nodeCounts); } else if (ignoreAwarenessAttributes()) { if (useAdaptiveReplicaSelection) { return indexShard.activeInitializingShardsRankedIt(collectorService, nodeCounts); diff --git a/server/src/main/java/org/opensearch/cluster/routing/WRRShardsCache.java b/server/src/main/java/org/opensearch/cluster/routing/WRRShardsCache.java index 966f185a29630..879f73a304c76 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WRRShardsCache.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WRRShardsCache.java @@ -18,17 +18,23 @@ import org.opensearch.common.lease.Releasable; import org.opensearch.index.shard.ShardId; -import java.util.ArrayList; +import java.util.List; +/** + * The WRR shards cache allows caching shard ordering returned by Weighted round-robin scheduling policy ,helping with + * improving similar requests. + * + * @opensearch.internal + */ public class WRRShardsCache implements Releasable, ClusterStateListener { private static final Logger logger = LogManager.getLogger(WRRShardsCache.class); - private final Cache> cache; + private final Cache> cache; public WRRShardsCache(ClusterService clusterService) { final long sizeInBytes = 2000000; - CacheBuilder> cacheBuilder = CacheBuilder.>builder() + CacheBuilder> cacheBuilder = CacheBuilder.>builder() .removalListener(notification -> logger.info("Object" + " {} removed from cache", notification.getKey().shardId)) .setMaximumWeight(sizeInBytes); cache = cacheBuilder.build(); @@ -41,10 +47,15 @@ public void close() { cache.invalidateAll(); } - public Cache> getCache() { + public Cache> getCache() { return cache; } + /** + * Listens to cluster state change event and invalidate cache on such events + * + * @param event cluster state change event + */ @Override public void clusterChanged(ClusterChangedEvent event) { logger.info("Invalidating WRRShardsCache on ClusterChangedEvent"); diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java index ad9702544a1e4..bd87789bc90b9 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java @@ -9,45 +9,15 @@ package org.opensearch.cluster.routing; import java.util.ArrayList; -import java.util.Iterator; import java.util.List; -public class WeightedRoundRobin implements Iterator, Iterable { +public class WeightedRoundRobin { private List> entities; private int turn; - private int lastSelectedEntity; - private double currentWeight = 0; public WeightedRoundRobin(List> entities) { this.entities = entities; - this.turn = 0; - this.lastSelectedEntity = -1; - } - - /* (non-Javadoc) - * @see java.util.Iterator#hasNext() - */ - @Override - public boolean hasNext() { - return entities.size() > 0; - } - - /* (non-Javadoc) - * @see java.util.Iterator#next() - */ - @Override - public T next() { - Entity entity = entities.get(turn++); - return entity.getTarget(); - } - - /* (non-Javadoc) - * @see java.lang.Iterable#iterator() - */ - @Override - public Iterator iterator() { - return this; } /** @@ -56,7 +26,9 @@ public Iterator iterator() { * http://kb.linuxvirtualserver.org/wiki/Weighted_Round-Robin_Scheduling */ public List> orderEntities() { + int lastSelectedEntity = -1; int size = entities.size(); + double currentWeight = 0; List> orderedWeight = new ArrayList<>(); if (size == 0) { return null; @@ -97,8 +69,8 @@ public List> orderEntities() { * Return greatest common divisor for two integers * https://en.wikipedia.org/wiki/Greatest_common_divisor#Using_Euclid.27s_algorithm * - * @param a - * @param b + * @param a first number + * @param b second number * @return greatest common divisor */ private double gcd(double a, double b) { diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index bc06987624918..971fb518ff1da 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -529,7 +529,6 @@ public void apply(Settings value, Settings current, Settings previous) { Node.BREAKER_TYPE_KEY, OperationRouting.USE_ADAPTIVE_REPLICA_SELECTION_SETTING, OperationRouting.IGNORE_AWARENESS_ATTRIBUTES_SETTING, - OperationRouting.USE_WEIGHTED_ROUND_ROBIN, IndexGraveyard.SETTING_MAX_TOMBSTONES, PersistentTasksClusterService.CLUSTER_TASKS_ALLOCATION_RECHECK_INTERVAL_SETTING, EnableAssignmentDecider.CLUSTER_TASKS_ALLOCATION_ENABLE_SETTING, diff --git a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java index e5c07f1b3daf1..071f7510abe14 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java @@ -766,10 +766,13 @@ public void testWRR() throws Exception { final int numIndices = 2; final int numShards = 3; final int numReplicas = 2; + // setting up indices final String[] indexNames = new String[numIndices]; for (int i = 0; i < numIndices; i++) { indexNames[i] = "test" + i; } + + // setting up domain nodes and attributes DiscoveryNode[] allNodes = setUpNodesWRR(); ClusterState state = ClusterStateCreationUtils.state(allNodes[0], allNodes[6], allNodes); @@ -789,12 +792,10 @@ public void testWRR() throws Exception { discoveryNodeMap.put("b", nodesZoneB); discoveryNodeMap.put("c", nodesZoneC); + // Updating cluster state with node, index and shard details state = updateStatetoTestWRR(indexNames, numShards, numReplicas, state, discoveryNodeMap); - Settings setting = Settings.builder() - .put("cluster.routing.allocation.awareness.attributes", "zone") - .put("cluster.routing.use_weighted_round_robin", "true") - .build(); + Settings setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); TestThreadPool threadPool = new TestThreadPool("testThatOnlyNodesSupport"); ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool); @@ -809,6 +810,7 @@ public void testWRR() throws Exception { ResponseCollectorService collector = new ResponseCollectorService(clusterService); Map outstandingRequests = new HashMap<>(); + // Setting up weights for weighted round-robin in cluster state Map weights = Map.of("a", "1", "b", "1", "c", "0"); WRRWeights wrrWeight = new WRRWeights("zone", weights); WeightedRoundRobinMetadata wrrMetadata = new WeightedRoundRobinMetadata(wrrWeight); @@ -816,6 +818,7 @@ public void testWRR() throws Exception { metadataBuilder.putCustom(WeightedRoundRobinMetadata.TYPE, wrrMetadata); state = ClusterState.builder(state).metadata(metadataBuilder).build(); + // search shards call GroupShardsIterator groupIterator = opRouting.searchShards( state, indexNames, @@ -832,27 +835,29 @@ public void testWRR() throws Exception { selectedNodes.add(shardRouting.currentNodeId()); } } + // tests no nodes are assigned to nodes in zone c for (String nodeID : selectedNodes) { // No shards are assigned to nodes in zone c since its weight is 0 assertFalse(nodeID.contains("c")); } selectedNodes = new HashSet<>(); - setting = Settings.builder() - .put("cluster.routing.allocation.awareness.attributes", "zone") - .put("cluster.routing.use_weighted_round_robin", "true") - .build(); + setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); + // Updating weighted round robin weights in cluster state weights = Map.of("a", "1", "b", "0", "c", "1"); wrrWeight = new WRRWeights("zone", weights); wrrMetadata = new WeightedRoundRobinMetadata(wrrWeight); metadataBuilder = Metadata.builder(state.metadata()); metadataBuilder.putCustom(WeightedRoundRobinMetadata.TYPE, wrrMetadata); + + // building cluster state with new weights state = ClusterState.builder(state).metadata(metadataBuilder).build(); opRouting = new OperationRouting(setting, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)); opRouting.setClusterService(clusterService); + // search shards call groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); for (ShardIterator it : groupIterator) { @@ -861,6 +866,7 @@ public void testWRR() throws Exception { selectedNodes.add(shardRouting.currentNodeId()); } } + // tests that no shards are assigned to zone with weight zero for (String nodeID : selectedNodes) { // No shards are assigned to nodes in zone b since its weight is 0 assertFalse(nodeID.contains("b")); @@ -873,10 +879,13 @@ public void testWRRShardsCaching() throws Exception { final int numIndices = 2; final int numShards = 3; final int numReplicas = 2; + // setting up indices final String[] indexNames = new String[numIndices]; for (int i = 0; i < numIndices; i++) { indexNames[i] = "test" + i; } + + // setting up domain nodes and attributes DiscoveryNode[] allNodes = setUpNodesWRR(); ClusterState state = ClusterStateCreationUtils.state(allNodes[0], allNodes[6], allNodes); @@ -896,12 +905,10 @@ public void testWRRShardsCaching() throws Exception { discoveryNodeMap.put("b", nodesZoneB); discoveryNodeMap.put("c", nodesZoneC); + // Updating cluster state with node, index and shard details state = updateStatetoTestWRR(indexNames, numShards, numReplicas, state, discoveryNodeMap); - Settings setting = Settings.builder() - .put("cluster.routing.allocation.awareness.attributes", "zone") - .put("cluster.routing.use_weighted_round_robin", "true") - .build(); + Settings setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); TestThreadPool threadPool = new TestThreadPool("testThatOnlyNodesSupport"); ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool); @@ -916,6 +923,7 @@ public void testWRRShardsCaching() throws Exception { ResponseCollectorService collector = new ResponseCollectorService(clusterService); Map outstandingRequests = new HashMap<>(); + // Setting up weights for weighted round-robin in cluster state Map weights = Map.of("a", "1", "b", "1", "c", "0"); WRRWeights wrrWeight = new WRRWeights("zone", weights); WeightedRoundRobinMetadata wrrMetadata = new WeightedRoundRobinMetadata(wrrWeight); @@ -923,6 +931,7 @@ public void testWRRShardsCaching() throws Exception { metadataBuilder.putCustom(WeightedRoundRobinMetadata.TYPE, wrrMetadata); state = ClusterState.builder(state).metadata(metadataBuilder).build(); + // search shards call GroupShardsIterator groupIterator = opRouting.searchShards( state, indexNames, @@ -932,30 +941,44 @@ public void testWRRShardsCaching() throws Exception { outstandingRequests ); + // shard wrr ordering details are not present in cache, the details are calculated and put in cache + assertEquals(6, opRouting.getWrrShardsCache().getCache().stats().getMisses()); assertEquals(6, opRouting.getWrrShardsCache().getCache().count()); - // Calling operation routing without any cluster state change + // Calling operation routing again without any cluster state change to test that wrr shard routing details are + // fetched from cache groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); + // details are fetched from cache assertEquals(12, opRouting.getWrrShardsCache().getCache().stats().getHits()); + // cache count stays same assertEquals(6, opRouting.getWrrShardsCache().getCache().count()); + // cache misses stay same + assertEquals(6, opRouting.getWrrShardsCache().getCache().stats().getMisses()); - // Calling Operation Routing after a change in cluster metadata + // Updating cluster state to test wrr shard routing details are calculated again weights = Map.of("a", "1", "b", "0", "c", "1"); wrrWeight = new WRRWeights("zone", weights); wrrMetadata = new WeightedRoundRobinMetadata(wrrWeight); metadataBuilder = Metadata.builder(state.metadata()); metadataBuilder.putCustom(WeightedRoundRobinMetadata.TYPE, wrrMetadata); + // building new cluster state ClusterState state2 = ClusterState.builder(state).metadata(metadataBuilder).build(); ClusterChangedEvent event = new ClusterChangedEvent("test", state2, state); - opRouting.getWrrShardsCache().clusterChanged(event); + // cache is invalidated after cluster state change, cache count is zero + assertEquals(0, opRouting.getWrrShardsCache().getCache().count()); + + // search shards call groupIterator = opRouting.searchShards(state2, indexNames, null, null, collector, outstandingRequests); + // cache hit remain same assertEquals(12, opRouting.getWrrShardsCache().getCache().stats().getHits()); + // cache miss increases by 6 assertEquals(12, opRouting.getWrrShardsCache().getCache().stats().getMisses()); + assertEquals(6, opRouting.getWrrShardsCache().getCache().count()); IOUtils.close(clusterService); diff --git a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoundRobinTests.java b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoundRobinTests.java new file mode 100644 index 0000000000000..398a36fba83e5 --- /dev/null +++ b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoundRobinTests.java @@ -0,0 +1,37 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.routing; + +import org.opensearch.test.OpenSearchTestCase; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +public class WeightedRoundRobinTests extends OpenSearchTestCase { + + public void testWRROrder() { + + List> entity = new ArrayList>(); + entity.add(new WeightedRoundRobin.Entity<>(4, "A")); + entity.add(new WeightedRoundRobin.Entity<>(3, "B")); + entity.add(new WeightedRoundRobin.Entity<>(2, "C")); + WeightedRoundRobin wrr = new WeightedRoundRobin(entity); + List> orderedEntities = wrr.orderEntities(); + + List expectedOrdering = Arrays.asList("A", "A", "B", "A", "B", "C", "A", "B", "C"); + List actualOrdering = new ArrayList<>(); + for (WeightedRoundRobin.Entity en : orderedEntities) { + actualOrdering.add(en.getTarget()); + } + + assertEquals(expectedOrdering, actualOrdering); + } + +} From 7a49e5ceb86d61fdf8de36914856848a3fdfd9ba Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Thu, 1 Sep 2022 13:02:14 +0530 Subject: [PATCH 04/31] Remove ARS and add tests for zone with undefined weight Signed-off-by: Anshu Agarwal --- .../org/opensearch/cluster/ClusterModule.java | 14 ++ ...=> WeightedRoundRobinRoutingMetadata.java} | 16 +- .../routing/IndexShardRoutingTable.java | 34 +--- .../cluster/routing/OperationRouting.java | 11 +- .../cluster/routing/WeightedRoundRobin.java | 1 - .../routing/OperationRoutingTests.java | 169 +++++++++++++----- 6 files changed, 160 insertions(+), 85 deletions(-) rename server/src/main/java/org/opensearch/cluster/metadata/{WeightedRoundRobinMetadata.java => WeightedRoundRobinRoutingMetadata.java} (88%) diff --git a/server/src/main/java/org/opensearch/cluster/ClusterModule.java b/server/src/main/java/org/opensearch/cluster/ClusterModule.java index f8ba520e465e2..99a342eaeb5ed 100644 --- a/server/src/main/java/org/opensearch/cluster/ClusterModule.java +++ b/server/src/main/java/org/opensearch/cluster/ClusterModule.java @@ -48,6 +48,7 @@ import org.opensearch.cluster.metadata.MetadataMappingService; import org.opensearch.cluster.metadata.MetadataUpdateSettingsService; import org.opensearch.cluster.metadata.RepositoriesMetadata; +import org.opensearch.cluster.metadata.WeightedRoundRobinRoutingMetadata; import org.opensearch.cluster.routing.DelayedAllocationService; import org.opensearch.cluster.routing.allocation.AllocationService; import org.opensearch.cluster.routing.allocation.ExistingShardsAllocator; @@ -191,6 +192,12 @@ public static List getNamedWriteables() { ComposableIndexTemplateMetadata::readDiffFrom ); registerMetadataCustom(entries, DataStreamMetadata.TYPE, DataStreamMetadata::new, DataStreamMetadata::readDiffFrom); + registerMetadataCustom( + entries, + WeightedRoundRobinRoutingMetadata.TYPE, + WeightedRoundRobinRoutingMetadata::new, + WeightedRoundRobinRoutingMetadata::readDiffFrom + ); // Task Status (not Diffable) entries.add(new Entry(Task.Status.class, PersistentTasksNodeService.Status.NAME, PersistentTasksNodeService.Status::new)); return entries; @@ -274,6 +281,13 @@ public static List getNamedXWriteables() { DataStreamMetadata::fromXContent ) ); + entries.add( + new NamedXContentRegistry.Entry( + Metadata.Custom.class, + new ParseField(WeightedRoundRobinRoutingMetadata.TYPE), + WeightedRoundRobinRoutingMetadata::fromXContent + ) + ); return entries; } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadata.java similarity index 88% rename from server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinMetadata.java rename to server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadata.java index 34a5a15114ce7..a05e3e8aaec4d 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadata.java @@ -32,8 +32,8 @@ * * @opensearch.internal */ -public class WeightedRoundRobinMetadata extends AbstractNamedDiffable implements Metadata.Custom { - private static final Logger logger = LogManager.getLogger(WeightedRoundRobinMetadata.class); +public class WeightedRoundRobinRoutingMetadata extends AbstractNamedDiffable implements Metadata.Custom { + private static final Logger logger = LogManager.getLogger(WeightedRoundRobinRoutingMetadata.class); public static final String TYPE = "wrr_shard_routing"; private WRRWeights wrrWeight; @@ -41,16 +41,16 @@ public WRRWeights getWrrWeight() { return wrrWeight; } - public WeightedRoundRobinMetadata setWrrWeight(WRRWeights wrrWeight) { + public WeightedRoundRobinRoutingMetadata setWrrWeight(WRRWeights wrrWeight) { this.wrrWeight = wrrWeight; return this; } - public WeightedRoundRobinMetadata(StreamInput in) throws IOException { + public WeightedRoundRobinRoutingMetadata(StreamInput in) throws IOException { this.wrrWeight = new WRRWeights(in); } - public WeightedRoundRobinMetadata(WRRWeights wrrWeight) { + public WeightedRoundRobinRoutingMetadata(WRRWeights wrrWeight) { this.wrrWeight = wrrWeight; } @@ -79,7 +79,7 @@ public static NamedDiff readDiffFrom(StreamInput in) throws IOE return readDiffFrom(Metadata.Custom.class, TYPE, in); } - public static WeightedRoundRobinMetadata fromXContent(XContentParser parser) throws IOException { + public static WeightedRoundRobinRoutingMetadata fromXContent(XContentParser parser) throws IOException { String attrKey = null; Object attrValue; String attributeName = null; @@ -117,14 +117,14 @@ public static WeightedRoundRobinMetadata fromXContent(XContentParser parser) thr } } wrrWeight = new WRRWeights(attributeName, weights); - return new WeightedRoundRobinMetadata(wrrWeight); + return new WeightedRoundRobinRoutingMetadata(wrrWeight); } @Override public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - WeightedRoundRobinMetadata that = (WeightedRoundRobinMetadata) o; + WeightedRoundRobinRoutingMetadata that = (WeightedRoundRobinRoutingMetadata) o; return wrrWeight.equals(that.wrrWeight); } diff --git a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java index 16d8dbddf75fd..a8ad49bd968e3 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java +++ b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java @@ -294,20 +294,13 @@ public ShardIterator activeInitializingShardsRankedIt( /** * Returns an iterator over active and initializing shards, shards are ordered by weighted round-robin scheduling - * policy with adaptive replica selection. The output from weighted round-robin is ordered using adaptive replica - * selection to select eligible nodes for better performance. + * policy. * @param wrrWeight Weighted round-robin weight entity * @param nodes discovered nodes in the cluster * @return an iterator over active and initializing shards, ordered by weighted round-robin * scheduling policy. Making sure that initializing shards are the last to iterate through. */ - public ShardIterator activeInitializingShardsWRR( - WRRWeights wrrWeight, - DiscoveryNodes nodes, - WRRShardsCache cache, - @Nullable ResponseCollectorService collector, - @Nullable Map nodeSearchCounts - ) { + public ShardIterator activeInitializingShardsWRR(WRRWeights wrrWeight, DiscoveryNodes nodes, WRRShardsCache cache) { final int seed = shuffler.nextSeed(); List ordered = new ArrayList<>(activeShards.size() + allInitializingShards.size()); List orderedActiveShards; @@ -317,25 +310,10 @@ public ShardIterator activeInitializingShardsWRR( orderedActiveShards = getShardsWRR(activeShards, wrrWeight, nodes); cache.getCache().put(new WRRShardsCache.Key(shardId), orderedActiveShards); } - - // In case the shardRouting list returned by weighted round-robin is empty, we fail open and consider all - // activeShards - orderedActiveShards = orderedActiveShards == null || orderedActiveShards.isEmpty() ? activeShards : orderedActiveShards; - - // output from weighted round-robin is ordered using adaptive replica selection - orderedActiveShards = rankShardsAndUpdateStats(shuffler.shuffle(orderedActiveShards, seed), collector, nodeSearchCounts); - ordered.addAll(orderedActiveShards); if (!allInitializingShards.isEmpty()) { List orderedInitializingShards = getShardsWRR(allInitializingShards, wrrWeight, nodes); - // In case the shardRouting list returned by weighted round-robin is empty, we fail open and consider all - // initializing shards - orderedInitializingShards = orderedInitializingShards == null || orderedInitializingShards.isEmpty() - ? allInitializingShards - : orderedInitializingShards; - // output from weighted round-robin is ordered using adaptive replica selection - orderedInitializingShards = rankShardsAndUpdateStats(orderedInitializingShards, collector, nodeSearchCounts); ordered.addAll(orderedInitializingShards); } return new PlainShardIterator(shardId, ordered); @@ -373,11 +351,13 @@ private List> calculateShardWeight( ) { List> weightedShards = new ArrayList<>(); for (ShardRouting shard : shards) { - shard.currentNodeId(); DiscoveryNode node = nodes.get(shard.currentNodeId()); String attVal = node.getAttributes().get(wrrWeight.attributeName()); - // If weight for a zone is not defined, considering it as 1 by default - Double weight = Double.parseDouble(wrrWeight.weights().getOrDefault(attVal, 1).toString()); + // If weight for a zone is not defined, not considering shards from that zone + if (wrrWeight.weights().get(attVal) == null) { + continue; + } + Double weight = Double.parseDouble(wrrWeight.weights().get(attVal).toString()); weightedShards.add(new WeightedRoundRobin.Entity<>(weight, shard)); } return weightedShards; diff --git a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java index c48450f72a244..9dd9f778fb660 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java +++ b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java @@ -34,7 +34,7 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.IndexMetadata; -import org.opensearch.cluster.metadata.WeightedRoundRobinMetadata; +import org.opensearch.cluster.metadata.WeightedRoundRobinRoutingMetadata; import org.opensearch.cluster.node.DiscoveryNodes; import org.opensearch.cluster.routing.allocation.decider.AwarenessAllocationDecider; import org.opensearch.cluster.service.ClusterService; @@ -221,10 +221,11 @@ public GroupShardsIterator searchShards( } private void setWeightedRoundRobinAttributes(ClusterState clusterState, ClusterService clusterService) { - WeightedRoundRobinMetadata weightedRoundRobinMetadata = clusterState.metadata().custom(WeightedRoundRobinMetadata.TYPE); - this.isWeightedRoundRobinEnabled = weightedRoundRobinMetadata != null ? true : false; + WeightedRoundRobinRoutingMetadata weightedRoundRobinRoutingMetadata = clusterState.metadata() + .custom(WeightedRoundRobinRoutingMetadata.TYPE); + this.isWeightedRoundRobinEnabled = weightedRoundRobinRoutingMetadata != null ? true : false; if (this.isWeightedRoundRobinEnabled) { - this.wrrWeights = weightedRoundRobinMetadata.getWrrWeight(); + this.wrrWeights = weightedRoundRobinRoutingMetadata.getWrrWeight(); this.wrrShardsCache = getWrrShardsCache() != null ? getWrrShardsCache() : new WRRShardsCache(clusterService); } } @@ -351,7 +352,7 @@ private ShardIterator shardRoutings( @Nullable Map nodeCounts ) { if (isWeightedRoundRobinEnabled()) { - return indexShard.activeInitializingShardsWRR(getWrrWeights(), nodes, wrrShardsCache, collectorService, nodeCounts); + return indexShard.activeInitializingShardsWRR(getWrrWeights(), nodes, wrrShardsCache); } else if (ignoreAwarenessAttributes()) { if (useAdaptiveReplicaSelection) { return indexShard.activeInitializingShardsRankedIt(collectorService, nodeCounts); diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java index bd87789bc90b9..e184bc4a5dc30 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java @@ -14,7 +14,6 @@ public class WeightedRoundRobin { private List> entities; - private int turn; public WeightedRoundRobin(List> entities) { this.entities = entities; diff --git a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java index 071f7510abe14..9ee13e1190ed2 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java @@ -37,7 +37,7 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.Metadata; -import org.opensearch.cluster.metadata.WeightedRoundRobinMetadata; +import org.opensearch.cluster.metadata.WeightedRoundRobinRoutingMetadata; import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.node.DiscoveryNodeRole; import org.opensearch.cluster.routing.allocation.decider.AwarenessAllocationDecider; @@ -762,17 +762,7 @@ public void testAdaptiveReplicaSelectionWithZoneAwarenessIgnored() throws Except terminate(threadPool); } - public void testWRR() throws Exception { - final int numIndices = 2; - final int numShards = 3; - final int numReplicas = 2; - // setting up indices - final String[] indexNames = new String[numIndices]; - for (int i = 0; i < numIndices; i++) { - indexNames[i] = "test" + i; - } - - // setting up domain nodes and attributes + private ClusterState clusterStateForWRR(String[] indexNames, int numShards, int numReplicas) { DiscoveryNode[] allNodes = setUpNodesWRR(); ClusterState state = ClusterStateCreationUtils.state(allNodes[0], allNodes[6], allNodes); @@ -795,6 +785,22 @@ public void testWRR() throws Exception { // Updating cluster state with node, index and shard details state = updateStatetoTestWRR(indexNames, numShards, numReplicas, state, discoveryNodeMap); + return state; + + } + + public void testWRR() throws Exception { + final int numIndices = 2; + final int numShards = 3; + final int numReplicas = 2; + // setting up indices + final String[] indexNames = new String[numIndices]; + for (int i = 0; i < numIndices; i++) { + indexNames[i] = "test" + i; + } + + ClusterState state = clusterStateForWRR(indexNames, numShards, numReplicas); + Settings setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); TestThreadPool threadPool = new TestThreadPool("testThatOnlyNodesSupport"); @@ -813,9 +819,9 @@ public void testWRR() throws Exception { // Setting up weights for weighted round-robin in cluster state Map weights = Map.of("a", "1", "b", "1", "c", "0"); WRRWeights wrrWeight = new WRRWeights("zone", weights); - WeightedRoundRobinMetadata wrrMetadata = new WeightedRoundRobinMetadata(wrrWeight); + WeightedRoundRobinRoutingMetadata wrrMetadata = new WeightedRoundRobinRoutingMetadata(wrrWeight); Metadata.Builder metadataBuilder = Metadata.builder(state.metadata()); - metadataBuilder.putCustom(WeightedRoundRobinMetadata.TYPE, wrrMetadata); + metadataBuilder.putCustom(WeightedRoundRobinRoutingMetadata.TYPE, wrrMetadata); state = ClusterState.builder(state).metadata(metadataBuilder).build(); // search shards call @@ -847,9 +853,9 @@ public void testWRR() throws Exception { // Updating weighted round robin weights in cluster state weights = Map.of("a", "1", "b", "0", "c", "1"); wrrWeight = new WRRWeights("zone", weights); - wrrMetadata = new WeightedRoundRobinMetadata(wrrWeight); + wrrMetadata = new WeightedRoundRobinRoutingMetadata(wrrWeight); metadataBuilder = Metadata.builder(state.metadata()); - metadataBuilder.putCustom(WeightedRoundRobinMetadata.TYPE, wrrMetadata); + metadataBuilder.putCustom(WeightedRoundRobinRoutingMetadata.TYPE, wrrMetadata); // building cluster state with new weights state = ClusterState.builder(state).metadata(metadataBuilder).build(); @@ -885,28 +891,7 @@ public void testWRRShardsCaching() throws Exception { indexNames[i] = "test" + i; } - // setting up domain nodes and attributes - DiscoveryNode[] allNodes = setUpNodesWRR(); - ClusterState state = ClusterStateCreationUtils.state(allNodes[0], allNodes[6], allNodes); - - Map> discoveryNodeMap = new HashMap<>(); - List nodesZoneA = new ArrayList<>(); - nodesZoneA.add(allNodes[0]); - nodesZoneA.add(allNodes[1]); - - List nodesZoneB = new ArrayList<>(); - nodesZoneB.add(allNodes[2]); - nodesZoneB.add(allNodes[3]); - - List nodesZoneC = new ArrayList<>(); - nodesZoneC.add(allNodes[4]); - nodesZoneC.add(allNodes[5]); - discoveryNodeMap.put("a", nodesZoneA); - discoveryNodeMap.put("b", nodesZoneB); - discoveryNodeMap.put("c", nodesZoneC); - - // Updating cluster state with node, index and shard details - state = updateStatetoTestWRR(indexNames, numShards, numReplicas, state, discoveryNodeMap); + ClusterState state = clusterStateForWRR(indexNames, numShards, numReplicas); Settings setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); @@ -926,9 +911,9 @@ public void testWRRShardsCaching() throws Exception { // Setting up weights for weighted round-robin in cluster state Map weights = Map.of("a", "1", "b", "1", "c", "0"); WRRWeights wrrWeight = new WRRWeights("zone", weights); - WeightedRoundRobinMetadata wrrMetadata = new WeightedRoundRobinMetadata(wrrWeight); + WeightedRoundRobinRoutingMetadata wrrMetadata = new WeightedRoundRobinRoutingMetadata(wrrWeight); Metadata.Builder metadataBuilder = Metadata.builder(state.metadata()); - metadataBuilder.putCustom(WeightedRoundRobinMetadata.TYPE, wrrMetadata); + metadataBuilder.putCustom(WeightedRoundRobinRoutingMetadata.TYPE, wrrMetadata); state = ClusterState.builder(state).metadata(metadataBuilder).build(); // search shards call @@ -959,9 +944,9 @@ public void testWRRShardsCaching() throws Exception { // Updating cluster state to test wrr shard routing details are calculated again weights = Map.of("a", "1", "b", "0", "c", "1"); wrrWeight = new WRRWeights("zone", weights); - wrrMetadata = new WeightedRoundRobinMetadata(wrrWeight); + wrrMetadata = new WeightedRoundRobinRoutingMetadata(wrrWeight); metadataBuilder = Metadata.builder(state.metadata()); - metadataBuilder.putCustom(WeightedRoundRobinMetadata.TYPE, wrrMetadata); + metadataBuilder.putCustom(WeightedRoundRobinRoutingMetadata.TYPE, wrrMetadata); // building new cluster state ClusterState state2 = ClusterState.builder(state).metadata(metadataBuilder).build(); @@ -985,6 +970,102 @@ public void testWRRShardsCaching() throws Exception { terminate(threadPool); } + public void testWRRWithWeightUndefinedForOneZone() throws Exception { + final int numIndices = 2; + final int numShards = 3; + final int numReplicas = 2; + // setting up indices + final String[] indexNames = new String[numIndices]; + for (int i = 0; i < numIndices; i++) { + indexNames[i] = "test" + i; + } + + ClusterState state = clusterStateForWRR(indexNames, numShards, numReplicas); + + Settings setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); + + TestThreadPool threadPool = new TestThreadPool("testThatOnlyNodesSupport"); + ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool); + + OperationRouting opRouting = new OperationRouting( + setting, + new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); + opRouting.setClusterService(clusterService); + assertTrue(opRouting.ignoreAwarenessAttributes()); + Set selectedNodes = new HashSet<>(); + ResponseCollectorService collector = new ResponseCollectorService(clusterService); + Map outstandingRequests = new HashMap<>(); + + // Setting up weights for weighted round-robin in cluster state, weight for nodes in zone b is not set + Map weights = Map.of("a", "1", "c", "0"); + WRRWeights wrrWeight = new WRRWeights("zone", weights); + WeightedRoundRobinRoutingMetadata wrrMetadata = new WeightedRoundRobinRoutingMetadata(wrrWeight); + Metadata.Builder metadataBuilder = Metadata.builder(state.metadata()); + metadataBuilder.putCustom(WeightedRoundRobinRoutingMetadata.TYPE, wrrMetadata); + state = ClusterState.builder(state).metadata(metadataBuilder).build(); + + // search shards call + GroupShardsIterator groupIterator = opRouting.searchShards( + state, + indexNames, + null, + null, + collector, + outstandingRequests + + ); + + for (ShardIterator it : groupIterator) { + List shardRoutings = Collections.singletonList(it.nextOrNull()); + for (ShardRouting shardRouting : shardRoutings) { + selectedNodes.add(shardRouting.currentNodeId()); + } + } + // tests no nodes are assigned to nodes in zone c + for (String nodeID : selectedNodes) { + // shard from nodes in zone c is not selected since its weight is 0 + assertFalse(nodeID.contains("c")); + // shard from nodes in zone b is not selected since its weight is not set + assertFalse(nodeID.contains("b")); + } + + selectedNodes = new HashSet<>(); + setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); + + // Updating weighted round robin weights in cluster state + weights = Map.of("a", "0", "b", "1"); + wrrWeight = new WRRWeights("zone", weights); + wrrMetadata = new WeightedRoundRobinRoutingMetadata(wrrWeight); + metadataBuilder = Metadata.builder(state.metadata()); + metadataBuilder.putCustom(WeightedRoundRobinRoutingMetadata.TYPE, wrrMetadata); + + // building cluster state with new weights + state = ClusterState.builder(state).metadata(metadataBuilder).build(); + + opRouting = new OperationRouting(setting, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)); + opRouting.setClusterService(clusterService); + + // search shards call + groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); + + for (ShardIterator it : groupIterator) { + List shardRoutings = Collections.singletonList(it.nextOrNull()); + for (ShardRouting shardRouting : shardRoutings) { + selectedNodes.add(shardRouting.currentNodeId()); + } + } + // tests that no shards are assigned to zone with weight zero + for (String nodeID : selectedNodes) { + // shard from nodes in zone a is not selected since its weight is 0 + assertFalse(nodeID.contains("a")); + // shard from nodes in zone c is not selected since its weight is not set + assertFalse(nodeID.contains("c")); + } + IOUtils.close(clusterService); + terminate(threadPool); + } + private DiscoveryNode[] setupNodes() { // Sets up two data nodes in zone-a and one data node in zone-b List zones = Arrays.asList("a", "a", "b"); @@ -1152,8 +1233,8 @@ private ClusterState updateStatetoTestWRR( // add wrr weights in metadata Map weights = Map.of("a", "1", "b", "1", "c", "0"); WRRWeights wrrWeights = new WRRWeights("zone", weights); - WeightedRoundRobinMetadata wrrMetadata = new WeightedRoundRobinMetadata(wrrWeights); - metadataBuilder.putCustom(WeightedRoundRobinMetadata.TYPE, wrrMetadata); + WeightedRoundRobinRoutingMetadata wrrMetadata = new WeightedRoundRobinRoutingMetadata(wrrWeights); + metadataBuilder.putCustom(WeightedRoundRobinRoutingMetadata.TYPE, wrrMetadata); clusterState.metadata(metadataBuilder); clusterState.routingTable(routingTableBuilder.build()); return clusterState.build(); From 03c4d2344372411a84752d52311eb80cc329e8d5 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Thu, 1 Sep 2022 13:07:26 +0530 Subject: [PATCH 05/31] Add changelog for the commit Signed-off-by: Anshu Agarwal --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8132c1281e412..f8ca0f9040ca1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Changed - Dependency updates (httpcore, mockito, slf4j, httpasyncclient, commons-codec) ([#4308](https://github.com/opensearch-project/OpenSearch/pull/4308)) + - Weighted round-robin scheduling policy for shard coordination traffic ([#4241](https://github.com/opensearch-project/OpenSearch/pull/4241)) + ### Deprecated From 154a3b94a85fcb180bd3966ccaa4e6ea6d1d5292 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Thu, 1 Sep 2022 14:09:42 +0530 Subject: [PATCH 06/31] Fix java doc, add test of WeightedRoundRobinRouting metadata Signed-off-by: Anshu Agarwal --- .../WeightedRoundRobinRoutingMetadata.java | 3 +- .../cluster/routing/WeightedRoundRobin.java | 5 ++- ...eightedRoundRobinRoutingMetadataTests.java | 37 +++++++++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 server/src/test/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadataTests.java diff --git a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadata.java index a05e3e8aaec4d..9ed11cb31c4d0 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadata.java @@ -35,6 +35,7 @@ public class WeightedRoundRobinRoutingMetadata extends AbstractNamedDiffable implements Metadata.Custom { private static final Logger logger = LogManager.getLogger(WeightedRoundRobinRoutingMetadata.class); public static final String TYPE = "wrr_shard_routing"; + public static final String AWARENESS = "awareness"; private WRRWeights wrrWeight; public WRRWeights getWrrWeight() { @@ -140,7 +141,7 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params par } public static void toXContent(WRRWeights wrrWeight, XContentBuilder builder) throws IOException { - builder.startObject("awareness"); + builder.startObject(AWARENESS); builder.startObject(wrrWeight.attributeName()); for (Map.Entry entry : wrrWeight.weights().entrySet()) { builder.field(entry.getKey(), entry.getValue()); diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java index e184bc4a5dc30..87efdc76d04bc 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java @@ -10,7 +10,10 @@ import java.util.ArrayList; import java.util.List; - +/** + * Weighted Round Robin Scheduling policy + * + */ public class WeightedRoundRobin { private List> entities; diff --git a/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadataTests.java b/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadataTests.java new file mode 100644 index 0000000000000..a5f3b8a1ccefa --- /dev/null +++ b/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadataTests.java @@ -0,0 +1,37 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.metadata; + +import org.opensearch.cluster.routing.WRRWeights; +import org.opensearch.common.xcontent.XContentParser; +import org.opensearch.test.AbstractXContentTestCase; + +import java.io.IOException; +import java.util.Map; + +public class WeightedRoundRobinRoutingMetadataTests extends AbstractXContentTestCase { + @Override + protected WeightedRoundRobinRoutingMetadata createTestInstance() { + Map weights = Map.of("a", "1", "b", "1", "c", "0"); + WRRWeights wrrWeights = new WRRWeights("zone", weights); + WeightedRoundRobinRoutingMetadata wrrMetadata = new WeightedRoundRobinRoutingMetadata(wrrWeights); + return wrrMetadata; + } + + @Override + protected WeightedRoundRobinRoutingMetadata doParseInstance(XContentParser parser) throws IOException { + return WeightedRoundRobinRoutingMetadata.fromXContent(parser); + } + + @Override + protected boolean supportsUnknownFields() { + return false; + } + +} From 3afb4e341e6a40bc534f90b63124592a75c50cf9 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Thu, 1 Sep 2022 15:35:30 +0530 Subject: [PATCH 07/31] Fix minor change related to shuffling wrr shard routings Signed-off-by: Anshu Agarwal --- .../org/opensearch/cluster/routing/IndexShardRoutingTable.java | 2 +- .../java/org/opensearch/cluster/routing/OperationRouting.java | 2 +- .../java/org/opensearch/cluster/routing/WeightedRoundRobin.java | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java index a8ad49bd968e3..275afb6a31586 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java +++ b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java @@ -310,7 +310,7 @@ public ShardIterator activeInitializingShardsWRR(WRRWeights wrrWeight, Discovery orderedActiveShards = getShardsWRR(activeShards, wrrWeight, nodes); cache.getCache().put(new WRRShardsCache.Key(shardId), orderedActiveShards); } - ordered.addAll(orderedActiveShards); + ordered.addAll(shuffler.shuffle(orderedActiveShards, seed)); if (!allInitializingShards.isEmpty()) { List orderedInitializingShards = getShardsWRR(allInitializingShards, wrrWeight, nodes); diff --git a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java index 9dd9f778fb660..4550a3a4b688d 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java +++ b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java @@ -223,7 +223,7 @@ public GroupShardsIterator searchShards( private void setWeightedRoundRobinAttributes(ClusterState clusterState, ClusterService clusterService) { WeightedRoundRobinRoutingMetadata weightedRoundRobinRoutingMetadata = clusterState.metadata() .custom(WeightedRoundRobinRoutingMetadata.TYPE); - this.isWeightedRoundRobinEnabled = weightedRoundRobinRoutingMetadata != null ? true : false; + this.isWeightedRoundRobinEnabled = weightedRoundRobinRoutingMetadata != null; if (this.isWeightedRoundRobinEnabled) { this.wrrWeights = weightedRoundRobinRoutingMetadata.getWrrWeight(); this.wrrShardsCache = getWrrShardsCache() != null ? getWrrShardsCache() : new WRRShardsCache(clusterService); diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java index 87efdc76d04bc..dd1966f1c6adb 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java @@ -10,6 +10,7 @@ import java.util.ArrayList; import java.util.List; + /** * Weighted Round Robin Scheduling policy * From 3aa6fbb522a17500074e707a8adc586047f4e498 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Wed, 7 Sep 2022 11:49:31 +0530 Subject: [PATCH 08/31] Add default weight 1 for zones with undefined weight Signed-off-by: Anshu Agarwal --- .../routing/IndexShardRoutingTable.java | 7 ++----- .../cluster/routing/OperationRoutingTests.java | 18 +++++++++++++----- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java index 275afb6a31586..b219a7178e7e0 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java +++ b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java @@ -353,11 +353,8 @@ private List> calculateShardWeight( for (ShardRouting shard : shards) { DiscoveryNode node = nodes.get(shard.currentNodeId()); String attVal = node.getAttributes().get(wrrWeight.attributeName()); - // If weight for a zone is not defined, not considering shards from that zone - if (wrrWeight.weights().get(attVal) == null) { - continue; - } - Double weight = Double.parseDouble(wrrWeight.weights().get(attVal).toString()); + // If weight for a zone is not defined, considering it as 1 by default + Double weight = Double.parseDouble(wrrWeight.weights().getOrDefault(attVal, 1).toString()); weightedShards.add(new WeightedRoundRobin.Entity<>(weight, shard)); } return weightedShards; diff --git a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java index 9ee13e1190ed2..3115070480aa9 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java @@ -1022,13 +1022,17 @@ public void testWRRWithWeightUndefinedForOneZone() throws Exception { selectedNodes.add(shardRouting.currentNodeId()); } } - // tests no nodes are assigned to nodes in zone c + boolean weighAwayNodesInUndefinedZone = true; + // tests no shards are assigned to nodes in zone c + // tests shards are assigned to nodes in zone b for (String nodeID : selectedNodes) { // shard from nodes in zone c is not selected since its weight is 0 assertFalse(nodeID.contains("c")); - // shard from nodes in zone b is not selected since its weight is not set - assertFalse(nodeID.contains("b")); + if (nodeID.contains("b")) { + weighAwayNodesInUndefinedZone = false; + } } + assertFalse(weighAwayNodesInUndefinedZone); selectedNodes = new HashSet<>(); setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); @@ -1056,12 +1060,16 @@ public void testWRRWithWeightUndefinedForOneZone() throws Exception { } } // tests that no shards are assigned to zone with weight zero + // tests shards are assigned to nodes in zone c + weighAwayNodesInUndefinedZone = true; for (String nodeID : selectedNodes) { // shard from nodes in zone a is not selected since its weight is 0 assertFalse(nodeID.contains("a")); - // shard from nodes in zone c is not selected since its weight is not set - assertFalse(nodeID.contains("c")); + if (nodeID.contains("c")) { + weighAwayNodesInUndefinedZone = false; + } } + assertFalse(weighAwayNodesInUndefinedZone); IOUtils.close(clusterService); terminate(threadPool); } From 111236657a53f27415308d178a9f8dc20096b579 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Mon, 12 Sep 2022 19:30:48 +0530 Subject: [PATCH 09/31] Inject WRRShardsCache on node start Signed-off-by: Anshu Agarwal --- .../WeightedRoundRobinRoutingMetadata.java | 11 ++- .../routing/IndexShardRoutingTable.java | 6 +- .../cluster/routing/OperationRouting.java | 64 +++++-------- .../cluster/routing/WRRShardsCache.java | 31 +++++-- .../cluster/service/ClusterService.java | 3 +- .../main/java/org/opensearch/node/Node.java | 6 ++ .../routing/OperationRoutingTests.java | 89 +++++++++---------- .../structure/RoutingIteratorTests.java | 9 +- 8 files changed, 116 insertions(+), 103 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadata.java index 9ed11cb31c4d0..b1baab52aaeb4 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadata.java @@ -48,7 +48,10 @@ public WeightedRoundRobinRoutingMetadata setWrrWeight(WRRWeights wrrWeight) { } public WeightedRoundRobinRoutingMetadata(StreamInput in) throws IOException { - this.wrrWeight = new WRRWeights(in); + if (in.available() != 0) { + this.wrrWeight = new WRRWeights(in); + + } } public WeightedRoundRobinRoutingMetadata(WRRWeights wrrWeight) { @@ -73,7 +76,9 @@ public Version getMinimalSupportedVersion() { @Override public void writeTo(StreamOutput out) throws IOException { - wrrWeight.writeTo(out); + if (wrrWeight != null) { + wrrWeight.writeTo(out); + } } public static NamedDiff readDiffFrom(StreamInput in) throws IOException { @@ -113,8 +118,6 @@ public static WeightedRoundRobinRoutingMetadata fromXContent(XContentParser pars } } } - } else { - throw new OpenSearchParseException("failed to parse wrr metadata attribute [{}]", attributeName); } } wrrWeight = new WRRWeights(attributeName, weights); diff --git a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java index b219a7178e7e0..d0e79f8efda2e 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java +++ b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java @@ -304,11 +304,11 @@ public ShardIterator activeInitializingShardsWRR(WRRWeights wrrWeight, Discovery final int seed = shuffler.nextSeed(); List ordered = new ArrayList<>(activeShards.size() + allInitializingShards.size()); List orderedActiveShards; - if (cache.getCache().get(new WRRShardsCache.Key(shardId)) != null) { - orderedActiveShards = cache.getCache().get(new WRRShardsCache.Key(shardId)); + if (cache.get(new WRRShardsCache.Key(shardId)) != null) { + orderedActiveShards = cache.get(new WRRShardsCache.Key(shardId)); } else { orderedActiveShards = getShardsWRR(activeShards, wrrWeight, nodes); - cache.getCache().put(new WRRShardsCache.Key(shardId), orderedActiveShards); + cache.put(new WRRShardsCache.Key(shardId), orderedActiveShards); } ordered.addAll(shuffler.shuffle(orderedActiveShards, seed)); diff --git a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java index 4550a3a4b688d..24b85d991bfda 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java +++ b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java @@ -81,32 +81,8 @@ public class OperationRouting { private volatile boolean useAdaptiveReplicaSelection; private volatile boolean ignoreAwarenessAttr; - // reads value from cluster setting - private volatile boolean useWeightedRoundRobin; - /** - * Reads value from cluster setting and cluster state to determine if weighted round-robin - * search routing is enabled - * This is true if useWeightedRoundRobin=true and weights are set in cluster metadata. - */ - private volatile boolean isWeightedRoundRobinEnabled; - - private volatile WRRWeights wrrWeights; - - public WRRShardsCache getWrrShardsCache() { - return wrrShardsCache; - } - - private WRRShardsCache wrrShardsCache; - - public ClusterService getClusterService() { - return clusterService; - } - - public void setClusterService(ClusterService clusterService) { - this.clusterService = clusterService; - } - - private ClusterService clusterService; + public WRRShardsCache wrrShardsCache; + public ClusterService clusterService; public OperationRouting(Settings settings, ClusterSettings clusterSettings) { // whether to ignore awareness attributes when routing requests @@ -146,8 +122,18 @@ public boolean ignoreAwarenessAttributes() { return this.awarenessAttributes.isEmpty() || this.ignoreAwarenessAttr; } - public WRRWeights getWrrWeights() { - return wrrWeights; + @Inject + public void setClusterService(ClusterService clusterService) { + this.clusterService = clusterService; + } + + @Inject + public void setWrrShardsCache(WRRShardsCache wrrShardsCache) { + this.wrrShardsCache = wrrShardsCache; + } + + public WRRShardsCache getWrrShardsCache() { + return wrrShardsCache; } public ShardIterator indexShards(ClusterState clusterState, String index, String id, @Nullable String routing) { @@ -173,7 +159,6 @@ public ShardIterator getShards( public ShardIterator getShards(ClusterState clusterState, String index, int shardId, @Nullable String preference) { final IndexShardRoutingTable indexShard = clusterState.getRoutingTable().shardRoutingTable(index, shardId); - setWeightedRoundRobinAttributes(clusterState, getClusterService()); return preferenceActiveShardIterator( indexShard, clusterState.nodes().getLocalNodeId(), @@ -203,7 +188,6 @@ public GroupShardsIterator searchShards( ) { final Set shards = computeTargetedShards(clusterState, concreteIndices, routing); final Set set = new HashSet<>(shards.size()); - setWeightedRoundRobinAttributes(clusterState, getClusterService()); for (IndexShardRoutingTable shard : shards) { ShardIterator iterator = preferenceActiveShardIterator( shard, @@ -220,18 +204,18 @@ public GroupShardsIterator searchShards( return GroupShardsIterator.sortAndCreate(new ArrayList<>(set)); } - private void setWeightedRoundRobinAttributes(ClusterState clusterState, ClusterService clusterService) { - WeightedRoundRobinRoutingMetadata weightedRoundRobinRoutingMetadata = clusterState.metadata() + private boolean isWeightedRoundRobinEnabled() { + WeightedRoundRobinRoutingMetadata weightedRoundRobinRoutingMetadata = clusterService.state() + .metadata() .custom(WeightedRoundRobinRoutingMetadata.TYPE); - this.isWeightedRoundRobinEnabled = weightedRoundRobinRoutingMetadata != null; - if (this.isWeightedRoundRobinEnabled) { - this.wrrWeights = weightedRoundRobinRoutingMetadata.getWrrWeight(); - this.wrrShardsCache = getWrrShardsCache() != null ? getWrrShardsCache() : new WRRShardsCache(clusterService); - } + return weightedRoundRobinRoutingMetadata != null; } - private boolean isWeightedRoundRobinEnabled() { - return isWeightedRoundRobinEnabled; + private WRRWeights fetchWRRWeights() { + WeightedRoundRobinRoutingMetadata weightedRoundRobinRoutingMetadata = clusterService.state() + .metadata() + .custom(WeightedRoundRobinRoutingMetadata.TYPE); + return weightedRoundRobinRoutingMetadata.getWrrWeight(); } public static ShardIterator getShards(ClusterState clusterState, ShardId shardId) { @@ -352,7 +336,7 @@ private ShardIterator shardRoutings( @Nullable Map nodeCounts ) { if (isWeightedRoundRobinEnabled()) { - return indexShard.activeInitializingShardsWRR(getWrrWeights(), nodes, wrrShardsCache); + return indexShard.activeInitializingShardsWRR(fetchWRRWeights(), nodes, wrrShardsCache); } else if (ignoreAwarenessAttributes()) { if (useAdaptiveReplicaSelection) { return indexShard.activeInitializingShardsRankedIt(collectorService, nodeCounts); diff --git a/server/src/main/java/org/opensearch/cluster/routing/WRRShardsCache.java b/server/src/main/java/org/opensearch/cluster/routing/WRRShardsCache.java index 879f73a304c76..f78d56d182291 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WRRShardsCache.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WRRShardsCache.java @@ -26,6 +26,7 @@ * * @opensearch.internal */ + public class WRRShardsCache implements Releasable, ClusterStateListener { private static final Logger logger = LogManager.getLogger(WRRShardsCache.class); @@ -41,16 +42,24 @@ public WRRShardsCache(ClusterService clusterService) { clusterService.addListener(this); } + public long hits() { + return cache.stats().getHits(); + } + + public long misses() { + return cache.stats().getMisses(); + } + + public long size() { + return cache.count(); + } + @Override public void close() { - logger.info("Invalidating WRRShardsCache on close"); + logger.debug("Invalidating WRRShardsCache on close"); cache.invalidateAll(); } - public Cache> getCache() { - return cache; - } - /** * Listens to cluster state change event and invalidate cache on such events * @@ -58,10 +67,18 @@ public Cache> getCache() { */ @Override public void clusterChanged(ClusterChangedEvent event) { - logger.info("Invalidating WRRShardsCache on ClusterChangedEvent"); + logger.debug("Invalidating WRRShardsCache on ClusterChangedEvent"); cache.invalidateAll(); } + public List get(Key k) { + return cache.get(k); + } + + public void put(Key key, List value) { + cache.put(key, value); + } + /** * Key for the WRRShardsCache * @@ -71,7 +88,6 @@ public static class Key { public final ShardId shardId; Key(ShardId shardId) { - this.shardId = shardId; } @@ -87,7 +103,6 @@ public boolean equals(Object o) { @Override public int hashCode() { int result = shardId.hashCode(); - return result; } diff --git a/server/src/main/java/org/opensearch/cluster/service/ClusterService.java b/server/src/main/java/org/opensearch/cluster/service/ClusterService.java index 4a8bf9275389a..2d31aca0073bf 100644 --- a/server/src/main/java/org/opensearch/cluster/service/ClusterService.java +++ b/server/src/main/java/org/opensearch/cluster/service/ClusterService.java @@ -45,7 +45,9 @@ import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.routing.OperationRouting; import org.opensearch.cluster.routing.RerouteService; +import org.opensearch.cluster.routing.WRRShardsCache; import org.opensearch.common.component.AbstractLifecycleComponent; +import org.opensearch.common.inject.Inject; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Setting.Property; @@ -108,7 +110,6 @@ public ClusterService( this.nodeName = Node.NODE_NAME_SETTING.get(settings); this.clusterManagerService = clusterManagerService; this.operationRouting = new OperationRouting(settings, clusterSettings); - this.operationRouting.setClusterService(this); this.clusterSettings = clusterSettings; this.clusterName = ClusterName.CLUSTER_NAME_SETTING.get(settings); // Add a no-op update consumer so changes are logged diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 92e9815313fa0..6c76096721754 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -36,6 +36,8 @@ import org.apache.logging.log4j.Logger; import org.apache.lucene.util.Constants; import org.apache.lucene.util.SetOnce; +import org.opensearch.cluster.routing.OperationRouting; +import org.opensearch.cluster.routing.WRRShardsCache; import org.opensearch.common.util.FeatureFlags; import org.opensearch.cluster.routing.allocation.AwarenessReplicaBalance; import org.opensearch.index.IndexingPressureService; @@ -906,6 +908,7 @@ protected Node( ); resourcesToClose.add(persistentTasksClusterService); final PersistentTasksService persistentTasksService = new PersistentTasksService(clusterService, threadPool, client); + final WRRShardsCache wrrShardsCache = new WRRShardsCache(clusterService); modules.add(b -> { b.bind(Node.class).toInstance(this); @@ -949,6 +952,7 @@ protected Node( b.bind(SnapshotsInfoService.class).toInstance(snapshotsInfoService); b.bind(GatewayMetaState.class).toInstance(gatewayMetaState); b.bind(Discovery.class).toInstance(discoveryModule.getDiscovery()); + b.bind(WRRShardsCache.class).toInstance(wrrShardsCache); { processRecoverySettings(settingsModule.getClusterSettings(), recoverySettings); b.bind(PeerRecoverySourceService.class) @@ -986,6 +990,7 @@ protected Node( b.bind(ShardLimitValidator.class).toInstance(shardLimitValidator); b.bind(FsHealthService.class).toInstance(fsHealthService); b.bind(SystemIndices.class).toInstance(systemIndices); + b.bind(OperationRouting.class).toInstance(clusterService.operationRouting()); }); injector = modules.createInjector(); @@ -1246,6 +1251,7 @@ private Node stop() { injector.getInstance(GatewayService.class).stop(); injector.getInstance(SearchService.class).stop(); injector.getInstance(TransportService.class).stop(); + injector.getInstance(WRRShardsCache.class).close(); pluginLifecycleComponents.forEach(LifecycleComponent::stop); // we should stop this last since it waits for resources to get released diff --git a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java index 3115070480aa9..efca5e77b4823 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java @@ -596,6 +596,7 @@ public void testAdaptiveReplicaSelection() throws Exception { ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool); ResponseCollectorService collector = new ResponseCollectorService(clusterService); Map outstandingRequests = new HashMap<>(); + opRouting.setClusterService(clusterService); GroupShardsIterator groupIterator = opRouting.searchShards( state, indexNames, @@ -700,7 +701,7 @@ public void testAdaptiveReplicaSelectionWithZoneAwarenessIgnored() throws Except Set selectedNodes = new HashSet<>(numShards); ResponseCollectorService collector = new ResponseCollectorService(clusterService); Map outstandingRequests = new HashMap<>(); - + opRouting.setClusterService(clusterService); GroupShardsIterator groupIterator = opRouting.searchShards( state, indexNames, @@ -789,6 +790,15 @@ private ClusterState clusterStateForWRR(String[] indexNames, int numShards, int } + private ClusterState setWRRWeights(ClusterState clusterState, Map weights) { + WRRWeights wrrWeight = new WRRWeights("zone", weights); + WeightedRoundRobinRoutingMetadata wrrMetadata = new WeightedRoundRobinRoutingMetadata(wrrWeight); + Metadata.Builder metadataBuilder = Metadata.builder(clusterState.metadata()); + metadataBuilder.putCustom(WeightedRoundRobinRoutingMetadata.TYPE, wrrMetadata); + clusterState = ClusterState.builder(clusterState).metadata(metadataBuilder).build(); + return clusterState; + } + public void testWRR() throws Exception { final int numIndices = 2; final int numShards = 3; @@ -810,7 +820,6 @@ public void testWRR() throws Exception { setting, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) ); - opRouting.setClusterService(clusterService); assertTrue(opRouting.ignoreAwarenessAttributes()); Set selectedNodes = new HashSet<>(); ResponseCollectorService collector = new ResponseCollectorService(clusterService); @@ -818,11 +827,13 @@ public void testWRR() throws Exception { // Setting up weights for weighted round-robin in cluster state Map weights = Map.of("a", "1", "b", "1", "c", "0"); - WRRWeights wrrWeight = new WRRWeights("zone", weights); - WeightedRoundRobinRoutingMetadata wrrMetadata = new WeightedRoundRobinRoutingMetadata(wrrWeight); - Metadata.Builder metadataBuilder = Metadata.builder(state.metadata()); - metadataBuilder.putCustom(WeightedRoundRobinRoutingMetadata.TYPE, wrrMetadata); - state = ClusterState.builder(state).metadata(metadataBuilder).build(); + state = setWRRWeights(state, weights); + + ClusterState.Builder builder = ClusterState.builder(state); + ClusterServiceUtils.setState(clusterService, builder); + + opRouting.setClusterService(clusterService); + opRouting.setWrrShardsCache(new WRRShardsCache(clusterService)); // search shards call GroupShardsIterator groupIterator = opRouting.searchShards( @@ -852,16 +863,14 @@ public void testWRR() throws Exception { // Updating weighted round robin weights in cluster state weights = Map.of("a", "1", "b", "0", "c", "1"); - wrrWeight = new WRRWeights("zone", weights); - wrrMetadata = new WeightedRoundRobinRoutingMetadata(wrrWeight); - metadataBuilder = Metadata.builder(state.metadata()); - metadataBuilder.putCustom(WeightedRoundRobinRoutingMetadata.TYPE, wrrMetadata); + state = setWRRWeights(state, weights); - // building cluster state with new weights - state = ClusterState.builder(state).metadata(metadataBuilder).build(); + builder = ClusterState.builder(state); + ClusterServiceUtils.setState(clusterService, builder); opRouting = new OperationRouting(setting, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)); opRouting.setClusterService(clusterService); + opRouting.setWrrShardsCache(new WRRShardsCache(clusterService)); // search shards call groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); @@ -903,6 +912,7 @@ public void testWRRShardsCaching() throws Exception { new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) ); opRouting.setClusterService(clusterService); + opRouting.setWrrShardsCache(new WRRShardsCache(clusterService)); assertTrue(opRouting.ignoreAwarenessAttributes()); ResponseCollectorService collector = new ResponseCollectorService(clusterService); @@ -910,11 +920,8 @@ public void testWRRShardsCaching() throws Exception { // Setting up weights for weighted round-robin in cluster state Map weights = Map.of("a", "1", "b", "1", "c", "0"); - WRRWeights wrrWeight = new WRRWeights("zone", weights); - WeightedRoundRobinRoutingMetadata wrrMetadata = new WeightedRoundRobinRoutingMetadata(wrrWeight); - Metadata.Builder metadataBuilder = Metadata.builder(state.metadata()); - metadataBuilder.putCustom(WeightedRoundRobinRoutingMetadata.TYPE, wrrMetadata); - state = ClusterState.builder(state).metadata(metadataBuilder).build(); + state = setWRRWeights(state, weights); + ClusterServiceUtils.setState(clusterService, ClusterState.builder(state)); // search shards call GroupShardsIterator groupIterator = opRouting.searchShards( @@ -927,44 +934,40 @@ public void testWRRShardsCaching() throws Exception { ); // shard wrr ordering details are not present in cache, the details are calculated and put in cache - assertEquals(6, opRouting.getWrrShardsCache().getCache().stats().getMisses()); - assertEquals(6, opRouting.getWrrShardsCache().getCache().count()); + assertEquals(6, opRouting.getWrrShardsCache().misses()); + assertEquals(6, opRouting.getWrrShardsCache().size()); // Calling operation routing again without any cluster state change to test that wrr shard routing details are // fetched from cache groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); // details are fetched from cache - assertEquals(12, opRouting.getWrrShardsCache().getCache().stats().getHits()); + assertEquals(12, opRouting.getWrrShardsCache().hits()); // cache count stays same - assertEquals(6, opRouting.getWrrShardsCache().getCache().count()); + assertEquals(6, opRouting.getWrrShardsCache().size()); // cache misses stay same - assertEquals(6, opRouting.getWrrShardsCache().getCache().stats().getMisses()); + assertEquals(6, opRouting.getWrrShardsCache().misses()); // Updating cluster state to test wrr shard routing details are calculated again weights = Map.of("a", "1", "b", "0", "c", "1"); - wrrWeight = new WRRWeights("zone", weights); - wrrMetadata = new WeightedRoundRobinRoutingMetadata(wrrWeight); - metadataBuilder = Metadata.builder(state.metadata()); - metadataBuilder.putCustom(WeightedRoundRobinRoutingMetadata.TYPE, wrrMetadata); // building new cluster state - ClusterState state2 = ClusterState.builder(state).metadata(metadataBuilder).build(); + ClusterState state2 = setWRRWeights(state, weights); + ClusterServiceUtils.setState(clusterService, ClusterState.builder(state2)); ClusterChangedEvent event = new ClusterChangedEvent("test", state2, state); opRouting.getWrrShardsCache().clusterChanged(event); // cache is invalidated after cluster state change, cache count is zero - assertEquals(0, opRouting.getWrrShardsCache().getCache().count()); + assertEquals(0, opRouting.getWrrShardsCache().size()); // search shards call groupIterator = opRouting.searchShards(state2, indexNames, null, null, collector, outstandingRequests); // cache hit remain same - assertEquals(12, opRouting.getWrrShardsCache().getCache().stats().getHits()); + assertEquals(12, opRouting.getWrrShardsCache().hits()); // cache miss increases by 6 - assertEquals(12, opRouting.getWrrShardsCache().getCache().stats().getMisses()); - - assertEquals(6, opRouting.getWrrShardsCache().getCache().count()); + assertEquals(12, opRouting.getWrrShardsCache().misses()); + assertEquals(6, opRouting.getWrrShardsCache().size()); IOUtils.close(clusterService); terminate(threadPool); @@ -979,7 +982,6 @@ public void testWRRWithWeightUndefinedForOneZone() throws Exception { for (int i = 0; i < numIndices; i++) { indexNames[i] = "test" + i; } - ClusterState state = clusterStateForWRR(indexNames, numShards, numReplicas); Settings setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); @@ -991,7 +993,6 @@ public void testWRRWithWeightUndefinedForOneZone() throws Exception { setting, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) ); - opRouting.setClusterService(clusterService); assertTrue(opRouting.ignoreAwarenessAttributes()); Set selectedNodes = new HashSet<>(); ResponseCollectorService collector = new ResponseCollectorService(clusterService); @@ -999,12 +1000,11 @@ public void testWRRWithWeightUndefinedForOneZone() throws Exception { // Setting up weights for weighted round-robin in cluster state, weight for nodes in zone b is not set Map weights = Map.of("a", "1", "c", "0"); - WRRWeights wrrWeight = new WRRWeights("zone", weights); - WeightedRoundRobinRoutingMetadata wrrMetadata = new WeightedRoundRobinRoutingMetadata(wrrWeight); - Metadata.Builder metadataBuilder = Metadata.builder(state.metadata()); - metadataBuilder.putCustom(WeightedRoundRobinRoutingMetadata.TYPE, wrrMetadata); - state = ClusterState.builder(state).metadata(metadataBuilder).build(); + state = setWRRWeights(state, weights); + ClusterServiceUtils.setState(clusterService, ClusterState.builder(state)); + opRouting.setClusterService(clusterService); + opRouting.setWrrShardsCache(new WRRShardsCache(clusterService)); // search shards call GroupShardsIterator groupIterator = opRouting.searchShards( state, @@ -1039,16 +1039,13 @@ public void testWRRWithWeightUndefinedForOneZone() throws Exception { // Updating weighted round robin weights in cluster state weights = Map.of("a", "0", "b", "1"); - wrrWeight = new WRRWeights("zone", weights); - wrrMetadata = new WeightedRoundRobinRoutingMetadata(wrrWeight); - metadataBuilder = Metadata.builder(state.metadata()); - metadataBuilder.putCustom(WeightedRoundRobinRoutingMetadata.TYPE, wrrMetadata); - // building cluster state with new weights - state = ClusterState.builder(state).metadata(metadataBuilder).build(); + state = setWRRWeights(state, weights); + ClusterServiceUtils.setState(clusterService, ClusterState.builder(state)); opRouting = new OperationRouting(setting, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)); opRouting.setClusterService(clusterService); + opRouting.setWrrShardsCache(new WRRShardsCache(clusterService)); // search shards call groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); diff --git a/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java b/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java index 68ad47fa1bbc9..223c9531dfe1c 100644 --- a/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java +++ b/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java @@ -50,9 +50,12 @@ import org.opensearch.cluster.routing.ShardsIterator; import org.opensearch.cluster.routing.allocation.AllocationService; import org.opensearch.cluster.routing.allocation.decider.ClusterRebalanceAllocationDecider; +import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.index.shard.ShardId; +import org.opensearch.test.ClusterServiceUtils; +import org.opensearch.threadpool.TestThreadPool; import java.util.Arrays; import java.util.Collections; @@ -422,11 +425,14 @@ public void testShardsAndPreferNodeRouting() { clusterState = startInitializingShardsAndReroute(strategy, clusterState); clusterState = startInitializingShardsAndReroute(strategy, clusterState); + TestThreadPool threadPool = new TestThreadPool("testThatOnlyNodesSupport"); + ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool); + OperationRouting operationRouting = new OperationRouting( Settings.EMPTY, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) ); - + operationRouting.setClusterService(clusterService); GroupShardsIterator shardIterators = operationRouting.searchShards( clusterState, new String[] { "test" }, @@ -468,6 +474,7 @@ public void testShardsAndPreferNodeRouting() { } else { assertThat(it.nextOrNull().currentNodeId(), equalTo("node1")); } + terminate(threadPool); } public void testReplicaShardPreferenceIters() throws Exception { From cd3e16c914ee2508ff24e5d751c04895167e7996 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Mon, 12 Sep 2022 19:49:38 +0530 Subject: [PATCH 10/31] Remove extra new lines Signed-off-by: Anshu Agarwal --- .../cluster/metadata/WeightedRoundRobinRoutingMetadata.java | 2 -- .../java/org/opensearch/cluster/routing/OperationRouting.java | 2 -- .../main/java/org/opensearch/cluster/routing/WRRWeights.java | 2 +- .../java/org/opensearch/cluster/service/ClusterService.java | 2 -- .../org/opensearch/cluster/routing/OperationRoutingTests.java | 1 - 5 files changed, 1 insertion(+), 8 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadata.java index b1baab52aaeb4..08c7f2901b684 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadata.java @@ -50,7 +50,6 @@ public WeightedRoundRobinRoutingMetadata setWrrWeight(WRRWeights wrrWeight) { public WeightedRoundRobinRoutingMetadata(StreamInput in) throws IOException { if (in.available() != 0) { this.wrrWeight = new WRRWeights(in); - } } @@ -71,7 +70,6 @@ public String getWriteableName() { @Override public Version getMinimalSupportedVersion() { return Version.V_2_3_0; - } @Override diff --git a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java index 24b85d991bfda..6eb0c16ef678b 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java +++ b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java @@ -95,7 +95,6 @@ public OperationRouting(Settings settings, ClusterSettings clusterSettings) { this.useAdaptiveReplicaSelection = USE_ADAPTIVE_REPLICA_SELECTION_SETTING.get(settings); clusterSettings.addSettingsUpdateConsumer(USE_ADAPTIVE_REPLICA_SELECTION_SETTING, this::setUseAdaptiveReplicaSelection); clusterSettings.addSettingsUpdateConsumer(IGNORE_AWARENESS_ATTRIBUTES_SETTING, this::setIgnoreAwarenessAttributes); - } void setUseAdaptiveReplicaSelection(boolean useAdaptiveReplicaSelection) { @@ -261,7 +260,6 @@ private ShardIterator preferenceActiveShardIterator( @Nullable ResponseCollectorService collectorService, @Nullable Map nodeCounts ) { - if (preference == null || preference.isEmpty()) { return shardRoutings(indexShard, nodes, collectorService, nodeCounts); } diff --git a/server/src/main/java/org/opensearch/cluster/routing/WRRWeights.java b/server/src/main/java/org/opensearch/cluster/routing/WRRWeights.java index daf2986ea8b71..58842dded1a52 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WRRWeights.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WRRWeights.java @@ -64,7 +64,7 @@ public int hashCode() { @Override public String toString() { - return "WRRWeightsDefinition{" + attributeName + "}{" + weights().toString() + "}"; + return "WRRWeights{" + attributeName + "}{" + weights().toString() + "}"; } public Map weights() { diff --git a/server/src/main/java/org/opensearch/cluster/service/ClusterService.java b/server/src/main/java/org/opensearch/cluster/service/ClusterService.java index 2d31aca0073bf..d393613118af8 100644 --- a/server/src/main/java/org/opensearch/cluster/service/ClusterService.java +++ b/server/src/main/java/org/opensearch/cluster/service/ClusterService.java @@ -45,9 +45,7 @@ import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.routing.OperationRouting; import org.opensearch.cluster.routing.RerouteService; -import org.opensearch.cluster.routing.WRRShardsCache; import org.opensearch.common.component.AbstractLifecycleComponent; -import org.opensearch.common.inject.Inject; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Setting.Property; diff --git a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java index efca5e77b4823..7ad5d413f6ad7 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java @@ -104,7 +104,6 @@ public void testGenerateShardId() { .settings(settings(Version.CURRENT)) .numberOfShards(shardSplits[2]) .numberOfReplicas(1) - .setRoutingNumShards(shardSplits[0]) .build(); shrunkShard = OperationRouting.generateShardId(shrunk, term, null); From 0c7dbd95f1067bdd543f3ee306d2d44bb92ec57e Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Mon, 12 Sep 2022 20:02:41 +0530 Subject: [PATCH 11/31] Fix import Signed-off-by: Anshu Agarwal --- .../java/org/opensearch/cluster/routing/OperationRouting.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java index 6eb0c16ef678b..b4cca1c691340 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java +++ b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java @@ -40,6 +40,7 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.Nullable; import org.opensearch.common.Strings; +import org.opensearch.common.inject.Inject; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; From 438fe9ee1df023ca1e0756b72c5b64a9df691424 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Mon, 12 Sep 2022 20:40:20 +0530 Subject: [PATCH 12/31] Add size for shard routing cache Signed-off-by: Anshu Agarwal --- .../java/org/opensearch/cluster/routing/WRRShardsCache.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/WRRShardsCache.java b/server/src/main/java/org/opensearch/cluster/routing/WRRShardsCache.java index f78d56d182291..40e9a8448489f 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WRRShardsCache.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WRRShardsCache.java @@ -31,10 +31,10 @@ public class WRRShardsCache implements Releasable, ClusterStateListener { private static final Logger logger = LogManager.getLogger(WRRShardsCache.class); private final Cache> cache; + private static final long sizeInBytes = 2000000; public WRRShardsCache(ClusterService clusterService) { - final long sizeInBytes = 2000000; CacheBuilder> cacheBuilder = CacheBuilder.>builder() .removalListener(notification -> logger.info("Object" + " {} removed from cache", notification.getKey().shardId)) .setMaximumWeight(sizeInBytes); From 694be4df1953e6323ff1e9a0a5a8fc3d780a9b03 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Mon, 12 Sep 2022 20:51:49 +0530 Subject: [PATCH 13/31] Invalidate shard routing cache on close Signed-off-by: Anshu Agarwal --- server/src/main/java/org/opensearch/node/Node.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 6c76096721754..156d544ee14c2 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -1251,7 +1251,6 @@ private Node stop() { injector.getInstance(GatewayService.class).stop(); injector.getInstance(SearchService.class).stop(); injector.getInstance(TransportService.class).stop(); - injector.getInstance(WRRShardsCache.class).close(); pluginLifecycleComponents.forEach(LifecycleComponent::stop); // we should stop this last since it waits for resources to get released @@ -1338,6 +1337,7 @@ public synchronized void close() throws IOException { toClose.add(() -> stopWatch.stop().start("node_environment")); toClose.add(injector.getInstance(NodeEnvironment.class)); toClose.add(stopWatch::stop); + toClose.add(injector.getInstance(WRRShardsCache.class)); if (logger.isTraceEnabled()) { toClose.add(() -> logger.trace("Close times for each service:\n{}", stopWatch.prettyPrint())); From 9236b8d05f290aa52c9622c6bf82dbd77be6413d Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Tue, 13 Sep 2022 19:13:43 +0530 Subject: [PATCH 14/31] Refactor code Signed-off-by: Anshu Agarwal --- .../org/opensearch/cluster/ClusterModule.java | 13 +-- .../opensearch/cluster/metadata/Metadata.java | 8 ++ ...data.java => WeightedRoutingMetadata.java} | 67 ++++++++------ .../routing/IndexShardRoutingTable.java | 66 +++++++------- .../cluster/routing/OperationRouting.java | 33 +++---- .../cluster/routing/WeightedRoundRobin.java | 2 +- .../{WRRWeights.java => WeightedRouting.java} | 16 ++-- ...dsCache.java => WeightedRoutingCache.java} | 16 ++-- .../main/java/org/opensearch/node/Node.java | 8 +- ...java => WeightedRoutingMetadataTests.java} | 14 +-- .../routing/OperationRoutingTests.java | 44 +++++----- .../routing/WeightedRoundRobinTests.java | 88 +++++++++++++++++-- 12 files changed, 224 insertions(+), 151 deletions(-) rename server/src/main/java/org/opensearch/cluster/metadata/{WeightedRoundRobinRoutingMetadata.java => WeightedRoutingMetadata.java} (62%) rename server/src/main/java/org/opensearch/cluster/routing/{WRRWeights.java => WeightedRouting.java} (75%) rename server/src/main/java/org/opensearch/cluster/routing/{WRRShardsCache.java => WeightedRoutingCache.java} (83%) rename server/src/test/java/org/opensearch/cluster/metadata/{WeightedRoundRobinRoutingMetadataTests.java => WeightedRoutingMetadataTests.java} (52%) diff --git a/server/src/main/java/org/opensearch/cluster/ClusterModule.java b/server/src/main/java/org/opensearch/cluster/ClusterModule.java index 99a342eaeb5ed..46552bb5d6a03 100644 --- a/server/src/main/java/org/opensearch/cluster/ClusterModule.java +++ b/server/src/main/java/org/opensearch/cluster/ClusterModule.java @@ -48,7 +48,7 @@ import org.opensearch.cluster.metadata.MetadataMappingService; import org.opensearch.cluster.metadata.MetadataUpdateSettingsService; import org.opensearch.cluster.metadata.RepositoriesMetadata; -import org.opensearch.cluster.metadata.WeightedRoundRobinRoutingMetadata; +import org.opensearch.cluster.metadata.WeightedRoutingMetadata; import org.opensearch.cluster.routing.DelayedAllocationService; import org.opensearch.cluster.routing.allocation.AllocationService; import org.opensearch.cluster.routing.allocation.ExistingShardsAllocator; @@ -192,12 +192,7 @@ public static List getNamedWriteables() { ComposableIndexTemplateMetadata::readDiffFrom ); registerMetadataCustom(entries, DataStreamMetadata.TYPE, DataStreamMetadata::new, DataStreamMetadata::readDiffFrom); - registerMetadataCustom( - entries, - WeightedRoundRobinRoutingMetadata.TYPE, - WeightedRoundRobinRoutingMetadata::new, - WeightedRoundRobinRoutingMetadata::readDiffFrom - ); + registerMetadataCustom(entries, WeightedRoutingMetadata.TYPE, WeightedRoutingMetadata::new, WeightedRoutingMetadata::readDiffFrom); // Task Status (not Diffable) entries.add(new Entry(Task.Status.class, PersistentTasksNodeService.Status.NAME, PersistentTasksNodeService.Status::new)); return entries; @@ -284,8 +279,8 @@ public static List getNamedXWriteables() { entries.add( new NamedXContentRegistry.Entry( Metadata.Custom.class, - new ParseField(WeightedRoundRobinRoutingMetadata.TYPE), - WeightedRoundRobinRoutingMetadata::fromXContent + new ParseField(WeightedRoutingMetadata.TYPE), + WeightedRoutingMetadata::fromXContent ) ); return entries; diff --git a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java index 5f7e98e9e1199..086865d2170c3 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java @@ -810,6 +810,14 @@ public IndexGraveyard indexGraveyard() { return custom(IndexGraveyard.TYPE); } + /** + * * + * @return The weighted routing metadata for search requests + */ + public WeightedRoutingMetadata weightedRoutingMetadata() { + return custom(WeightedRoutingMetadata.TYPE); + } + public T custom(String type) { return (T) customs.get(type); } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java similarity index 62% rename from server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadata.java rename to server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java index 08c7f2901b684..9c9b8bf38b4a7 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java @@ -14,7 +14,7 @@ import org.opensearch.Version; import org.opensearch.cluster.AbstractNamedDiffable; import org.opensearch.cluster.NamedDiff; -import org.opensearch.cluster.routing.WRRWeights; +import org.opensearch.cluster.routing.WeightedRouting; import org.opensearch.common.Strings; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; @@ -32,29 +32,29 @@ * * @opensearch.internal */ -public class WeightedRoundRobinRoutingMetadata extends AbstractNamedDiffable implements Metadata.Custom { - private static final Logger logger = LogManager.getLogger(WeightedRoundRobinRoutingMetadata.class); - public static final String TYPE = "wrr_shard_routing"; +public class WeightedRoutingMetadata extends AbstractNamedDiffable implements Metadata.Custom { + private static final Logger logger = LogManager.getLogger(WeightedRoutingMetadata.class); + public static final String TYPE = "weighted_shard_routing"; public static final String AWARENESS = "awareness"; - private WRRWeights wrrWeight; + private WeightedRouting weightedRouting; - public WRRWeights getWrrWeight() { - return wrrWeight; + public WeightedRouting getWeightedRouting() { + return weightedRouting; } - public WeightedRoundRobinRoutingMetadata setWrrWeight(WRRWeights wrrWeight) { - this.wrrWeight = wrrWeight; + public WeightedRoutingMetadata setWeightedRouting(WeightedRouting weightedRouting) { + this.weightedRouting = weightedRouting; return this; } - public WeightedRoundRobinRoutingMetadata(StreamInput in) throws IOException { + public WeightedRoutingMetadata(StreamInput in) throws IOException { if (in.available() != 0) { - this.wrrWeight = new WRRWeights(in); + this.weightedRouting = new WeightedRouting(in); } } - public WeightedRoundRobinRoutingMetadata(WRRWeights wrrWeight) { - this.wrrWeight = wrrWeight; + public WeightedRoutingMetadata(WeightedRouting weightedRouting) { + this.weightedRouting = weightedRouting; } @Override @@ -74,8 +74,8 @@ public Version getMinimalSupportedVersion() { @Override public void writeTo(StreamOutput out) throws IOException { - if (wrrWeight != null) { - wrrWeight.writeTo(out); + if (weightedRouting != null) { + weightedRouting.writeTo(out); } } @@ -83,12 +83,12 @@ public static NamedDiff readDiffFrom(StreamInput in) throws IOE return readDiffFrom(Metadata.Custom.class, TYPE, in); } - public static WeightedRoundRobinRoutingMetadata fromXContent(XContentParser parser) throws IOException { + public static WeightedRoutingMetadata fromXContent(XContentParser parser) throws IOException { String attrKey = null; Object attrValue; String attributeName = null; Map weights = new HashMap<>(); - WRRWeights wrrWeight = null; + WeightedRouting weightedRouting = null; XContentParser.Token token; // move to the first alias parser.nextToken(); @@ -98,12 +98,18 @@ public static WeightedRoundRobinRoutingMetadata fromXContent(XContentParser pars if (token == XContentParser.Token.FIELD_NAME) { awarenessField = parser.currentName(); if (parser.nextToken() != XContentParser.Token.START_OBJECT) { - throw new OpenSearchParseException("failed to parse wrr metadata [{}], expected object", awarenessField); + throw new OpenSearchParseException( + "failed to parse weighted routing metadata [{}], expected " + "object", + awarenessField + ); } while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { attributeName = parser.currentName(); if (parser.nextToken() != XContentParser.Token.START_OBJECT) { - throw new OpenSearchParseException("failed to parse wrr metadata [{}], expected object", attributeName); + throw new OpenSearchParseException( + "failed to parse weighted routing metadata [{}], expected" + " object", + attributeName + ); } while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { @@ -112,39 +118,42 @@ public static WeightedRoundRobinRoutingMetadata fromXContent(XContentParser pars attrValue = parser.text(); weights.put(attrKey, attrValue); } else { - throw new OpenSearchParseException("failed to parse wrr metadata attribute [{}], unknown type", attributeName); + throw new OpenSearchParseException( + "failed to parse weighted routing metadata attribute " + "[{}], unknown type", + attributeName + ); } } } } } - wrrWeight = new WRRWeights(attributeName, weights); - return new WeightedRoundRobinRoutingMetadata(wrrWeight); + weightedRouting = new WeightedRouting(attributeName, weights); + return new WeightedRoutingMetadata(weightedRouting); } @Override public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - WeightedRoundRobinRoutingMetadata that = (WeightedRoundRobinRoutingMetadata) o; - return wrrWeight.equals(that.wrrWeight); + WeightedRoutingMetadata that = (WeightedRoutingMetadata) o; + return weightedRouting.equals(that.weightedRouting); } @Override public int hashCode() { - return wrrWeight.hashCode(); + return weightedRouting.hashCode(); } @Override public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException { - toXContent(wrrWeight, builder); + toXContent(weightedRouting, builder); return builder; } - public static void toXContent(WRRWeights wrrWeight, XContentBuilder builder) throws IOException { + public static void toXContent(WeightedRouting weightedRouting, XContentBuilder builder) throws IOException { builder.startObject(AWARENESS); - builder.startObject(wrrWeight.attributeName()); - for (Map.Entry entry : wrrWeight.weights().entrySet()) { + builder.startObject(weightedRouting.attributeName()); + for (Map.Entry entry : weightedRouting.weights().entrySet()) { builder.field(entry.getKey(), entry.getValue()); } builder.endObject(); diff --git a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java index d0e79f8efda2e..e2352dd6ce6f3 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java +++ b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java @@ -59,6 +59,7 @@ import java.util.function.Predicate; import static java.util.Collections.emptyMap; +import static java.util.stream.Collectors.toList; /** * {@link IndexShardRoutingTable} encapsulates all instances of a single shard. @@ -293,71 +294,66 @@ public ShardIterator activeInitializingShardsRankedIt( } /** - * Returns an iterator over active and initializing shards, shards are ordered by weighted round-robin scheduling - * policy. - * @param wrrWeight Weighted round-robin weight entity - * @param nodes discovered nodes in the cluster + * Returns an iterator over active and initializing shards, shards are ordered by weighted + * round-robin scheduling policy. + * + * @param weightedRouting entity + * @param nodes discovered nodes in the cluster * @return an iterator over active and initializing shards, ordered by weighted round-robin * scheduling policy. Making sure that initializing shards are the last to iterate through. */ - public ShardIterator activeInitializingShardsWRR(WRRWeights wrrWeight, DiscoveryNodes nodes, WRRShardsCache cache) { + public ShardIterator activeInitializingShardsWeightedIt( + WeightedRouting weightedRouting, + DiscoveryNodes nodes, + WeightedRoutingCache cache + ) { final int seed = shuffler.nextSeed(); List ordered = new ArrayList<>(activeShards.size() + allInitializingShards.size()); List orderedActiveShards; - if (cache.get(new WRRShardsCache.Key(shardId)) != null) { - orderedActiveShards = cache.get(new WRRShardsCache.Key(shardId)); + if (cache.get(new WeightedRoutingCache.Key(shardId)) != null) { + orderedActiveShards = cache.get(new WeightedRoutingCache.Key(shardId)); } else { - orderedActiveShards = getShardsWRR(activeShards, wrrWeight, nodes); - cache.put(new WRRShardsCache.Key(shardId), orderedActiveShards); + orderedActiveShards = shardsOrderedByWeight(activeShards, weightedRouting, nodes); + cache.put(new WeightedRoutingCache.Key(shardId), orderedActiveShards); } ordered.addAll(shuffler.shuffle(orderedActiveShards, seed)); if (!allInitializingShards.isEmpty()) { - List orderedInitializingShards = getShardsWRR(allInitializingShards, wrrWeight, nodes); + List orderedInitializingShards = shardsOrderedByWeight(allInitializingShards, weightedRouting, nodes); ordered.addAll(orderedInitializingShards); } return new PlainShardIterator(shardId, ordered); } /** - * - * @param shards shards to be ordered using weighted round-robin scheduling policy - * @param wrrWeight weights to be considered for routing - * @param nodes discovered nodes in the cluster - * @return list of shards ordered using weighted round-robin scheduling. + * Returns a list containing shard routings ordered using weighted round-robin scheduling. */ - private List getShardsWRR(List shards, WRRWeights wrrWeight, DiscoveryNodes nodes) { - List> weightedShards = calculateShardWeight(shards, wrrWeight, nodes); - WeightedRoundRobin wrr = new WeightedRoundRobin<>(weightedShards); - List> wrrOrderedActiveShards = wrr.orderEntities(); - List orderedActiveShards = new ArrayList<>(activeShards.size()); - for (WeightedRoundRobin.Entity shardRouting : wrrOrderedActiveShards) { - orderedActiveShards.add(shardRouting.getTarget()); - } - return orderedActiveShards; + private List shardsOrderedByWeight(List shards, WeightedRouting weightedRouting, DiscoveryNodes nodes) { + WeightedRoundRobin weightedRoundRobin = new WeightedRoundRobin<>( + calculateShardWeight(shards, weightedRouting, nodes) + ); + return weightedRoundRobin.orderEntities().stream().map(WeightedRoundRobin.Entity::getTarget).collect(toList()); } /** - * - * @param shards associate weights to shards - * @param wrrWeight weights to be used for association - * @param nodes discovered nodes in the cluster - * @return list of entity containing shard routing and associated weight. + * Returns a list containing shard routing and associated weight. This function iterates through all the shards and + * uses weighted routing to find weight for the corresponding shard. This is fed to weighted round-robin scheduling + * to order shards by weight. */ private List> calculateShardWeight( List shards, - WRRWeights wrrWeight, + WeightedRouting weightedRouting, DiscoveryNodes nodes ) { - List> weightedShards = new ArrayList<>(); + List> shardsWithWeights = new ArrayList<>(); for (ShardRouting shard : shards) { DiscoveryNode node = nodes.get(shard.currentNodeId()); - String attVal = node.getAttributes().get(wrrWeight.attributeName()); + String attVal = node.getAttributes().get(weightedRouting.attributeName()); // If weight for a zone is not defined, considering it as 1 by default - Double weight = Double.parseDouble(wrrWeight.weights().getOrDefault(attVal, 1).toString()); - weightedShards.add(new WeightedRoundRobin.Entity<>(weight, shard)); + Double weight = Double.parseDouble(weightedRouting.weights().getOrDefault(attVal, 1).toString()); + shardsWithWeights.add(new WeightedRoundRobin.Entity<>(weight, shard)); } - return weightedShards; + return shardsWithWeights; } private static Set getAllNodeIds(final List shards) { diff --git a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java index b4cca1c691340..a5acbed983cc3 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java +++ b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java @@ -34,7 +34,7 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.IndexMetadata; -import org.opensearch.cluster.metadata.WeightedRoundRobinRoutingMetadata; +import org.opensearch.cluster.metadata.WeightedRoutingMetadata; import org.opensearch.cluster.node.DiscoveryNodes; import org.opensearch.cluster.routing.allocation.decider.AwarenessAllocationDecider; import org.opensearch.cluster.service.ClusterService; @@ -82,8 +82,8 @@ public class OperationRouting { private volatile boolean useAdaptiveReplicaSelection; private volatile boolean ignoreAwarenessAttr; - public WRRShardsCache wrrShardsCache; - public ClusterService clusterService; + private WeightedRoutingCache weightedRoutingCache; + private ClusterService clusterService; public OperationRouting(Settings settings, ClusterSettings clusterSettings) { // whether to ignore awareness attributes when routing requests @@ -128,12 +128,12 @@ public void setClusterService(ClusterService clusterService) { } @Inject - public void setWrrShardsCache(WRRShardsCache wrrShardsCache) { - this.wrrShardsCache = wrrShardsCache; + public void setWeightedRoutingCache(WeightedRoutingCache weightedRoutingCache) { + this.weightedRoutingCache = weightedRoutingCache; } - public WRRShardsCache getWrrShardsCache() { - return wrrShardsCache; + public WeightedRoutingCache getWeightedRoutingCache() { + return weightedRoutingCache; } public ShardIterator indexShards(ClusterState clusterState, String index, String id, @Nullable String routing) { @@ -204,20 +204,6 @@ public GroupShardsIterator searchShards( return GroupShardsIterator.sortAndCreate(new ArrayList<>(set)); } - private boolean isWeightedRoundRobinEnabled() { - WeightedRoundRobinRoutingMetadata weightedRoundRobinRoutingMetadata = clusterService.state() - .metadata() - .custom(WeightedRoundRobinRoutingMetadata.TYPE); - return weightedRoundRobinRoutingMetadata != null; - } - - private WRRWeights fetchWRRWeights() { - WeightedRoundRobinRoutingMetadata weightedRoundRobinRoutingMetadata = clusterService.state() - .metadata() - .custom(WeightedRoundRobinRoutingMetadata.TYPE); - return weightedRoundRobinRoutingMetadata.getWrrWeight(); - } - public static ShardIterator getShards(ClusterState clusterState, ShardId shardId) { final IndexShardRoutingTable shard = clusterState.routingTable().shardRoutingTable(shardId); return shard.activeInitializingShardsRandomIt(); @@ -334,8 +320,9 @@ private ShardIterator shardRoutings( @Nullable ResponseCollectorService collectorService, @Nullable Map nodeCounts ) { - if (isWeightedRoundRobinEnabled()) { - return indexShard.activeInitializingShardsWRR(fetchWRRWeights(), nodes, wrrShardsCache); + WeightedRoutingMetadata weightedRoutingMetadata = clusterService.state().metadata().weightedRoutingMetadata(); + if (weightedRoutingMetadata != null) { + return indexShard.activeInitializingShardsWeightedIt(weightedRoutingMetadata.getWeightedRouting(), nodes, weightedRoutingCache); } else if (ignoreAwarenessAttributes()) { if (useAdaptiveReplicaSelection) { return indexShard.activeInitializingShardsRankedIt(collectorService, nodeCounts); diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java index dd1966f1c6adb..0484f8cd0d93e 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java @@ -46,7 +46,7 @@ public List> orderEntities() { for (WeightedRoundRobin.Entity entity : entities) { maxWeight = Math.max(maxWeight, entity.getWeight()); gcd = (gcd == null) ? entity.getWeight() : gcd(gcd, entity.getWeight()); - sumWeight += entity.getWeight(); + sumWeight += entity.getWeight() > 0 ? entity.getWeight() : 0; } int count = 0; while (count < sumWeight) { diff --git a/server/src/main/java/org/opensearch/cluster/routing/WRRWeights.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRouting.java similarity index 75% rename from server/src/main/java/org/opensearch/cluster/routing/WRRWeights.java rename to server/src/main/java/org/opensearch/cluster/routing/WeightedRouting.java index 58842dded1a52..90f9b1beb2ed1 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WRRWeights.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRouting.java @@ -21,21 +21,21 @@ * * @opensearch.internal */ -public class WRRWeights implements Writeable { +public class WeightedRouting implements Writeable { private String attributeName; private Map weights; - public WRRWeights(String attributeName, Map weights) { + public WeightedRouting(String attributeName, Map weights) { this.attributeName = attributeName; this.weights = weights; } - public WRRWeights(WRRWeights wrrWeight) { - this.attributeName = wrrWeight.attributeName(); - this.weights = wrrWeight.weights; + public WeightedRouting(WeightedRouting weightedRouting) { + this.attributeName = weightedRouting.attributeName(); + this.weights = weightedRouting.weights; } - public WRRWeights(StreamInput in) throws IOException { + public WeightedRouting(StreamInput in) throws IOException { attributeName = in.readString(); weights = in.readMap(); } @@ -51,7 +51,7 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - WRRWeights that = (WRRWeights) o; + WeightedRouting that = (WeightedRouting) o; if (!attributeName.equals(that.attributeName)) return false; return weights.equals(that.weights); @@ -64,7 +64,7 @@ public int hashCode() { @Override public String toString() { - return "WRRWeights{" + attributeName + "}{" + weights().toString() + "}"; + return "WeightedRouting{" + attributeName + "}{" + weights().toString() + "}"; } public Map weights() { diff --git a/server/src/main/java/org/opensearch/cluster/routing/WRRShardsCache.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingCache.java similarity index 83% rename from server/src/main/java/org/opensearch/cluster/routing/WRRShardsCache.java rename to server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingCache.java index 40e9a8448489f..a525f26e0176e 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WRRShardsCache.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingCache.java @@ -27,13 +27,13 @@ * @opensearch.internal */ -public class WRRShardsCache implements Releasable, ClusterStateListener { - private static final Logger logger = LogManager.getLogger(WRRShardsCache.class); +public class WeightedRoutingCache implements Releasable, ClusterStateListener { + private static final Logger logger = LogManager.getLogger(WeightedRoutingCache.class); private final Cache> cache; private static final long sizeInBytes = 2000000; - public WRRShardsCache(ClusterService clusterService) { + public WeightedRoutingCache(ClusterService clusterService) { CacheBuilder> cacheBuilder = CacheBuilder.>builder() .removalListener(notification -> logger.info("Object" + " {} removed from cache", notification.getKey().shardId)) @@ -56,7 +56,7 @@ public long size() { @Override public void close() { - logger.debug("Invalidating WRRShardsCache on close"); + logger.debug("Invalidating WeightedRoutingCache on close"); cache.invalidateAll(); } @@ -67,7 +67,7 @@ public void close() { */ @Override public void clusterChanged(ClusterChangedEvent event) { - logger.debug("Invalidating WRRShardsCache on ClusterChangedEvent"); + logger.debug("Invalidating WeightedRoutingCache on ClusterChangedEvent"); cache.invalidateAll(); } @@ -80,12 +80,12 @@ public void put(Key key, List value) { } /** - * Key for the WRRShardsCache + * Key for the WeightedRoutingCache * * @opensearch.internal */ public static class Key { - public final ShardId shardId; + private final ShardId shardId; Key(ShardId shardId) { this.shardId = shardId; @@ -95,7 +95,7 @@ public static class Key { public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - WRRShardsCache.Key key = (WRRShardsCache.Key) o; + WeightedRoutingCache.Key key = (WeightedRoutingCache.Key) o; if (!shardId.equals(key.shardId)) return false; return true; } diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 156d544ee14c2..d0ab73e4fcc29 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -37,7 +37,7 @@ import org.apache.lucene.util.Constants; import org.apache.lucene.util.SetOnce; import org.opensearch.cluster.routing.OperationRouting; -import org.opensearch.cluster.routing.WRRShardsCache; +import org.opensearch.cluster.routing.WeightedRoutingCache; import org.opensearch.common.util.FeatureFlags; import org.opensearch.cluster.routing.allocation.AwarenessReplicaBalance; import org.opensearch.index.IndexingPressureService; @@ -908,7 +908,7 @@ protected Node( ); resourcesToClose.add(persistentTasksClusterService); final PersistentTasksService persistentTasksService = new PersistentTasksService(clusterService, threadPool, client); - final WRRShardsCache wrrShardsCache = new WRRShardsCache(clusterService); + final WeightedRoutingCache weightedRoutingCache = new WeightedRoutingCache(clusterService); modules.add(b -> { b.bind(Node.class).toInstance(this); @@ -952,7 +952,7 @@ protected Node( b.bind(SnapshotsInfoService.class).toInstance(snapshotsInfoService); b.bind(GatewayMetaState.class).toInstance(gatewayMetaState); b.bind(Discovery.class).toInstance(discoveryModule.getDiscovery()); - b.bind(WRRShardsCache.class).toInstance(wrrShardsCache); + b.bind(WeightedRoutingCache.class).toInstance(weightedRoutingCache); { processRecoverySettings(settingsModule.getClusterSettings(), recoverySettings); b.bind(PeerRecoverySourceService.class) @@ -1337,7 +1337,7 @@ public synchronized void close() throws IOException { toClose.add(() -> stopWatch.stop().start("node_environment")); toClose.add(injector.getInstance(NodeEnvironment.class)); toClose.add(stopWatch::stop); - toClose.add(injector.getInstance(WRRShardsCache.class)); + toClose.add(injector.getInstance(WeightedRoutingCache.class)); if (logger.isTraceEnabled()) { toClose.add(() -> logger.trace("Close times for each service:\n{}", stopWatch.prettyPrint())); diff --git a/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadataTests.java b/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoutingMetadataTests.java similarity index 52% rename from server/src/test/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadataTests.java rename to server/src/test/java/org/opensearch/cluster/metadata/WeightedRoutingMetadataTests.java index a5f3b8a1ccefa..c1e4c380db9ac 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoundRobinRoutingMetadataTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoutingMetadataTests.java @@ -8,25 +8,25 @@ package org.opensearch.cluster.metadata; -import org.opensearch.cluster.routing.WRRWeights; +import org.opensearch.cluster.routing.WeightedRouting; import org.opensearch.common.xcontent.XContentParser; import org.opensearch.test.AbstractXContentTestCase; import java.io.IOException; import java.util.Map; -public class WeightedRoundRobinRoutingMetadataTests extends AbstractXContentTestCase { +public class WeightedRoutingMetadataTests extends AbstractXContentTestCase { @Override - protected WeightedRoundRobinRoutingMetadata createTestInstance() { + protected WeightedRoutingMetadata createTestInstance() { Map weights = Map.of("a", "1", "b", "1", "c", "0"); - WRRWeights wrrWeights = new WRRWeights("zone", weights); - WeightedRoundRobinRoutingMetadata wrrMetadata = new WeightedRoundRobinRoutingMetadata(wrrWeights); + WeightedRouting weightedRouting = new WeightedRouting("zone", weights); + WeightedRoutingMetadata wrrMetadata = new WeightedRoutingMetadata(weightedRouting); return wrrMetadata; } @Override - protected WeightedRoundRobinRoutingMetadata doParseInstance(XContentParser parser) throws IOException { - return WeightedRoundRobinRoutingMetadata.fromXContent(parser); + protected WeightedRoutingMetadata doParseInstance(XContentParser parser) throws IOException { + return WeightedRoutingMetadata.fromXContent(parser); } @Override diff --git a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java index 7ad5d413f6ad7..2f9fe3cd991ac 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java @@ -37,7 +37,7 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.Metadata; -import org.opensearch.cluster.metadata.WeightedRoundRobinRoutingMetadata; +import org.opensearch.cluster.metadata.WeightedRoutingMetadata; import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.node.DiscoveryNodeRole; import org.opensearch.cluster.routing.allocation.decider.AwarenessAllocationDecider; @@ -790,10 +790,10 @@ private ClusterState clusterStateForWRR(String[] indexNames, int numShards, int } private ClusterState setWRRWeights(ClusterState clusterState, Map weights) { - WRRWeights wrrWeight = new WRRWeights("zone", weights); - WeightedRoundRobinRoutingMetadata wrrMetadata = new WeightedRoundRobinRoutingMetadata(wrrWeight); + WeightedRouting wrrWeight = new WeightedRouting("zone", weights); + WeightedRoutingMetadata wrrMetadata = new WeightedRoutingMetadata(wrrWeight); Metadata.Builder metadataBuilder = Metadata.builder(clusterState.metadata()); - metadataBuilder.putCustom(WeightedRoundRobinRoutingMetadata.TYPE, wrrMetadata); + metadataBuilder.putCustom(WeightedRoutingMetadata.TYPE, wrrMetadata); clusterState = ClusterState.builder(clusterState).metadata(metadataBuilder).build(); return clusterState; } @@ -832,7 +832,7 @@ public void testWRR() throws Exception { ClusterServiceUtils.setState(clusterService, builder); opRouting.setClusterService(clusterService); - opRouting.setWrrShardsCache(new WRRShardsCache(clusterService)); + opRouting.setWeightedRoutingCache(new WeightedRoutingCache(clusterService)); // search shards call GroupShardsIterator groupIterator = opRouting.searchShards( @@ -869,7 +869,7 @@ public void testWRR() throws Exception { opRouting = new OperationRouting(setting, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)); opRouting.setClusterService(clusterService); - opRouting.setWrrShardsCache(new WRRShardsCache(clusterService)); + opRouting.setWeightedRoutingCache(new WeightedRoutingCache(clusterService)); // search shards call groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); @@ -911,7 +911,7 @@ public void testWRRShardsCaching() throws Exception { new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) ); opRouting.setClusterService(clusterService); - opRouting.setWrrShardsCache(new WRRShardsCache(clusterService)); + opRouting.setWeightedRoutingCache(new WeightedRoutingCache(clusterService)); assertTrue(opRouting.ignoreAwarenessAttributes()); ResponseCollectorService collector = new ResponseCollectorService(clusterService); @@ -933,19 +933,19 @@ public void testWRRShardsCaching() throws Exception { ); // shard wrr ordering details are not present in cache, the details are calculated and put in cache - assertEquals(6, opRouting.getWrrShardsCache().misses()); - assertEquals(6, opRouting.getWrrShardsCache().size()); + assertEquals(6, opRouting.getWeightedRoutingCache().misses()); + assertEquals(6, opRouting.getWeightedRoutingCache().size()); // Calling operation routing again without any cluster state change to test that wrr shard routing details are // fetched from cache groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); // details are fetched from cache - assertEquals(12, opRouting.getWrrShardsCache().hits()); + assertEquals(12, opRouting.getWeightedRoutingCache().hits()); // cache count stays same - assertEquals(6, opRouting.getWrrShardsCache().size()); + assertEquals(6, opRouting.getWeightedRoutingCache().size()); // cache misses stay same - assertEquals(6, opRouting.getWrrShardsCache().misses()); + assertEquals(6, opRouting.getWeightedRoutingCache().misses()); // Updating cluster state to test wrr shard routing details are calculated again weights = Map.of("a", "1", "b", "0", "c", "1"); @@ -954,19 +954,19 @@ public void testWRRShardsCaching() throws Exception { ClusterServiceUtils.setState(clusterService, ClusterState.builder(state2)); ClusterChangedEvent event = new ClusterChangedEvent("test", state2, state); - opRouting.getWrrShardsCache().clusterChanged(event); + opRouting.getWeightedRoutingCache().clusterChanged(event); // cache is invalidated after cluster state change, cache count is zero - assertEquals(0, opRouting.getWrrShardsCache().size()); + assertEquals(0, opRouting.getWeightedRoutingCache().size()); // search shards call groupIterator = opRouting.searchShards(state2, indexNames, null, null, collector, outstandingRequests); // cache hit remain same - assertEquals(12, opRouting.getWrrShardsCache().hits()); + assertEquals(12, opRouting.getWeightedRoutingCache().hits()); // cache miss increases by 6 - assertEquals(12, opRouting.getWrrShardsCache().misses()); - assertEquals(6, opRouting.getWrrShardsCache().size()); + assertEquals(12, opRouting.getWeightedRoutingCache().misses()); + assertEquals(6, opRouting.getWeightedRoutingCache().size()); IOUtils.close(clusterService); terminate(threadPool); @@ -1003,7 +1003,7 @@ public void testWRRWithWeightUndefinedForOneZone() throws Exception { ClusterServiceUtils.setState(clusterService, ClusterState.builder(state)); opRouting.setClusterService(clusterService); - opRouting.setWrrShardsCache(new WRRShardsCache(clusterService)); + opRouting.setWeightedRoutingCache(new WeightedRoutingCache(clusterService)); // search shards call GroupShardsIterator groupIterator = opRouting.searchShards( state, @@ -1044,7 +1044,7 @@ public void testWRRWithWeightUndefinedForOneZone() throws Exception { opRouting = new OperationRouting(setting, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)); opRouting.setClusterService(clusterService); - opRouting.setWrrShardsCache(new WRRShardsCache(clusterService)); + opRouting.setWeightedRoutingCache(new WeightedRoutingCache(clusterService)); // search shards call groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); @@ -1236,9 +1236,9 @@ private ClusterState updateStatetoTestWRR( } // add wrr weights in metadata Map weights = Map.of("a", "1", "b", "1", "c", "0"); - WRRWeights wrrWeights = new WRRWeights("zone", weights); - WeightedRoundRobinRoutingMetadata wrrMetadata = new WeightedRoundRobinRoutingMetadata(wrrWeights); - metadataBuilder.putCustom(WeightedRoundRobinRoutingMetadata.TYPE, wrrMetadata); + WeightedRouting weightedRouting = new WeightedRouting("zone", weights); + WeightedRoutingMetadata wrrMetadata = new WeightedRoutingMetadata(weightedRouting); + metadataBuilder.putCustom(WeightedRoutingMetadata.TYPE, wrrMetadata); clusterState.metadata(metadataBuilder); clusterState.routingTable(routingTableBuilder.build()); return clusterState.build(); diff --git a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoundRobinTests.java b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoundRobinTests.java index 398a36fba83e5..81e7992410940 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoundRobinTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoundRobinTests.java @@ -16,22 +16,100 @@ public class WeightedRoundRobinTests extends OpenSearchTestCase { - public void testWRROrder() { - + public void testWeightedRoundRobinOrder() { + // weights set as A:4, B:3, C:2 List> entity = new ArrayList>(); entity.add(new WeightedRoundRobin.Entity<>(4, "A")); entity.add(new WeightedRoundRobin.Entity<>(3, "B")); entity.add(new WeightedRoundRobin.Entity<>(2, "C")); - WeightedRoundRobin wrr = new WeightedRoundRobin(entity); - List> orderedEntities = wrr.orderEntities(); - + WeightedRoundRobin weightedRoundRobin = new WeightedRoundRobin(entity); + List> orderedEntities = weightedRoundRobin.orderEntities(); List expectedOrdering = Arrays.asList("A", "A", "B", "A", "B", "C", "A", "B", "C"); List actualOrdering = new ArrayList<>(); for (WeightedRoundRobin.Entity en : orderedEntities) { actualOrdering.add(en.getTarget()); } + assertEquals(expectedOrdering, actualOrdering); + + // weights set as A:1, B:1, C:0 + entity = new ArrayList>(); + entity.add(new WeightedRoundRobin.Entity<>(1, "A")); + entity.add(new WeightedRoundRobin.Entity<>(1, "B")); + entity.add(new WeightedRoundRobin.Entity<>(0, "C")); + weightedRoundRobin = new WeightedRoundRobin(entity); + orderedEntities = weightedRoundRobin.orderEntities(); + expectedOrdering = Arrays.asList("A", "B"); + actualOrdering = new ArrayList<>(); + for (WeightedRoundRobin.Entity en : orderedEntities) { + actualOrdering.add(en.getTarget()); + } + assertEquals(expectedOrdering, actualOrdering); + + // weights set as A:0, B:0, C:0 + entity = new ArrayList>(); + entity.add(new WeightedRoundRobin.Entity<>(0, "A")); + entity.add(new WeightedRoundRobin.Entity<>(0, "B")); + entity.add(new WeightedRoundRobin.Entity<>(0, "C")); + weightedRoundRobin = new WeightedRoundRobin(entity); + orderedEntities = weightedRoundRobin.orderEntities(); + expectedOrdering = Arrays.asList(); + actualOrdering = new ArrayList<>(); + for (WeightedRoundRobin.Entity en : orderedEntities) { + actualOrdering.add(en.getTarget()); + } + assertEquals(expectedOrdering, actualOrdering); + + // weights set as A:-1, B:0, C:1 + entity = new ArrayList>(); + entity.add(new WeightedRoundRobin.Entity<>(-1, "A")); + entity.add(new WeightedRoundRobin.Entity<>(0, "B")); + entity.add(new WeightedRoundRobin.Entity<>(1, "C")); + weightedRoundRobin = new WeightedRoundRobin(entity); + orderedEntities = weightedRoundRobin.orderEntities(); + expectedOrdering = Arrays.asList("C"); + actualOrdering = new ArrayList<>(); + for (WeightedRoundRobin.Entity en : orderedEntities) { + actualOrdering.add(en.getTarget()); + } + assertEquals(expectedOrdering, actualOrdering); + // weights set as A:-1, B:3, C:0, D:10 + entity = new ArrayList>(); + entity.add(new WeightedRoundRobin.Entity<>(-1, "A")); + entity.add(new WeightedRoundRobin.Entity<>(3, "B")); + entity.add(new WeightedRoundRobin.Entity<>(0, "C")); + entity.add(new WeightedRoundRobin.Entity<>(10, "D")); + weightedRoundRobin = new WeightedRoundRobin(entity); + orderedEntities = weightedRoundRobin.orderEntities(); + expectedOrdering = Arrays.asList("B", "D", "B", "D", "B", "D", "D", "D", "D", "D", "D", "D", "D"); + actualOrdering = new ArrayList<>(); + for (WeightedRoundRobin.Entity en : orderedEntities) { + actualOrdering.add(en.getTarget()); + } assertEquals(expectedOrdering, actualOrdering); + + // weights set as A:-1, B:3, C:0, D:10000 + entity = new ArrayList>(); + entity.add(new WeightedRoundRobin.Entity<>(-1, "A")); + entity.add(new WeightedRoundRobin.Entity<>(3, "B")); + entity.add(new WeightedRoundRobin.Entity<>(0, "C")); + entity.add(new WeightedRoundRobin.Entity<>(10000, "D")); + weightedRoundRobin = new WeightedRoundRobin(entity); + orderedEntities = weightedRoundRobin.orderEntities(); + assertEquals(10003, orderedEntities.size()); + // Count of D's + int countD = 0; + // Count of B's + int countB = 0; + for (WeightedRoundRobin.Entity en : orderedEntities) { + if (en.getTarget().equals("D")) { + countD++; + } else if (en.getTarget().equals("B")) { + countB++; + } + } + assertEquals(3, countB); + assertEquals(10000, countD); } } From 93e7587bc031c10d1b43835ac718e179f2565af2 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Tue, 13 Sep 2022 21:33:53 +0530 Subject: [PATCH 15/31] Add test for Weighted routing iterator and some refactoring changes Signed-off-by: Anshu Agarwal --- .../routing/IndexShardRoutingTable.java | 12 +- .../cluster/routing/WeightedRoutingCache.java | 5 +- .../WeightedRoutingMetadataTests.java | 5 +- .../routing/OperationRoutingTests.java | 451 +++++++++--------- .../structure/RoutingIteratorTests.java | 92 ++++ 5 files changed, 337 insertions(+), 228 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java index e2352dd6ce6f3..c9f4c8a69e490 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java +++ b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java @@ -59,7 +59,6 @@ import java.util.function.Predicate; import static java.util.Collections.emptyMap; -import static java.util.stream.Collectors.toList; /** * {@link IndexShardRoutingTable} encapsulates all instances of a single shard. @@ -308,7 +307,7 @@ public ShardIterator activeInitializingShardsWeightedIt( WeightedRoutingCache cache ) { final int seed = shuffler.nextSeed(); - List ordered = new ArrayList<>(activeShards.size() + allInitializingShards.size()); + List ordered = new ArrayList<>(); List orderedActiveShards; if (cache.get(new WeightedRoutingCache.Key(shardId)) != null) { orderedActiveShards = cache.get(new WeightedRoutingCache.Key(shardId)); @@ -332,7 +331,14 @@ private List shardsOrderedByWeight(List shards, Weig WeightedRoundRobin weightedRoundRobin = new WeightedRoundRobin<>( calculateShardWeight(shards, weightedRouting, nodes) ); - return weightedRoundRobin.orderEntities().stream().map(WeightedRoundRobin.Entity::getTarget).collect(toList()); + List> shardsOrderedbyWeight = weightedRoundRobin.orderEntities(); + List orderedShardRouting = new ArrayList<>(activeShards.size()); + if (shardsOrderedbyWeight != null) { + for (WeightedRoundRobin.Entity shardRouting : shardsOrderedbyWeight) { + orderedShardRouting.add(shardRouting.getTarget()); + } + } + return orderedShardRouting; } /** diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingCache.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingCache.java index a525f26e0176e..ee4fef0260a8a 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingCache.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingCache.java @@ -21,12 +21,11 @@ import java.util.List; /** - * The WRR shards cache allows caching shard ordering returned by Weighted round-robin scheduling policy ,helping with - * improving similar requests. + * The weighted shard routing cache allows caching shard routing iterator returned by Weighted round-robin scheduling + * policy, helping with improving similar requests. * * @opensearch.internal */ - public class WeightedRoutingCache implements Releasable, ClusterStateListener { private static final Logger logger = LogManager.getLogger(WeightedRoutingCache.class); diff --git a/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoutingMetadataTests.java b/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoutingMetadataTests.java index c1e4c380db9ac..01a55f7f4fa4a 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoutingMetadataTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoutingMetadataTests.java @@ -20,8 +20,8 @@ public class WeightedRoutingMetadataTests extends AbstractXContentTestCase weights = Map.of("a", "1", "b", "1", "c", "0"); WeightedRouting weightedRouting = new WeightedRouting("zone", weights); - WeightedRoutingMetadata wrrMetadata = new WeightedRoutingMetadata(weightedRouting); - return wrrMetadata; + WeightedRoutingMetadata weightedRoutingMetadata = new WeightedRoutingMetadata(weightedRouting); + return weightedRoutingMetadata; } @Override @@ -33,5 +33,4 @@ protected WeightedRoutingMetadata doParseInstance(XContentParser parser) throws protected boolean supportsUnknownFields() { return false; } - } diff --git a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java index 2f9fe3cd991ac..c9c3a929d2b97 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java @@ -762,8 +762,8 @@ public void testAdaptiveReplicaSelectionWithZoneAwarenessIgnored() throws Except terminate(threadPool); } - private ClusterState clusterStateForWRR(String[] indexNames, int numShards, int numReplicas) { - DiscoveryNode[] allNodes = setUpNodesWRR(); + private ClusterState clusterStateForWeightedRouting(String[] indexNames, int numShards, int numReplicas) { + DiscoveryNode[] allNodes = setUpNodesForWeightedRouting(); ClusterState state = ClusterStateCreationUtils.state(allNodes[0], allNodes[6], allNodes); Map> discoveryNodeMap = new HashMap<>(); @@ -783,22 +783,22 @@ private ClusterState clusterStateForWRR(String[] indexNames, int numShards, int discoveryNodeMap.put("c", nodesZoneC); // Updating cluster state with node, index and shard details - state = updateStatetoTestWRR(indexNames, numShards, numReplicas, state, discoveryNodeMap); + state = updateStatetoTestWeightedRouting(indexNames, numShards, numReplicas, state, discoveryNodeMap); return state; } - private ClusterState setWRRWeights(ClusterState clusterState, Map weights) { - WeightedRouting wrrWeight = new WeightedRouting("zone", weights); - WeightedRoutingMetadata wrrMetadata = new WeightedRoutingMetadata(wrrWeight); + private ClusterState setWeightedRoutingWeights(ClusterState clusterState, Map weights) { + WeightedRouting weightedRouting = new WeightedRouting("zone", weights); + WeightedRoutingMetadata weightedRoutingMetadata = new WeightedRoutingMetadata(weightedRouting); Metadata.Builder metadataBuilder = Metadata.builder(clusterState.metadata()); - metadataBuilder.putCustom(WeightedRoutingMetadata.TYPE, wrrMetadata); + metadataBuilder.putCustom(WeightedRoutingMetadata.TYPE, weightedRoutingMetadata); clusterState = ClusterState.builder(clusterState).metadata(metadataBuilder).build(); return clusterState; } - public void testWRR() throws Exception { + public void testWeightedOperationRouting() throws Exception { final int numIndices = 2; final int numShards = 3; final int numReplicas = 2; @@ -807,89 +807,93 @@ public void testWRR() throws Exception { for (int i = 0; i < numIndices; i++) { indexNames[i] = "test" + i; } + ClusterService clusterService = null; + TestThreadPool threadPool = null; + try { + ClusterState state = clusterStateForWeightedRouting(indexNames, numShards, numReplicas); - ClusterState state = clusterStateForWRR(indexNames, numShards, numReplicas); - - Settings setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); - - TestThreadPool threadPool = new TestThreadPool("testThatOnlyNodesSupport"); - ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool); - - OperationRouting opRouting = new OperationRouting( - setting, - new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) - ); - assertTrue(opRouting.ignoreAwarenessAttributes()); - Set selectedNodes = new HashSet<>(); - ResponseCollectorService collector = new ResponseCollectorService(clusterService); - Map outstandingRequests = new HashMap<>(); - - // Setting up weights for weighted round-robin in cluster state - Map weights = Map.of("a", "1", "b", "1", "c", "0"); - state = setWRRWeights(state, weights); - - ClusterState.Builder builder = ClusterState.builder(state); - ClusterServiceUtils.setState(clusterService, builder); + Settings setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); - opRouting.setClusterService(clusterService); - opRouting.setWeightedRoutingCache(new WeightedRoutingCache(clusterService)); + threadPool = new TestThreadPool("testThatOnlyNodesSupport"); + clusterService = ClusterServiceUtils.createClusterService(threadPool); - // search shards call - GroupShardsIterator groupIterator = opRouting.searchShards( - state, - indexNames, - null, - null, - collector, - outstandingRequests + OperationRouting opRouting = new OperationRouting( + setting, + new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); + assertTrue(opRouting.ignoreAwarenessAttributes()); + Set selectedNodes = new HashSet<>(); + ResponseCollectorService collector = new ResponseCollectorService(clusterService); + Map outstandingRequests = new HashMap<>(); + + // Setting up weights for weighted round-robin in cluster state + Map weights = Map.of("a", "1", "b", "1", "c", "0"); + state = setWeightedRoutingWeights(state, weights); + + ClusterState.Builder builder = ClusterState.builder(state); + ClusterServiceUtils.setState(clusterService, builder); + + opRouting.setClusterService(clusterService); + opRouting.setWeightedRoutingCache(new WeightedRoutingCache(clusterService)); + + // search shards call + GroupShardsIterator groupIterator = opRouting.searchShards( + state, + indexNames, + null, + null, + collector, + outstandingRequests - ); + ); - for (ShardIterator it : groupIterator) { - List shardRoutings = Collections.singletonList(it.nextOrNull()); - for (ShardRouting shardRouting : shardRoutings) { - selectedNodes.add(shardRouting.currentNodeId()); + for (ShardIterator it : groupIterator) { + List shardRoutings = Collections.singletonList(it.nextOrNull()); + for (ShardRouting shardRouting : shardRoutings) { + selectedNodes.add(shardRouting.currentNodeId()); + } + } + // tests no nodes are assigned to nodes in zone c + for (String nodeID : selectedNodes) { + // No shards are assigned to nodes in zone c since its weight is 0 + assertFalse(nodeID.contains("c")); } - } - // tests no nodes are assigned to nodes in zone c - for (String nodeID : selectedNodes) { - // No shards are assigned to nodes in zone c since its weight is 0 - assertFalse(nodeID.contains("c")); - } - selectedNodes = new HashSet<>(); - setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); + selectedNodes = new HashSet<>(); + setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); - // Updating weighted round robin weights in cluster state - weights = Map.of("a", "1", "b", "0", "c", "1"); - state = setWRRWeights(state, weights); + // Updating weighted round robin weights in cluster state + weights = Map.of("a", "1", "b", "0", "c", "1"); + state = setWeightedRoutingWeights(state, weights); - builder = ClusterState.builder(state); - ClusterServiceUtils.setState(clusterService, builder); + builder = ClusterState.builder(state); + ClusterServiceUtils.setState(clusterService, builder); - opRouting = new OperationRouting(setting, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)); - opRouting.setClusterService(clusterService); - opRouting.setWeightedRoutingCache(new WeightedRoutingCache(clusterService)); + opRouting = new OperationRouting(setting, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)); + opRouting.setClusterService(clusterService); + opRouting.setWeightedRoutingCache(new WeightedRoutingCache(clusterService)); - // search shards call - groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); + // search shards call + groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); - for (ShardIterator it : groupIterator) { - List shardRoutings = Collections.singletonList(it.nextOrNull()); - for (ShardRouting shardRouting : shardRoutings) { - selectedNodes.add(shardRouting.currentNodeId()); + for (ShardIterator it : groupIterator) { + List shardRoutings = Collections.singletonList(it.nextOrNull()); + for (ShardRouting shardRouting : shardRoutings) { + selectedNodes.add(shardRouting.currentNodeId()); + } } + // tests that no shards are assigned to zone with weight zero + for (String nodeID : selectedNodes) { + // No shards are assigned to nodes in zone b since its weight is 0 + assertFalse(nodeID.contains("b")); + } + } finally { + IOUtils.close(clusterService); + terminate(threadPool); } - // tests that no shards are assigned to zone with weight zero - for (String nodeID : selectedNodes) { - // No shards are assigned to nodes in zone b since its weight is 0 - assertFalse(nodeID.contains("b")); - } - IOUtils.close(clusterService); - terminate(threadPool); } - public void testWRRShardsCaching() throws Exception { + public void testWeightedOperationRoutingCaching() throws Exception { final int numIndices = 2; final int numShards = 3; final int numReplicas = 2; @@ -898,81 +902,84 @@ public void testWRRShardsCaching() throws Exception { for (int i = 0; i < numIndices; i++) { indexNames[i] = "test" + i; } + ClusterService clusterService = null; + TestThreadPool threadPool = null; + try { + ClusterState state = clusterStateForWeightedRouting(indexNames, numShards, numReplicas); - ClusterState state = clusterStateForWRR(indexNames, numShards, numReplicas); - - Settings setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); - - TestThreadPool threadPool = new TestThreadPool("testThatOnlyNodesSupport"); - ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool); - - OperationRouting opRouting = new OperationRouting( - setting, - new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) - ); - opRouting.setClusterService(clusterService); - opRouting.setWeightedRoutingCache(new WeightedRoutingCache(clusterService)); - - assertTrue(opRouting.ignoreAwarenessAttributes()); - ResponseCollectorService collector = new ResponseCollectorService(clusterService); - Map outstandingRequests = new HashMap<>(); - - // Setting up weights for weighted round-robin in cluster state - Map weights = Map.of("a", "1", "b", "1", "c", "0"); - state = setWRRWeights(state, weights); - ClusterServiceUtils.setState(clusterService, ClusterState.builder(state)); - - // search shards call - GroupShardsIterator groupIterator = opRouting.searchShards( - state, - indexNames, - null, - null, - collector, - outstandingRequests - ); - - // shard wrr ordering details are not present in cache, the details are calculated and put in cache - assertEquals(6, opRouting.getWeightedRoutingCache().misses()); - assertEquals(6, opRouting.getWeightedRoutingCache().size()); - - // Calling operation routing again without any cluster state change to test that wrr shard routing details are - // fetched from cache - groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); - - // details are fetched from cache - assertEquals(12, opRouting.getWeightedRoutingCache().hits()); - // cache count stays same - assertEquals(6, opRouting.getWeightedRoutingCache().size()); - // cache misses stay same - assertEquals(6, opRouting.getWeightedRoutingCache().misses()); - - // Updating cluster state to test wrr shard routing details are calculated again - weights = Map.of("a", "1", "b", "0", "c", "1"); - // building new cluster state - ClusterState state2 = setWRRWeights(state, weights); - ClusterServiceUtils.setState(clusterService, ClusterState.builder(state2)); - - ClusterChangedEvent event = new ClusterChangedEvent("test", state2, state); - opRouting.getWeightedRoutingCache().clusterChanged(event); - - // cache is invalidated after cluster state change, cache count is zero - assertEquals(0, opRouting.getWeightedRoutingCache().size()); + Settings setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); - // search shards call - groupIterator = opRouting.searchShards(state2, indexNames, null, null, collector, outstandingRequests); + threadPool = new TestThreadPool("testThatOnlyNodesSupport"); + clusterService = ClusterServiceUtils.createClusterService(threadPool); - // cache hit remain same - assertEquals(12, opRouting.getWeightedRoutingCache().hits()); - // cache miss increases by 6 - assertEquals(12, opRouting.getWeightedRoutingCache().misses()); - assertEquals(6, opRouting.getWeightedRoutingCache().size()); + OperationRouting opRouting = new OperationRouting( + setting, + new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); + opRouting.setClusterService(clusterService); + opRouting.setWeightedRoutingCache(new WeightedRoutingCache(clusterService)); + + assertTrue(opRouting.ignoreAwarenessAttributes()); + ResponseCollectorService collector = new ResponseCollectorService(clusterService); + Map outstandingRequests = new HashMap<>(); + + // Setting up weights for weighted round-robin in cluster state + Map weights = Map.of("a", "1", "b", "1", "c", "0"); + state = setWeightedRoutingWeights(state, weights); + ClusterServiceUtils.setState(clusterService, ClusterState.builder(state)); + + // search shards call + GroupShardsIterator groupIterator = opRouting.searchShards( + state, + indexNames, + null, + null, + collector, + outstandingRequests + ); - IOUtils.close(clusterService); - terminate(threadPool); + // shard weighted routing ordering details are not present in cache, the details are calculated and put in cache + assertEquals(6, opRouting.getWeightedRoutingCache().misses()); + assertEquals(6, opRouting.getWeightedRoutingCache().size()); + + // Calling operation routing again without any cluster state change to test that weighted routing details + // are fetched from cache + groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); + + // details are fetched from cache + assertEquals(12, opRouting.getWeightedRoutingCache().hits()); + // cache count stays same + assertEquals(6, opRouting.getWeightedRoutingCache().size()); + // cache misses stay same + assertEquals(6, opRouting.getWeightedRoutingCache().misses()); + + // Updating cluster state to test weighted routing details are calculated again + weights = Map.of("a", "1", "b", "0", "c", "1"); + // building new cluster state + ClusterState state2 = setWeightedRoutingWeights(state, weights); + ClusterServiceUtils.setState(clusterService, ClusterState.builder(state2)); + + ClusterChangedEvent event = new ClusterChangedEvent("test", state2, state); + opRouting.getWeightedRoutingCache().clusterChanged(event); + + // cache is invalidated after cluster state change, cache count is zero + assertEquals(0, opRouting.getWeightedRoutingCache().size()); + + // search shards call + groupIterator = opRouting.searchShards(state2, indexNames, null, null, collector, outstandingRequests); + + // cache hit remain same + assertEquals(12, opRouting.getWeightedRoutingCache().hits()); + // cache miss increases by 6 + assertEquals(12, opRouting.getWeightedRoutingCache().misses()); + assertEquals(6, opRouting.getWeightedRoutingCache().size()); + } finally { + IOUtils.close(clusterService); + terminate(threadPool); + } } - public void testWRRWithWeightUndefinedForOneZone() throws Exception { + public void testWeightedOperationRoutingWeightUndefinedForOneZone() throws Exception { final int numIndices = 2; final int numShards = 3; final int numReplicas = 2; @@ -981,93 +988,99 @@ public void testWRRWithWeightUndefinedForOneZone() throws Exception { for (int i = 0; i < numIndices; i++) { indexNames[i] = "test" + i; } - ClusterState state = clusterStateForWRR(indexNames, numShards, numReplicas); - - Settings setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); - TestThreadPool threadPool = new TestThreadPool("testThatOnlyNodesSupport"); - ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool); + ClusterService clusterService = null; + TestThreadPool threadPool = null; + try { + ClusterState state = clusterStateForWeightedRouting(indexNames, numShards, numReplicas); - OperationRouting opRouting = new OperationRouting( - setting, - new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) - ); - assertTrue(opRouting.ignoreAwarenessAttributes()); - Set selectedNodes = new HashSet<>(); - ResponseCollectorService collector = new ResponseCollectorService(clusterService); - Map outstandingRequests = new HashMap<>(); + Settings setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); - // Setting up weights for weighted round-robin in cluster state, weight for nodes in zone b is not set - Map weights = Map.of("a", "1", "c", "0"); - state = setWRRWeights(state, weights); - ClusterServiceUtils.setState(clusterService, ClusterState.builder(state)); + threadPool = new TestThreadPool("testThatOnlyNodesSupport"); + clusterService = ClusterServiceUtils.createClusterService(threadPool); - opRouting.setClusterService(clusterService); - opRouting.setWeightedRoutingCache(new WeightedRoutingCache(clusterService)); - // search shards call - GroupShardsIterator groupIterator = opRouting.searchShards( - state, - indexNames, - null, - null, - collector, - outstandingRequests + OperationRouting opRouting = new OperationRouting( + setting, + new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); + assertTrue(opRouting.ignoreAwarenessAttributes()); + Set selectedNodes = new HashSet<>(); + ResponseCollectorService collector = new ResponseCollectorService(clusterService); + Map outstandingRequests = new HashMap<>(); + + // Setting up weights for weighted round-robin in cluster state, weight for nodes in zone b is not set + Map weights = Map.of("a", "1", "c", "0"); + state = setWeightedRoutingWeights(state, weights); + ClusterServiceUtils.setState(clusterService, ClusterState.builder(state)); + + opRouting.setClusterService(clusterService); + opRouting.setWeightedRoutingCache(new WeightedRoutingCache(clusterService)); + // search shards call + GroupShardsIterator groupIterator = opRouting.searchShards( + state, + indexNames, + null, + null, + collector, + outstandingRequests - ); + ); - for (ShardIterator it : groupIterator) { - List shardRoutings = Collections.singletonList(it.nextOrNull()); - for (ShardRouting shardRouting : shardRoutings) { - selectedNodes.add(shardRouting.currentNodeId()); + for (ShardIterator it : groupIterator) { + List shardRoutings = Collections.singletonList(it.nextOrNull()); + for (ShardRouting shardRouting : shardRoutings) { + selectedNodes.add(shardRouting.currentNodeId()); + } } - } - boolean weighAwayNodesInUndefinedZone = true; - // tests no shards are assigned to nodes in zone c - // tests shards are assigned to nodes in zone b - for (String nodeID : selectedNodes) { - // shard from nodes in zone c is not selected since its weight is 0 - assertFalse(nodeID.contains("c")); - if (nodeID.contains("b")) { - weighAwayNodesInUndefinedZone = false; + boolean weighAwayNodesInUndefinedZone = true; + // tests no shards are assigned to nodes in zone c + // tests shards are assigned to nodes in zone b + for (String nodeID : selectedNodes) { + // shard from nodes in zone c is not selected since its weight is 0 + assertFalse(nodeID.contains("c")); + if (nodeID.contains("b")) { + weighAwayNodesInUndefinedZone = false; + } } - } - assertFalse(weighAwayNodesInUndefinedZone); + assertFalse(weighAwayNodesInUndefinedZone); - selectedNodes = new HashSet<>(); - setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); + selectedNodes = new HashSet<>(); + setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); - // Updating weighted round robin weights in cluster state - weights = Map.of("a", "0", "b", "1"); + // Updating weighted round robin weights in cluster state + weights = Map.of("a", "0", "b", "1"); - state = setWRRWeights(state, weights); - ClusterServiceUtils.setState(clusterService, ClusterState.builder(state)); + state = setWeightedRoutingWeights(state, weights); + ClusterServiceUtils.setState(clusterService, ClusterState.builder(state)); - opRouting = new OperationRouting(setting, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)); - opRouting.setClusterService(clusterService); - opRouting.setWeightedRoutingCache(new WeightedRoutingCache(clusterService)); + opRouting = new OperationRouting(setting, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)); + opRouting.setClusterService(clusterService); + opRouting.setWeightedRoutingCache(new WeightedRoutingCache(clusterService)); - // search shards call - groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); + // search shards call + groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); - for (ShardIterator it : groupIterator) { - List shardRoutings = Collections.singletonList(it.nextOrNull()); - for (ShardRouting shardRouting : shardRoutings) { - selectedNodes.add(shardRouting.currentNodeId()); + for (ShardIterator it : groupIterator) { + List shardRoutings = Collections.singletonList(it.nextOrNull()); + for (ShardRouting shardRouting : shardRoutings) { + selectedNodes.add(shardRouting.currentNodeId()); + } } - } - // tests that no shards are assigned to zone with weight zero - // tests shards are assigned to nodes in zone c - weighAwayNodesInUndefinedZone = true; - for (String nodeID : selectedNodes) { - // shard from nodes in zone a is not selected since its weight is 0 - assertFalse(nodeID.contains("a")); - if (nodeID.contains("c")) { - weighAwayNodesInUndefinedZone = false; + // tests that no shards are assigned to zone with weight zero + // tests shards are assigned to nodes in zone c + weighAwayNodesInUndefinedZone = true; + for (String nodeID : selectedNodes) { + // shard from nodes in zone a is not selected since its weight is 0 + assertFalse(nodeID.contains("a")); + if (nodeID.contains("c")) { + weighAwayNodesInUndefinedZone = false; + } } + assertFalse(weighAwayNodesInUndefinedZone); + } finally { + IOUtils.close(clusterService); + terminate(threadPool); } - assertFalse(weighAwayNodesInUndefinedZone); - IOUtils.close(clusterService); - terminate(threadPool); } private DiscoveryNode[] setupNodes() { @@ -1096,7 +1109,7 @@ private DiscoveryNode[] setupNodes() { return allNodes; } - private DiscoveryNode[] setUpNodesWRR() { + private DiscoveryNode[] setUpNodesForWeightedRouting() { List zones = Arrays.asList("a", "a", "b", "b", "c", "c"); DiscoveryNode[] allNodes = new DiscoveryNode[7]; int i = 0; @@ -1179,7 +1192,7 @@ private ClusterState updateStatetoTestARS( return clusterState.build(); } - private ClusterState updateStatetoTestWRR( + private ClusterState updateStatetoTestWeightedRouting( String[] indices, int numberOfShards, int numberOfReplicas, @@ -1234,11 +1247,11 @@ private ClusterState updateStatetoTestWRR( } routingTableBuilder.add(indexRoutingTableBuilder.build()); } - // add wrr weights in metadata + // add weighted routing weights in metadata Map weights = Map.of("a", "1", "b", "1", "c", "0"); WeightedRouting weightedRouting = new WeightedRouting("zone", weights); - WeightedRoutingMetadata wrrMetadata = new WeightedRoutingMetadata(weightedRouting); - metadataBuilder.putCustom(WeightedRoutingMetadata.TYPE, wrrMetadata); + WeightedRoutingMetadata weightedRoutingMetadata = new WeightedRoutingMetadata(weightedRouting); + metadataBuilder.putCustom(WeightedRoutingMetadata.TYPE, weightedRoutingMetadata); clusterState.metadata(metadataBuilder); clusterState.routingTable(routingTableBuilder.build()); return clusterState.build(); diff --git a/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java b/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java index 223c9531dfe1c..5ff229682cd5e 100644 --- a/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java +++ b/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java @@ -48,6 +48,8 @@ import org.opensearch.cluster.routing.ShardRouting; import org.opensearch.cluster.routing.ShardShuffler; import org.opensearch.cluster.routing.ShardsIterator; +import org.opensearch.cluster.routing.WeightedRouting; +import org.opensearch.cluster.routing.WeightedRoutingCache; import org.opensearch.cluster.routing.allocation.AllocationService; import org.opensearch.cluster.routing.allocation.decider.ClusterRebalanceAllocationDecider; import org.opensearch.cluster.service.ClusterService; @@ -504,4 +506,94 @@ public void testReplicaShardPreferenceIters() throws Exception { } } + public void testWeightedRouting() { + TestThreadPool threadPool = null; + try { + Settings.Builder settings = Settings.builder() + .put("cluster.routing.allocation.node_concurrent_recoveries", 10) + .put("cluster.routing.allocation.awareness.attributes", "zone"); + AllocationService strategy = createAllocationService(settings.build()); + + Metadata metadata = Metadata.builder() + .put(IndexMetadata.builder("test").settings(settings(Version.CURRENT)).numberOfShards(1).numberOfReplicas(2)) + .build(); + + RoutingTable routingTable = RoutingTable.builder().addAsNew(metadata.index("test")).build(); + + ClusterState clusterState = ClusterState.builder(ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)) + .metadata(metadata) + .routingTable(routingTable) + .build(); + + threadPool = new TestThreadPool("testThatOnlyNodesSupport"); + ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool); + + Map node1Attributes = new HashMap<>(); + node1Attributes.put("zone", "zone1"); + Map node2Attributes = new HashMap<>(); + node2Attributes.put("zone", "zone2"); + Map node3Attributes = new HashMap<>(); + node3Attributes.put("zone", "zone3"); + clusterState = ClusterState.builder(clusterState) + .nodes( + DiscoveryNodes.builder() + .add(newNode("node1", unmodifiableMap(node1Attributes))) + .add(newNode("node2", unmodifiableMap(node2Attributes))) + .add(newNode("node3", unmodifiableMap(node3Attributes))) + .localNodeId("node1") + ) + .build(); + clusterState = strategy.reroute(clusterState, "reroute"); + + clusterState = startInitializingShardsAndReroute(strategy, clusterState); + clusterState = startInitializingShardsAndReroute(strategy, clusterState); + + WeightedRoutingCache cache = new WeightedRoutingCache(clusterService); + Map weights = Map.of("zone1", "1", "zone2", "1", "zone3", "0"); + WeightedRouting weightedRouting = new WeightedRouting("zone", weights); + + ShardIterator shardIterator = clusterState.routingTable() + .index("test") + .shard(0) + .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), cache); + + assertEquals(2, shardIterator.size()); + ShardRouting shardRouting = shardIterator.nextOrNull(); + assertNotNull(shardRouting); + assertFalse(shardRouting.currentNodeId().equals("node3")); + shardRouting = shardIterator.nextOrNull(); + assertNotNull(shardRouting); + assertFalse(shardRouting.currentNodeId().equals("node3")); + cache.close(); + + weights = Map.of("zone1", "1", "zone2", "1", "zone3", "1"); + weightedRouting = new WeightedRouting("zone", weights); + shardIterator = clusterState.routingTable() + .index("test") + .shard(0) + .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), cache); + assertEquals(3, shardIterator.size()); + cache.close(); + + weights = Map.of("zone1", "-1", "zone2", "0", "zone3", "1"); + weightedRouting = new WeightedRouting("zone", weights); + shardIterator = clusterState.routingTable() + .index("test") + .shard(0) + .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), cache); + assertEquals(1, shardIterator.size()); + cache.close(); + + weights = Map.of("zone1", "0", "zone2", "0", "zone3", "0"); + weightedRouting = new WeightedRouting("zone", weights); + shardIterator = clusterState.routingTable() + .index("test") + .shard(0) + .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), cache); + assertEquals(0, shardIterator.size()); + cache.close(); + } finally { + terminate(threadPool); + } + } } From 17dc58c59f5d69eb6a22ae63896a50eb998f1201 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Tue, 13 Sep 2022 21:57:12 +0530 Subject: [PATCH 16/31] Update metadata minimal supported version Signed-off-by: Anshu Agarwal --- .../opensearch/cluster/metadata/WeightedRoutingMetadata.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java index 9c9b8bf38b4a7..9c85fdfcc1ed2 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java @@ -69,7 +69,7 @@ public String getWriteableName() { @Override public Version getMinimalSupportedVersion() { - return Version.V_2_3_0; + return Version.V_2_4_0; } @Override From ed0cc1bc24f6ec936acf4db7f62a4b90884da04c Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Wed, 14 Sep 2022 08:11:40 +0530 Subject: [PATCH 17/31] Add cluster setting for default weight Signed-off-by: Anshu Agarwal --- CHANGELOG.md | 1 - .../routing/IndexShardRoutingTable.java | 26 ++++++++++++++----- .../cluster/routing/OperationRouting.java | 25 +++++++++++++++++- .../common/settings/ClusterSettings.java | 1 + .../structure/RoutingIteratorTests.java | 8 +++--- 5 files changed, 48 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75c3d8d407a10..cc898f1ccf834 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,7 +32,6 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Use RemoteSegmentStoreDirectory instead of RemoteDirectory ([#4240](https://github.com/opensearch-project/OpenSearch/pull/4240)) - Plugin ZIP publication groupId value is configurable ([#4156](https://github.com/opensearch-project/OpenSearch/pull/4156)) - Weighted round-robin scheduling policy for shard coordination traffic ([#4241](https://github.com/opensearch-project/OpenSearch/pull/4241)) -- Update to Netty 4.1.80.Final ([#4359](https://github.com/opensearch-project/OpenSearch/pull/4359)) - Add index specific setting for remote repository ([#4253](https://github.com/opensearch-project/OpenSearch/pull/4253)) - [Segment Replication] Update replicas to commit SegmentInfos instead of relying on SIS files from primary shards. ([#4402](https://github.com/opensearch-project/OpenSearch/pull/4402)) - [CCR] Add getHistoryOperationsFromTranslog method to fetch the history snapshot from translogs ([#3948](https://github.com/opensearch-project/OpenSearch/pull/3948)) diff --git a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java index c9f4c8a69e490..b6ca69d75a90d 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java +++ b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java @@ -304,7 +304,8 @@ public ShardIterator activeInitializingShardsRankedIt( public ShardIterator activeInitializingShardsWeightedIt( WeightedRouting weightedRouting, DiscoveryNodes nodes, - WeightedRoutingCache cache + WeightedRoutingCache cache, + int defaultWeight ) { final int seed = shuffler.nextSeed(); List ordered = new ArrayList<>(); @@ -312,13 +313,18 @@ public ShardIterator activeInitializingShardsWeightedIt( if (cache.get(new WeightedRoutingCache.Key(shardId)) != null) { orderedActiveShards = cache.get(new WeightedRoutingCache.Key(shardId)); } else { - orderedActiveShards = shardsOrderedByWeight(activeShards, weightedRouting, nodes); + orderedActiveShards = shardsOrderedByWeight(activeShards, weightedRouting, nodes, defaultWeight); cache.put(new WeightedRoutingCache.Key(shardId), orderedActiveShards); } ordered.addAll(shuffler.shuffle(orderedActiveShards, seed)); if (!allInitializingShards.isEmpty()) { - List orderedInitializingShards = shardsOrderedByWeight(allInitializingShards, weightedRouting, nodes); + List orderedInitializingShards = shardsOrderedByWeight( + allInitializingShards, + weightedRouting, + nodes, + defaultWeight + ); ordered.addAll(orderedInitializingShards); } return new PlainShardIterator(shardId, ordered); @@ -327,9 +333,14 @@ public ShardIterator activeInitializingShardsWeightedIt( /** * Returns a list containing shard routings ordered using weighted round-robin scheduling. */ - private List shardsOrderedByWeight(List shards, WeightedRouting weightedRouting, DiscoveryNodes nodes) { + private List shardsOrderedByWeight( + List shards, + WeightedRouting weightedRouting, + DiscoveryNodes nodes, + int defaultWeight + ) { WeightedRoundRobin weightedRoundRobin = new WeightedRoundRobin<>( - calculateShardWeight(shards, weightedRouting, nodes) + calculateShardWeight(shards, weightedRouting, nodes, defaultWeight) ); List> shardsOrderedbyWeight = weightedRoundRobin.orderEntities(); List orderedShardRouting = new ArrayList<>(activeShards.size()); @@ -349,14 +360,15 @@ private List shardsOrderedByWeight(List shards, Weig private List> calculateShardWeight( List shards, WeightedRouting weightedRouting, - DiscoveryNodes nodes + DiscoveryNodes nodes, + int defaultWeight ) { List> shardsWithWeights = new ArrayList<>(); for (ShardRouting shard : shards) { DiscoveryNode node = nodes.get(shard.currentNodeId()); String attVal = node.getAttributes().get(weightedRouting.attributeName()); // If weight for a zone is not defined, considering it as 1 by default - Double weight = Double.parseDouble(weightedRouting.weights().getOrDefault(attVal, 1).toString()); + Double weight = Double.parseDouble(weightedRouting.weights().getOrDefault(attVal, defaultWeight).toString()); shardsWithWeights.add(new WeightedRoundRobin.Entity<>(weight, shard)); } return shardsWithWeights; diff --git a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java index a5acbed983cc3..9c99a871658dd 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java +++ b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java @@ -78,9 +78,17 @@ public class OperationRouting { Setting.Property.Dynamic, Setting.Property.NodeScope ); + public static final Setting WEIGHTED_ROUTING_DEFAULT_WEIGHT = Setting.intSetting( + "cluster.routing.weighted.default_weight", + 1, + 1, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); private volatile List awarenessAttributes; private volatile boolean useAdaptiveReplicaSelection; private volatile boolean ignoreAwarenessAttr; + private volatile int weightedRoutingDefaultWeight; private WeightedRoutingCache weightedRoutingCache; private ClusterService clusterService; @@ -94,8 +102,10 @@ public OperationRouting(Settings settings, ClusterSettings clusterSettings) { this::setAwarenessAttributes ); this.useAdaptiveReplicaSelection = USE_ADAPTIVE_REPLICA_SELECTION_SETTING.get(settings); + this.weightedRoutingDefaultWeight = WEIGHTED_ROUTING_DEFAULT_WEIGHT.get(settings); clusterSettings.addSettingsUpdateConsumer(USE_ADAPTIVE_REPLICA_SELECTION_SETTING, this::setUseAdaptiveReplicaSelection); clusterSettings.addSettingsUpdateConsumer(IGNORE_AWARENESS_ATTRIBUTES_SETTING, this::setIgnoreAwarenessAttributes); + clusterSettings.addSettingsUpdateConsumer(WEIGHTED_ROUTING_DEFAULT_WEIGHT, this::setWeightedRoutingDefaultWeight); } void setUseAdaptiveReplicaSelection(boolean useAdaptiveReplicaSelection) { @@ -106,6 +116,10 @@ void setIgnoreAwarenessAttributes(boolean ignoreAwarenessAttributes) { this.ignoreAwarenessAttr = ignoreAwarenessAttributes; } + void setWeightedRoutingDefaultWeight(int weightedRoutingDefaultWeight) { + this.weightedRoutingDefaultWeight = weightedRoutingDefaultWeight; + } + public boolean isIgnoreAwarenessAttr() { return ignoreAwarenessAttr; } @@ -122,6 +136,10 @@ public boolean ignoreAwarenessAttributes() { return this.awarenessAttributes.isEmpty() || this.ignoreAwarenessAttr; } + public int getWeightedRoutingDefaultWeight() { + return this.weightedRoutingDefaultWeight; + } + @Inject public void setClusterService(ClusterService clusterService) { this.clusterService = clusterService; @@ -322,7 +340,12 @@ private ShardIterator shardRoutings( ) { WeightedRoutingMetadata weightedRoutingMetadata = clusterService.state().metadata().weightedRoutingMetadata(); if (weightedRoutingMetadata != null) { - return indexShard.activeInitializingShardsWeightedIt(weightedRoutingMetadata.getWeightedRouting(), nodes, weightedRoutingCache); + return indexShard.activeInitializingShardsWeightedIt( + weightedRoutingMetadata.getWeightedRouting(), + nodes, + weightedRoutingCache, + getWeightedRoutingDefaultWeight() + ); } else if (ignoreAwarenessAttributes()) { if (useAdaptiveReplicaSelection) { return indexShard.activeInitializingShardsRankedIt(collectorService, nodeCounts); diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index 971fb518ff1da..1665614c18496 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -529,6 +529,7 @@ public void apply(Settings value, Settings current, Settings previous) { Node.BREAKER_TYPE_KEY, OperationRouting.USE_ADAPTIVE_REPLICA_SELECTION_SETTING, OperationRouting.IGNORE_AWARENESS_ATTRIBUTES_SETTING, + OperationRouting.WEIGHTED_ROUTING_DEFAULT_WEIGHT, IndexGraveyard.SETTING_MAX_TOMBSTONES, PersistentTasksClusterService.CLUSTER_TASKS_ALLOCATION_RECHECK_INTERVAL_SETTING, EnableAssignmentDecider.CLUSTER_TASKS_ALLOCATION_ENABLE_SETTING, diff --git a/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java b/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java index 5ff229682cd5e..c75480ab782ba 100644 --- a/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java +++ b/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java @@ -555,7 +555,7 @@ public void testWeightedRouting() { ShardIterator shardIterator = clusterState.routingTable() .index("test") .shard(0) - .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), cache); + .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), cache, 1); assertEquals(2, shardIterator.size()); ShardRouting shardRouting = shardIterator.nextOrNull(); @@ -571,7 +571,7 @@ public void testWeightedRouting() { shardIterator = clusterState.routingTable() .index("test") .shard(0) - .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), cache); + .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), cache, 1); assertEquals(3, shardIterator.size()); cache.close(); @@ -580,7 +580,7 @@ public void testWeightedRouting() { shardIterator = clusterState.routingTable() .index("test") .shard(0) - .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), cache); + .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), cache, 1); assertEquals(1, shardIterator.size()); cache.close(); @@ -589,7 +589,7 @@ public void testWeightedRouting() { shardIterator = clusterState.routingTable() .index("test") .shard(0) - .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), cache); + .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), cache, 1); assertEquals(0, shardIterator.size()); cache.close(); } finally { From 5a5cb11a9a450526945f658853b00f1b61001f8b Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Wed, 14 Sep 2022 08:37:30 +0530 Subject: [PATCH 18/31] Fix tests due to the change Signed-off-by: Anshu Agarwal --- .../opensearch/cluster/metadata/WeightedRoutingMetadata.java | 2 +- .../org/opensearch/cluster/routing/OperationRouting.java | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java index 9c85fdfcc1ed2..6ab2cf22adb02 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java @@ -28,7 +28,7 @@ import java.util.Map; /** - * Contains metadata for weighted round-robin shard routing weights + * Contains metadata for weighted routing * * @opensearch.internal */ diff --git a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java index 9c99a871658dd..8763784a76e2e 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java +++ b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java @@ -338,7 +338,10 @@ private ShardIterator shardRoutings( @Nullable ResponseCollectorService collectorService, @Nullable Map nodeCounts ) { - WeightedRoutingMetadata weightedRoutingMetadata = clusterService.state().metadata().weightedRoutingMetadata(); + WeightedRoutingMetadata weightedRoutingMetadata = null; + if (clusterService != null) { + weightedRoutingMetadata = clusterService.state().metadata().weightedRoutingMetadata(); + } if (weightedRoutingMetadata != null) { return indexShard.activeInitializingShardsWeightedIt( weightedRoutingMetadata.getWeightedRouting(), From 1c339648379fe26c210c475317e12fc2935b0312 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Wed, 14 Sep 2022 10:27:37 +0530 Subject: [PATCH 19/31] Fix cache concurrency issue Signed-off-by: Anshu Agarwal --- .../opensearch/cluster/routing/IndexShardRoutingTable.java | 7 +++---- .../opensearch/cluster/routing/OperationRoutingTests.java | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java index b6ca69d75a90d..a972ce4e61b5b 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java +++ b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java @@ -309,10 +309,9 @@ public ShardIterator activeInitializingShardsWeightedIt( ) { final int seed = shuffler.nextSeed(); List ordered = new ArrayList<>(); - List orderedActiveShards; - if (cache.get(new WeightedRoutingCache.Key(shardId)) != null) { - orderedActiveShards = cache.get(new WeightedRoutingCache.Key(shardId)); - } else { + List orderedActiveShards = cache.get(new WeightedRoutingCache.Key(shardId)); + if (orderedActiveShards == null) + { orderedActiveShards = shardsOrderedByWeight(activeShards, weightedRouting, nodes, defaultWeight); cache.put(new WeightedRoutingCache.Key(shardId), orderedActiveShards); } diff --git a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java index c9c3a929d2b97..6cad82e8ead6b 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java @@ -947,7 +947,7 @@ public void testWeightedOperationRoutingCaching() throws Exception { groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); // details are fetched from cache - assertEquals(12, opRouting.getWeightedRoutingCache().hits()); + assertEquals(6, opRouting.getWeightedRoutingCache().hits()); // cache count stays same assertEquals(6, opRouting.getWeightedRoutingCache().size()); // cache misses stay same @@ -969,7 +969,7 @@ public void testWeightedOperationRoutingCaching() throws Exception { groupIterator = opRouting.searchShards(state2, indexNames, null, null, collector, outstandingRequests); // cache hit remain same - assertEquals(12, opRouting.getWeightedRoutingCache().hits()); + assertEquals(6, opRouting.getWeightedRoutingCache().hits()); // cache miss increases by 6 assertEquals(12, opRouting.getWeightedRoutingCache().misses()); assertEquals(6, opRouting.getWeightedRoutingCache().size()); From f8638f210b009c1c35ad84d5563b3358e133f89a Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Wed, 14 Sep 2022 10:40:25 +0530 Subject: [PATCH 20/31] Spotless check fix Signed-off-by: Anshu Agarwal --- .../org/opensearch/cluster/routing/IndexShardRoutingTable.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java index a972ce4e61b5b..8727652d8611a 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java +++ b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java @@ -310,8 +310,7 @@ public ShardIterator activeInitializingShardsWeightedIt( final int seed = shuffler.nextSeed(); List ordered = new ArrayList<>(); List orderedActiveShards = cache.get(new WeightedRoutingCache.Key(shardId)); - if (orderedActiveShards == null) - { + if (orderedActiveShards == null) { orderedActiveShards = shardsOrderedByWeight(activeShards, weightedRouting, nodes, defaultWeight); cache.put(new WeightedRoutingCache.Key(shardId), orderedActiveShards); } From 4016d2759ad6314d731d684aa7c0cb7206da42b9 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Thu, 15 Sep 2022 12:18:47 +0530 Subject: [PATCH 21/31] Fix weighted round robin logic case when there is an entity with weight 0 Signed-off-by: Anshu Agarwal --- .../cluster/routing/WeightedRoundRobin.java | 3 -- .../routing/WeightedRoundRobinTests.java | 36 +++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java index 0484f8cd0d93e..15d437db9c8ff 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoundRobin.java @@ -36,9 +36,6 @@ public List> orderEntities() { if (size == 0) { return null; } - if (size == 1) { - return entities; - } // Find maximum weight and greatest common divisor of weight across all entities double maxWeight = 0; double sumWeight = 0; diff --git a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoundRobinTests.java b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoundRobinTests.java index 81e7992410940..5f62d30486e86 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoundRobinTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoundRobinTests.java @@ -110,6 +110,42 @@ public void testWeightedRoundRobinOrder() { } assertEquals(3, countB); assertEquals(10000, countD); + + // weights set C:0 + entity = new ArrayList>(); + entity.add(new WeightedRoundRobin.Entity<>(0, "C")); + weightedRoundRobin = new WeightedRoundRobin(entity); + orderedEntities = weightedRoundRobin.orderEntities(); + expectedOrdering = Arrays.asList(); + actualOrdering = new ArrayList<>(); + for (WeightedRoundRobin.Entity en : orderedEntities) { + actualOrdering.add(en.getTarget()); + } + assertEquals(expectedOrdering, actualOrdering); + + // weights set C:1 + entity = new ArrayList>(); + entity.add(new WeightedRoundRobin.Entity<>(1, "C")); + weightedRoundRobin = new WeightedRoundRobin(entity); + orderedEntities = weightedRoundRobin.orderEntities(); + expectedOrdering = Arrays.asList("C"); + actualOrdering = new ArrayList<>(); + for (WeightedRoundRobin.Entity en : orderedEntities) { + actualOrdering.add(en.getTarget()); + } + assertEquals(expectedOrdering, actualOrdering); + + // weights set C:2 + entity = new ArrayList>(); + entity.add(new WeightedRoundRobin.Entity<>(2, "C")); + weightedRoundRobin = new WeightedRoundRobin(entity); + orderedEntities = weightedRoundRobin.orderEntities(); + expectedOrdering = Arrays.asList("C", "C"); + actualOrdering = new ArrayList<>(); + for (WeightedRoundRobin.Entity en : orderedEntities) { + actualOrdering.add(en.getTarget()); + } + assertEquals(expectedOrdering, actualOrdering); } } From c98606de0d87f0feedb3ca46ceb0ae0dea4dc2fc Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Thu, 15 Sep 2022 16:48:34 +0530 Subject: [PATCH 22/31] Changes weight data type to double Signed-off-by: Anshu Agarwal --- .../metadata/WeightedRoutingMetadata.java | 8 ++++---- .../cluster/routing/IndexShardRoutingTable.java | 8 ++++---- .../cluster/routing/OperationRouting.java | 12 ++++++------ .../cluster/routing/WeightedRouting.java | 10 +++++----- .../metadata/WeightedRoutingMetadataTests.java | 2 +- .../cluster/routing/OperationRoutingTests.java | 16 ++++++++-------- .../cluster/structure/RoutingIteratorTests.java | 8 ++++---- 7 files changed, 32 insertions(+), 32 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java index 6ab2cf22adb02..fbe7692bd52e2 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java @@ -85,9 +85,9 @@ public static NamedDiff readDiffFrom(StreamInput in) throws IOE public static WeightedRoutingMetadata fromXContent(XContentParser parser) throws IOException { String attrKey = null; - Object attrValue; + Double attrValue; String attributeName = null; - Map weights = new HashMap<>(); + Map weights = new HashMap<>(); WeightedRouting weightedRouting = null; XContentParser.Token token; // move to the first alias @@ -115,7 +115,7 @@ public static WeightedRoutingMetadata fromXContent(XContentParser parser) throws if (token == XContentParser.Token.FIELD_NAME) { attrKey = parser.currentName(); } else if (token == XContentParser.Token.VALUE_STRING) { - attrValue = parser.text(); + attrValue = Double.parseDouble(parser.text()); weights.put(attrKey, attrValue); } else { throw new OpenSearchParseException( @@ -153,7 +153,7 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params par public static void toXContent(WeightedRouting weightedRouting, XContentBuilder builder) throws IOException { builder.startObject(AWARENESS); builder.startObject(weightedRouting.attributeName()); - for (Map.Entry entry : weightedRouting.weights().entrySet()) { + for (Map.Entry entry : weightedRouting.weights().entrySet()) { builder.field(entry.getKey(), entry.getValue()); } builder.endObject(); diff --git a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java index 8727652d8611a..b54caad6584e3 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java +++ b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java @@ -305,7 +305,7 @@ public ShardIterator activeInitializingShardsWeightedIt( WeightedRouting weightedRouting, DiscoveryNodes nodes, WeightedRoutingCache cache, - int defaultWeight + double defaultWeight ) { final int seed = shuffler.nextSeed(); List ordered = new ArrayList<>(); @@ -335,7 +335,7 @@ private List shardsOrderedByWeight( List shards, WeightedRouting weightedRouting, DiscoveryNodes nodes, - int defaultWeight + double defaultWeight ) { WeightedRoundRobin weightedRoundRobin = new WeightedRoundRobin<>( calculateShardWeight(shards, weightedRouting, nodes, defaultWeight) @@ -359,14 +359,14 @@ private List> calculateShardWeight( List shards, WeightedRouting weightedRouting, DiscoveryNodes nodes, - int defaultWeight + double defaultWeight ) { List> shardsWithWeights = new ArrayList<>(); for (ShardRouting shard : shards) { DiscoveryNode node = nodes.get(shard.currentNodeId()); String attVal = node.getAttributes().get(weightedRouting.attributeName()); // If weight for a zone is not defined, considering it as 1 by default - Double weight = Double.parseDouble(weightedRouting.weights().getOrDefault(attVal, defaultWeight).toString()); + Double weight = weightedRouting.weights().getOrDefault(attVal, defaultWeight); shardsWithWeights.add(new WeightedRoundRobin.Entity<>(weight, shard)); } return shardsWithWeights; diff --git a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java index 8763784a76e2e..175298374415d 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java +++ b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java @@ -78,17 +78,17 @@ public class OperationRouting { Setting.Property.Dynamic, Setting.Property.NodeScope ); - public static final Setting WEIGHTED_ROUTING_DEFAULT_WEIGHT = Setting.intSetting( + public static final Setting WEIGHTED_ROUTING_DEFAULT_WEIGHT = Setting.doubleSetting( "cluster.routing.weighted.default_weight", - 1, - 1, + 1.0, + 1.0, Setting.Property.Dynamic, Setting.Property.NodeScope ); private volatile List awarenessAttributes; private volatile boolean useAdaptiveReplicaSelection; private volatile boolean ignoreAwarenessAttr; - private volatile int weightedRoutingDefaultWeight; + private volatile double weightedRoutingDefaultWeight; private WeightedRoutingCache weightedRoutingCache; private ClusterService clusterService; @@ -116,7 +116,7 @@ void setIgnoreAwarenessAttributes(boolean ignoreAwarenessAttributes) { this.ignoreAwarenessAttr = ignoreAwarenessAttributes; } - void setWeightedRoutingDefaultWeight(int weightedRoutingDefaultWeight) { + void setWeightedRoutingDefaultWeight(double weightedRoutingDefaultWeight) { this.weightedRoutingDefaultWeight = weightedRoutingDefaultWeight; } @@ -136,7 +136,7 @@ public boolean ignoreAwarenessAttributes() { return this.awarenessAttributes.isEmpty() || this.ignoreAwarenessAttr; } - public int getWeightedRoutingDefaultWeight() { + public double getWeightedRoutingDefaultWeight() { return this.weightedRoutingDefaultWeight; } diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRouting.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRouting.java index 90f9b1beb2ed1..02ed0a110a42e 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRouting.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRouting.java @@ -23,9 +23,9 @@ */ public class WeightedRouting implements Writeable { private String attributeName; - private Map weights; + private Map weights; - public WeightedRouting(String attributeName, Map weights) { + public WeightedRouting(String attributeName, Map weights) { this.attributeName = attributeName; this.weights = weights; } @@ -37,13 +37,13 @@ public WeightedRouting(WeightedRouting weightedRouting) { public WeightedRouting(StreamInput in) throws IOException { attributeName = in.readString(); - weights = in.readMap(); + weights = (Map) in.readGenericValue(); } @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(attributeName); - out.writeMap(weights); + out.writeGenericValue(weights); } @Override @@ -67,7 +67,7 @@ public String toString() { return "WeightedRouting{" + attributeName + "}{" + weights().toString() + "}"; } - public Map weights() { + public Map weights() { return this.weights; } diff --git a/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoutingMetadataTests.java b/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoutingMetadataTests.java index 01a55f7f4fa4a..a0a9d2bd9586b 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoutingMetadataTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoutingMetadataTests.java @@ -18,7 +18,7 @@ public class WeightedRoutingMetadataTests extends AbstractXContentTestCase { @Override protected WeightedRoutingMetadata createTestInstance() { - Map weights = Map.of("a", "1", "b", "1", "c", "0"); + Map weights = Map.of("a", 1.0, "b", 1.0, "c", 0.0); WeightedRouting weightedRouting = new WeightedRouting("zone", weights); WeightedRoutingMetadata weightedRoutingMetadata = new WeightedRoutingMetadata(weightedRouting); return weightedRoutingMetadata; diff --git a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java index 6cad82e8ead6b..2a2c5d0fd731b 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java @@ -789,7 +789,7 @@ private ClusterState clusterStateForWeightedRouting(String[] indexNames, int num } - private ClusterState setWeightedRoutingWeights(ClusterState clusterState, Map weights) { + private ClusterState setWeightedRoutingWeights(ClusterState clusterState, Map weights) { WeightedRouting weightedRouting = new WeightedRouting("zone", weights); WeightedRoutingMetadata weightedRoutingMetadata = new WeightedRoutingMetadata(weightedRouting); Metadata.Builder metadataBuilder = Metadata.builder(clusterState.metadata()); @@ -827,7 +827,7 @@ public void testWeightedOperationRouting() throws Exception { Map outstandingRequests = new HashMap<>(); // Setting up weights for weighted round-robin in cluster state - Map weights = Map.of("a", "1", "b", "1", "c", "0"); + Map weights = Map.of("a", 1.0, "b", 1.0, "c", 0.0); state = setWeightedRoutingWeights(state, weights); ClusterState.Builder builder = ClusterState.builder(state); @@ -863,7 +863,7 @@ public void testWeightedOperationRouting() throws Exception { setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); // Updating weighted round robin weights in cluster state - weights = Map.of("a", "1", "b", "0", "c", "1"); + weights = Map.of("a", 1.0, "b", 0.0, "c", 1.0); state = setWeightedRoutingWeights(state, weights); builder = ClusterState.builder(state); @@ -924,7 +924,7 @@ public void testWeightedOperationRoutingCaching() throws Exception { Map outstandingRequests = new HashMap<>(); // Setting up weights for weighted round-robin in cluster state - Map weights = Map.of("a", "1", "b", "1", "c", "0"); + Map weights = Map.of("a", 1.0, "b", 1.0, "c", 0.0); state = setWeightedRoutingWeights(state, weights); ClusterServiceUtils.setState(clusterService, ClusterState.builder(state)); @@ -954,7 +954,7 @@ public void testWeightedOperationRoutingCaching() throws Exception { assertEquals(6, opRouting.getWeightedRoutingCache().misses()); // Updating cluster state to test weighted routing details are calculated again - weights = Map.of("a", "1", "b", "0", "c", "1"); + weights = Map.of("a", 1.0, "b", 0.0, "c", 1.0); // building new cluster state ClusterState state2 = setWeightedRoutingWeights(state, weights); ClusterServiceUtils.setState(clusterService, ClusterState.builder(state2)); @@ -1009,7 +1009,7 @@ public void testWeightedOperationRoutingWeightUndefinedForOneZone() throws Excep Map outstandingRequests = new HashMap<>(); // Setting up weights for weighted round-robin in cluster state, weight for nodes in zone b is not set - Map weights = Map.of("a", "1", "c", "0"); + Map weights = Map.of("a", 1.0, "c", 0.0); state = setWeightedRoutingWeights(state, weights); ClusterServiceUtils.setState(clusterService, ClusterState.builder(state)); @@ -1048,7 +1048,7 @@ public void testWeightedOperationRoutingWeightUndefinedForOneZone() throws Excep setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); // Updating weighted round robin weights in cluster state - weights = Map.of("a", "0", "b", "1"); + weights = Map.of("a", 0.0, "b", 1.0); state = setWeightedRoutingWeights(state, weights); ClusterServiceUtils.setState(clusterService, ClusterState.builder(state)); @@ -1248,7 +1248,7 @@ private ClusterState updateStatetoTestWeightedRouting( routingTableBuilder.add(indexRoutingTableBuilder.build()); } // add weighted routing weights in metadata - Map weights = Map.of("a", "1", "b", "1", "c", "0"); + Map weights = Map.of("a", 1.0, "b", 1.0, "c", 0.0); WeightedRouting weightedRouting = new WeightedRouting("zone", weights); WeightedRoutingMetadata weightedRoutingMetadata = new WeightedRoutingMetadata(weightedRouting); metadataBuilder.putCustom(WeightedRoutingMetadata.TYPE, weightedRoutingMetadata); diff --git a/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java b/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java index c75480ab782ba..0e6e7eb51b168 100644 --- a/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java +++ b/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java @@ -549,7 +549,7 @@ public void testWeightedRouting() { clusterState = startInitializingShardsAndReroute(strategy, clusterState); WeightedRoutingCache cache = new WeightedRoutingCache(clusterService); - Map weights = Map.of("zone1", "1", "zone2", "1", "zone3", "0"); + Map weights = Map.of("zone1", 1.0, "zone2", 1.0, "zone3", 0.0); WeightedRouting weightedRouting = new WeightedRouting("zone", weights); ShardIterator shardIterator = clusterState.routingTable() @@ -566,7 +566,7 @@ public void testWeightedRouting() { assertFalse(shardRouting.currentNodeId().equals("node3")); cache.close(); - weights = Map.of("zone1", "1", "zone2", "1", "zone3", "1"); + weights = Map.of("zone1", 1.0, "zone2", 1.0, "zone3", 1.0); weightedRouting = new WeightedRouting("zone", weights); shardIterator = clusterState.routingTable() .index("test") @@ -575,7 +575,7 @@ public void testWeightedRouting() { assertEquals(3, shardIterator.size()); cache.close(); - weights = Map.of("zone1", "-1", "zone2", "0", "zone3", "1"); + weights = Map.of("zone1", -1.0, "zone2", 0.0, "zone3", 1.0); weightedRouting = new WeightedRouting("zone", weights); shardIterator = clusterState.routingTable() .index("test") @@ -584,7 +584,7 @@ public void testWeightedRouting() { assertEquals(1, shardIterator.size()); cache.close(); - weights = Map.of("zone1", "0", "zone2", "0", "zone3", "0"); + weights = Map.of("zone1", 0.0, "zone2", 0.0, "zone3", 0.0); weightedRouting = new WeightedRouting("zone", weights); shardIterator = clusterState.routingTable() .index("test") From 1bd83f3c536b302bf9af389ec5ee7d0794282b2d Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Thu, 15 Sep 2022 17:19:49 +0530 Subject: [PATCH 23/31] Fix test Signed-off-by: Anshu Agarwal --- .../opensearch/cluster/metadata/WeightedRoutingMetadata.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java index fbe7692bd52e2..6b69a208d5b97 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java @@ -114,7 +114,7 @@ public static WeightedRoutingMetadata fromXContent(XContentParser parser) throws while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { attrKey = parser.currentName(); - } else if (token == XContentParser.Token.VALUE_STRING) { + } else if (token == XContentParser.Token.VALUE_NUMBER) { attrValue = Double.parseDouble(parser.text()); weights.put(attrKey, attrValue); } else { From 8a72dc0d85732b6b7195e2b4c70389198e9fc85f Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Thu, 15 Sep 2022 18:00:59 +0530 Subject: [PATCH 24/31] Empty commit Signed-off-by: Anshu Agarwal From bff61b92ff73d43d9b93ecac7d9d71442dc299d8 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Thu, 15 Sep 2022 18:00:59 +0530 Subject: [PATCH 25/31] Empty commit Signed-off-by: Anshu Agarwal --- .../cluster/metadata/WeightedRoutingMetadataTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoutingMetadataTests.java b/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoutingMetadataTests.java index a0a9d2bd9586b..08e9167471fe1 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoutingMetadataTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoutingMetadataTests.java @@ -21,7 +21,7 @@ protected WeightedRoutingMetadata createTestInstance() { Map weights = Map.of("a", 1.0, "b", 1.0, "c", 0.0); WeightedRouting weightedRouting = new WeightedRouting("zone", weights); WeightedRoutingMetadata weightedRoutingMetadata = new WeightedRoutingMetadata(weightedRouting); - return weightedRoutingMetadata; + return weightedRoutingMetadata; } @Override From 910261669efc86128e62c8d2cebd928e56b7540e Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Thu, 15 Sep 2022 18:32:36 +0530 Subject: [PATCH 26/31] Fix spotless check Signed-off-by: Anshu Agarwal --- .../cluster/metadata/WeightedRoutingMetadataTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoutingMetadataTests.java b/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoutingMetadataTests.java index 08e9167471fe1..a0a9d2bd9586b 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoutingMetadataTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/WeightedRoutingMetadataTests.java @@ -21,7 +21,7 @@ protected WeightedRoutingMetadata createTestInstance() { Map weights = Map.of("a", 1.0, "b", 1.0, "c", 0.0); WeightedRouting weightedRouting = new WeightedRouting("zone", weights); WeightedRoutingMetadata weightedRoutingMetadata = new WeightedRoutingMetadata(weightedRouting); - return weightedRoutingMetadata; + return weightedRoutingMetadata; } @Override From a291634ac30e26d5d8c91f09141930fda7bba24e Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Fri, 16 Sep 2022 19:45:58 +0530 Subject: [PATCH 27/31] Create in-memory cache for shard routings Signed-off-by: Anshu Agarwal --- .../routing/IndexShardRoutingTable.java | 83 +++++++++---- .../cluster/routing/OperationRouting.java | 43 ++----- .../cluster/routing/WeightedRoutingCache.java | 109 ------------------ .../main/java/org/opensearch/node/Node.java | 6 - .../routing/OperationRoutingTests.java | 98 ---------------- .../structure/RoutingIteratorTests.java | 15 +-- 6 files changed, 77 insertions(+), 277 deletions(-) delete mode 100644 server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingCache.java diff --git a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java index b54caad6584e3..bc24d66447a67 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java +++ b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java @@ -85,6 +85,10 @@ public class IndexShardRoutingTable implements Iterable { private volatile Map activeShardsByAttributes = emptyMap(); private volatile Map initializingShardsByAttributes = emptyMap(); private final Object shardsByAttributeMutex = new Object(); + private final Object activeShardsByWeightMutex = new Object(); + private final Object initializingShardsByWeightMutex = new Object(); + private volatile Map> activeShardsByWeight = emptyMap(); + private volatile Map> initializingShardsByWeight = emptyMap(); /** * The initializing list, including ones that are initializing on a target node because of relocation. @@ -301,28 +305,13 @@ public ShardIterator activeInitializingShardsRankedIt( * @return an iterator over active and initializing shards, ordered by weighted round-robin * scheduling policy. Making sure that initializing shards are the last to iterate through. */ - public ShardIterator activeInitializingShardsWeightedIt( - WeightedRouting weightedRouting, - DiscoveryNodes nodes, - WeightedRoutingCache cache, - double defaultWeight - ) { + public ShardIterator activeInitializingShardsWeightedIt(WeightedRouting weightedRouting, DiscoveryNodes nodes, double defaultWeight) { final int seed = shuffler.nextSeed(); List ordered = new ArrayList<>(); - List orderedActiveShards = cache.get(new WeightedRoutingCache.Key(shardId)); - if (orderedActiveShards == null) { - orderedActiveShards = shardsOrderedByWeight(activeShards, weightedRouting, nodes, defaultWeight); - cache.put(new WeightedRoutingCache.Key(shardId), orderedActiveShards); - } + List orderedActiveShards = getActiveShardsByWeight(weightedRouting, nodes, defaultWeight); ordered.addAll(shuffler.shuffle(orderedActiveShards, seed)); - if (!allInitializingShards.isEmpty()) { - List orderedInitializingShards = shardsOrderedByWeight( - allInitializingShards, - weightedRouting, - nodes, - defaultWeight - ); + List orderedInitializingShards = getInitializingShardsByWeight(weightedRouting, nodes, defaultWeight); ordered.addAll(orderedInitializingShards); } return new PlainShardIterator(shardId, ordered); @@ -675,7 +664,7 @@ private AttributesRoutings getActiveAttribute(AttributesKey key, DiscoveryNodes List to = collectAttributeShards(key, nodes, from); shardRoutings = new AttributesRoutings(to, Collections.unmodifiableList(from)); - activeShardsByAttributes = MapBuilder.newMapBuilder(activeShardsByAttributes).put(key, shardRoutings).immutableMap(); + activeShardsByAttributes = new MapBuilder().put(key, shardRoutings).immutableMap(); } } return shardRoutings; @@ -688,9 +677,7 @@ private AttributesRoutings getInitializingAttribute(AttributesKey key, Discovery ArrayList from = new ArrayList<>(allInitializingShards); List to = collectAttributeShards(key, nodes, from); shardRoutings = new AttributesRoutings(to, Collections.unmodifiableList(from)); - initializingShardsByAttributes = MapBuilder.newMapBuilder(initializingShardsByAttributes) - .put(key, shardRoutings) - .immutableMap(); + initializingShardsByAttributes = new MapBuilder().put(key, shardRoutings).immutableMap(); } } return shardRoutings; @@ -778,6 +765,58 @@ public int shardsMatchingPredicateCount(Predicate predicate) { return count; } + /** + * Key for WeightedRouting + * + * @opensearch.internal + */ + static class WeightedRoutingKey { + private final WeightedRouting weightedRouting; + + WeightedRoutingKey(WeightedRouting weightedRouting) { + this.weightedRouting = weightedRouting; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + WeightedRoutingKey key = (WeightedRoutingKey) o; + if (!weightedRouting.equals(key.weightedRouting)) return false; + return true; + } + + @Override + public int hashCode() { + int result = weightedRouting.hashCode(); + return result; + } + } + + private List getActiveShardsByWeight(WeightedRouting weightedRouting, DiscoveryNodes nodes, double defaultWeight) { + WeightedRoutingKey key = new WeightedRoutingKey(weightedRouting); + List shardRoutings = activeShardsByWeight.get(key); + if (shardRoutings == null) { + synchronized (activeShardsByWeightMutex) { + shardRoutings = shardsOrderedByWeight(activeShards, weightedRouting, nodes, defaultWeight); + activeShardsByWeight = MapBuilder.newMapBuilder(activeShardsByWeight).put(key, shardRoutings).immutableMap(); + } + } + return shardRoutings; + } + + private List getInitializingShardsByWeight(WeightedRouting weightedRouting, DiscoveryNodes nodes, double defaultWeight) { + WeightedRoutingKey key = new WeightedRoutingKey(weightedRouting); + List shardRoutings = initializingShardsByWeight.get(key); + if (shardRoutings == null) { + synchronized (initializingShardsByWeightMutex) { + shardRoutings = shardsOrderedByWeight(activeShards, weightedRouting, nodes, defaultWeight); + initializingShardsByWeight = MapBuilder.newMapBuilder(initializingShardsByWeight).put(key, shardRoutings).immutableMap(); + } + } + return shardRoutings; + } + /** * Builder of an index shard routing table. * diff --git a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java index 175298374415d..9026da667ccb0 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java +++ b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java @@ -37,10 +37,8 @@ import org.opensearch.cluster.metadata.WeightedRoutingMetadata; import org.opensearch.cluster.node.DiscoveryNodes; import org.opensearch.cluster.routing.allocation.decider.AwarenessAllocationDecider; -import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.Nullable; import org.opensearch.common.Strings; -import org.opensearch.common.inject.Inject; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; @@ -90,9 +88,6 @@ public class OperationRouting { private volatile boolean ignoreAwarenessAttr; private volatile double weightedRoutingDefaultWeight; - private WeightedRoutingCache weightedRoutingCache; - private ClusterService clusterService; - public OperationRouting(Settings settings, ClusterSettings clusterSettings) { // whether to ignore awareness attributes when routing requests this.ignoreAwarenessAttr = clusterSettings.get(IGNORE_AWARENESS_ATTRIBUTES_SETTING); @@ -140,20 +135,6 @@ public double getWeightedRoutingDefaultWeight() { return this.weightedRoutingDefaultWeight; } - @Inject - public void setClusterService(ClusterService clusterService) { - this.clusterService = clusterService; - } - - @Inject - public void setWeightedRoutingCache(WeightedRoutingCache weightedRoutingCache) { - this.weightedRoutingCache = weightedRoutingCache; - } - - public WeightedRoutingCache getWeightedRoutingCache() { - return weightedRoutingCache; - } - public ShardIterator indexShards(ClusterState clusterState, String index, String id, @Nullable String routing) { return shards(clusterState, index, id, routing).shardsIt(); } @@ -171,7 +152,8 @@ public ShardIterator getShards( clusterState.nodes(), preference, null, - null + null, + clusterState.getMetadata().weightedRoutingMetadata() ); } @@ -183,7 +165,8 @@ public ShardIterator getShards(ClusterState clusterState, String index, int shar clusterState.nodes(), preference, null, - null + null, + clusterState.metadata().weightedRoutingMetadata() ); } @@ -213,7 +196,8 @@ public GroupShardsIterator searchShards( clusterState.nodes(), preference, collectorService, - nodeCounts + nodeCounts, + clusterState.metadata().weightedRoutingMetadata() ); if (iterator != null) { set.add(iterator); @@ -263,10 +247,11 @@ private ShardIterator preferenceActiveShardIterator( DiscoveryNodes nodes, @Nullable String preference, @Nullable ResponseCollectorService collectorService, - @Nullable Map nodeCounts + @Nullable Map nodeCounts, + @Nullable WeightedRoutingMetadata weightedRoutingMetadata ) { if (preference == null || preference.isEmpty()) { - return shardRoutings(indexShard, nodes, collectorService, nodeCounts); + return shardRoutings(indexShard, nodes, collectorService, nodeCounts, weightedRoutingMetadata); } if (preference.charAt(0) == '_') { Preference preferenceType = Preference.parse(preference); @@ -293,7 +278,7 @@ private ShardIterator preferenceActiveShardIterator( } // no more preference if (index == -1 || index == preference.length() - 1) { - return shardRoutings(indexShard, nodes, collectorService, nodeCounts); + return shardRoutings(indexShard, nodes, collectorService, nodeCounts, weightedRoutingMetadata); } else { // update the preference and continue preference = preference.substring(index + 1); @@ -336,17 +321,13 @@ private ShardIterator shardRoutings( IndexShardRoutingTable indexShard, DiscoveryNodes nodes, @Nullable ResponseCollectorService collectorService, - @Nullable Map nodeCounts + @Nullable Map nodeCounts, + @Nullable WeightedRoutingMetadata weightedRoutingMetadata ) { - WeightedRoutingMetadata weightedRoutingMetadata = null; - if (clusterService != null) { - weightedRoutingMetadata = clusterService.state().metadata().weightedRoutingMetadata(); - } if (weightedRoutingMetadata != null) { return indexShard.activeInitializingShardsWeightedIt( weightedRoutingMetadata.getWeightedRouting(), nodes, - weightedRoutingCache, getWeightedRoutingDefaultWeight() ); } else if (ignoreAwarenessAttributes()) { diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingCache.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingCache.java deleted file mode 100644 index ee4fef0260a8a..0000000000000 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingCache.java +++ /dev/null @@ -1,109 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.cluster.routing; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.opensearch.cluster.ClusterChangedEvent; -import org.opensearch.cluster.ClusterStateListener; -import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.cache.Cache; -import org.opensearch.common.cache.CacheBuilder; -import org.opensearch.common.lease.Releasable; -import org.opensearch.index.shard.ShardId; - -import java.util.List; - -/** - * The weighted shard routing cache allows caching shard routing iterator returned by Weighted round-robin scheduling - * policy, helping with improving similar requests. - * - * @opensearch.internal - */ -public class WeightedRoutingCache implements Releasable, ClusterStateListener { - private static final Logger logger = LogManager.getLogger(WeightedRoutingCache.class); - - private final Cache> cache; - private static final long sizeInBytes = 2000000; - - public WeightedRoutingCache(ClusterService clusterService) { - - CacheBuilder> cacheBuilder = CacheBuilder.>builder() - .removalListener(notification -> logger.info("Object" + " {} removed from cache", notification.getKey().shardId)) - .setMaximumWeight(sizeInBytes); - cache = cacheBuilder.build(); - clusterService.addListener(this); - } - - public long hits() { - return cache.stats().getHits(); - } - - public long misses() { - return cache.stats().getMisses(); - } - - public long size() { - return cache.count(); - } - - @Override - public void close() { - logger.debug("Invalidating WeightedRoutingCache on close"); - cache.invalidateAll(); - } - - /** - * Listens to cluster state change event and invalidate cache on such events - * - * @param event cluster state change event - */ - @Override - public void clusterChanged(ClusterChangedEvent event) { - logger.debug("Invalidating WeightedRoutingCache on ClusterChangedEvent"); - cache.invalidateAll(); - } - - public List get(Key k) { - return cache.get(k); - } - - public void put(Key key, List value) { - cache.put(key, value); - } - - /** - * Key for the WeightedRoutingCache - * - * @opensearch.internal - */ - public static class Key { - private final ShardId shardId; - - Key(ShardId shardId) { - this.shardId = shardId; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - WeightedRoutingCache.Key key = (WeightedRoutingCache.Key) o; - if (!shardId.equals(key.shardId)) return false; - return true; - } - - @Override - public int hashCode() { - int result = shardId.hashCode(); - return result; - } - - } -} diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index d0ab73e4fcc29..92e9815313fa0 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -36,8 +36,6 @@ import org.apache.logging.log4j.Logger; import org.apache.lucene.util.Constants; import org.apache.lucene.util.SetOnce; -import org.opensearch.cluster.routing.OperationRouting; -import org.opensearch.cluster.routing.WeightedRoutingCache; import org.opensearch.common.util.FeatureFlags; import org.opensearch.cluster.routing.allocation.AwarenessReplicaBalance; import org.opensearch.index.IndexingPressureService; @@ -908,7 +906,6 @@ protected Node( ); resourcesToClose.add(persistentTasksClusterService); final PersistentTasksService persistentTasksService = new PersistentTasksService(clusterService, threadPool, client); - final WeightedRoutingCache weightedRoutingCache = new WeightedRoutingCache(clusterService); modules.add(b -> { b.bind(Node.class).toInstance(this); @@ -952,7 +949,6 @@ protected Node( b.bind(SnapshotsInfoService.class).toInstance(snapshotsInfoService); b.bind(GatewayMetaState.class).toInstance(gatewayMetaState); b.bind(Discovery.class).toInstance(discoveryModule.getDiscovery()); - b.bind(WeightedRoutingCache.class).toInstance(weightedRoutingCache); { processRecoverySettings(settingsModule.getClusterSettings(), recoverySettings); b.bind(PeerRecoverySourceService.class) @@ -990,7 +986,6 @@ protected Node( b.bind(ShardLimitValidator.class).toInstance(shardLimitValidator); b.bind(FsHealthService.class).toInstance(fsHealthService); b.bind(SystemIndices.class).toInstance(systemIndices); - b.bind(OperationRouting.class).toInstance(clusterService.operationRouting()); }); injector = modules.createInjector(); @@ -1337,7 +1332,6 @@ public synchronized void close() throws IOException { toClose.add(() -> stopWatch.stop().start("node_environment")); toClose.add(injector.getInstance(NodeEnvironment.class)); toClose.add(stopWatch::stop); - toClose.add(injector.getInstance(WeightedRoutingCache.class)); if (logger.isTraceEnabled()) { toClose.add(() -> logger.trace("Close times for each service:\n{}", stopWatch.prettyPrint())); diff --git a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java index 2a2c5d0fd731b..aa32e78549125 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java @@ -33,7 +33,6 @@ import org.opensearch.Version; import org.opensearch.action.support.replication.ClusterStateCreationUtils; -import org.opensearch.cluster.ClusterChangedEvent; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.Metadata; @@ -595,7 +594,6 @@ public void testAdaptiveReplicaSelection() throws Exception { ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool); ResponseCollectorService collector = new ResponseCollectorService(clusterService); Map outstandingRequests = new HashMap<>(); - opRouting.setClusterService(clusterService); GroupShardsIterator groupIterator = opRouting.searchShards( state, indexNames, @@ -700,7 +698,6 @@ public void testAdaptiveReplicaSelectionWithZoneAwarenessIgnored() throws Except Set selectedNodes = new HashSet<>(numShards); ResponseCollectorService collector = new ResponseCollectorService(clusterService); Map outstandingRequests = new HashMap<>(); - opRouting.setClusterService(clusterService); GroupShardsIterator groupIterator = opRouting.searchShards( state, indexNames, @@ -833,9 +830,6 @@ public void testWeightedOperationRouting() throws Exception { ClusterState.Builder builder = ClusterState.builder(state); ClusterServiceUtils.setState(clusterService, builder); - opRouting.setClusterService(clusterService); - opRouting.setWeightedRoutingCache(new WeightedRoutingCache(clusterService)); - // search shards call GroupShardsIterator groupIterator = opRouting.searchShards( state, @@ -870,8 +864,6 @@ public void testWeightedOperationRouting() throws Exception { ClusterServiceUtils.setState(clusterService, builder); opRouting = new OperationRouting(setting, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)); - opRouting.setClusterService(clusterService); - opRouting.setWeightedRoutingCache(new WeightedRoutingCache(clusterService)); // search shards call groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); @@ -893,92 +885,6 @@ public void testWeightedOperationRouting() throws Exception { } } - public void testWeightedOperationRoutingCaching() throws Exception { - final int numIndices = 2; - final int numShards = 3; - final int numReplicas = 2; - // setting up indices - final String[] indexNames = new String[numIndices]; - for (int i = 0; i < numIndices; i++) { - indexNames[i] = "test" + i; - } - ClusterService clusterService = null; - TestThreadPool threadPool = null; - try { - ClusterState state = clusterStateForWeightedRouting(indexNames, numShards, numReplicas); - - Settings setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); - - threadPool = new TestThreadPool("testThatOnlyNodesSupport"); - clusterService = ClusterServiceUtils.createClusterService(threadPool); - - OperationRouting opRouting = new OperationRouting( - setting, - new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) - ); - opRouting.setClusterService(clusterService); - opRouting.setWeightedRoutingCache(new WeightedRoutingCache(clusterService)); - - assertTrue(opRouting.ignoreAwarenessAttributes()); - ResponseCollectorService collector = new ResponseCollectorService(clusterService); - Map outstandingRequests = new HashMap<>(); - - // Setting up weights for weighted round-robin in cluster state - Map weights = Map.of("a", 1.0, "b", 1.0, "c", 0.0); - state = setWeightedRoutingWeights(state, weights); - ClusterServiceUtils.setState(clusterService, ClusterState.builder(state)); - - // search shards call - GroupShardsIterator groupIterator = opRouting.searchShards( - state, - indexNames, - null, - null, - collector, - outstandingRequests - ); - - // shard weighted routing ordering details are not present in cache, the details are calculated and put in cache - assertEquals(6, opRouting.getWeightedRoutingCache().misses()); - assertEquals(6, opRouting.getWeightedRoutingCache().size()); - - // Calling operation routing again without any cluster state change to test that weighted routing details - // are fetched from cache - groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); - - // details are fetched from cache - assertEquals(6, opRouting.getWeightedRoutingCache().hits()); - // cache count stays same - assertEquals(6, opRouting.getWeightedRoutingCache().size()); - // cache misses stay same - assertEquals(6, opRouting.getWeightedRoutingCache().misses()); - - // Updating cluster state to test weighted routing details are calculated again - weights = Map.of("a", 1.0, "b", 0.0, "c", 1.0); - // building new cluster state - ClusterState state2 = setWeightedRoutingWeights(state, weights); - ClusterServiceUtils.setState(clusterService, ClusterState.builder(state2)); - - ClusterChangedEvent event = new ClusterChangedEvent("test", state2, state); - opRouting.getWeightedRoutingCache().clusterChanged(event); - - // cache is invalidated after cluster state change, cache count is zero - assertEquals(0, opRouting.getWeightedRoutingCache().size()); - - // search shards call - groupIterator = opRouting.searchShards(state2, indexNames, null, null, collector, outstandingRequests); - - // cache hit remain same - assertEquals(6, opRouting.getWeightedRoutingCache().hits()); - // cache miss increases by 6 - assertEquals(12, opRouting.getWeightedRoutingCache().misses()); - assertEquals(6, opRouting.getWeightedRoutingCache().size()); - } finally { - IOUtils.close(clusterService); - terminate(threadPool); - } - } - public void testWeightedOperationRoutingWeightUndefinedForOneZone() throws Exception { final int numIndices = 2; final int numShards = 3; @@ -1013,8 +919,6 @@ public void testWeightedOperationRoutingWeightUndefinedForOneZone() throws Excep state = setWeightedRoutingWeights(state, weights); ClusterServiceUtils.setState(clusterService, ClusterState.builder(state)); - opRouting.setClusterService(clusterService); - opRouting.setWeightedRoutingCache(new WeightedRoutingCache(clusterService)); // search shards call GroupShardsIterator groupIterator = opRouting.searchShards( state, @@ -1054,8 +958,6 @@ public void testWeightedOperationRoutingWeightUndefinedForOneZone() throws Excep ClusterServiceUtils.setState(clusterService, ClusterState.builder(state)); opRouting = new OperationRouting(setting, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)); - opRouting.setClusterService(clusterService); - opRouting.setWeightedRoutingCache(new WeightedRoutingCache(clusterService)); // search shards call groupIterator = opRouting.searchShards(state, indexNames, null, null, collector, outstandingRequests); diff --git a/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java b/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java index 0e6e7eb51b168..75587be4762d7 100644 --- a/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java +++ b/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java @@ -49,7 +49,6 @@ import org.opensearch.cluster.routing.ShardShuffler; import org.opensearch.cluster.routing.ShardsIterator; import org.opensearch.cluster.routing.WeightedRouting; -import org.opensearch.cluster.routing.WeightedRoutingCache; import org.opensearch.cluster.routing.allocation.AllocationService; import org.opensearch.cluster.routing.allocation.decider.ClusterRebalanceAllocationDecider; import org.opensearch.cluster.service.ClusterService; @@ -434,7 +433,6 @@ public void testShardsAndPreferNodeRouting() { Settings.EMPTY, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) ); - operationRouting.setClusterService(clusterService); GroupShardsIterator shardIterators = operationRouting.searchShards( clusterState, new String[] { "test" }, @@ -548,14 +546,13 @@ public void testWeightedRouting() { clusterState = startInitializingShardsAndReroute(strategy, clusterState); clusterState = startInitializingShardsAndReroute(strategy, clusterState); - WeightedRoutingCache cache = new WeightedRoutingCache(clusterService); Map weights = Map.of("zone1", 1.0, "zone2", 1.0, "zone3", 0.0); WeightedRouting weightedRouting = new WeightedRouting("zone", weights); ShardIterator shardIterator = clusterState.routingTable() .index("test") .shard(0) - .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), cache, 1); + .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), 1); assertEquals(2, shardIterator.size()); ShardRouting shardRouting = shardIterator.nextOrNull(); @@ -564,34 +561,30 @@ public void testWeightedRouting() { shardRouting = shardIterator.nextOrNull(); assertNotNull(shardRouting); assertFalse(shardRouting.currentNodeId().equals("node3")); - cache.close(); weights = Map.of("zone1", 1.0, "zone2", 1.0, "zone3", 1.0); weightedRouting = new WeightedRouting("zone", weights); shardIterator = clusterState.routingTable() .index("test") .shard(0) - .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), cache, 1); + .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), 1); assertEquals(3, shardIterator.size()); - cache.close(); weights = Map.of("zone1", -1.0, "zone2", 0.0, "zone3", 1.0); weightedRouting = new WeightedRouting("zone", weights); shardIterator = clusterState.routingTable() .index("test") .shard(0) - .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), cache, 1); + .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), 1); assertEquals(1, shardIterator.size()); - cache.close(); weights = Map.of("zone1", 0.0, "zone2", 0.0, "zone3", 0.0); weightedRouting = new WeightedRouting("zone", weights); shardIterator = clusterState.routingTable() .index("test") .shard(0) - .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), cache, 1); + .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), 1); assertEquals(0, shardIterator.size()); - cache.close(); } finally { terminate(threadPool); } From bd6c9e7e41b43d0b015d4b196475562133f69547 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Fri, 16 Sep 2022 20:00:59 +0530 Subject: [PATCH 28/31] Fix put operation for weighted shard routings Signed-off-by: Anshu Agarwal --- .../cluster/metadata/WeightedRoutingMetadata.java | 1 - .../cluster/routing/IndexShardRoutingTable.java | 10 ++++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java index 6b69a208d5b97..27beb21f28f7c 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/WeightedRoutingMetadata.java @@ -164,5 +164,4 @@ public static void toXContent(WeightedRouting weightedRouting, XContentBuilder b public String toString() { return Strings.toString(this); } - } diff --git a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java index bc24d66447a67..297100269d22a 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java +++ b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java @@ -664,7 +664,7 @@ private AttributesRoutings getActiveAttribute(AttributesKey key, DiscoveryNodes List to = collectAttributeShards(key, nodes, from); shardRoutings = new AttributesRoutings(to, Collections.unmodifiableList(from)); - activeShardsByAttributes = new MapBuilder().put(key, shardRoutings).immutableMap(); + activeShardsByAttributes = MapBuilder.newMapBuilder(activeShardsByAttributes).put(key, shardRoutings).immutableMap(); } } return shardRoutings; @@ -677,7 +677,9 @@ private AttributesRoutings getInitializingAttribute(AttributesKey key, Discovery ArrayList from = new ArrayList<>(allInitializingShards); List to = collectAttributeShards(key, nodes, from); shardRoutings = new AttributesRoutings(to, Collections.unmodifiableList(from)); - initializingShardsByAttributes = new MapBuilder().put(key, shardRoutings).immutableMap(); + initializingShardsByAttributes = MapBuilder.newMapBuilder(initializingShardsByAttributes) + .put(key, shardRoutings) + .immutableMap(); } } return shardRoutings; @@ -799,7 +801,7 @@ private List getActiveShardsByWeight(WeightedRouting weightedRouti if (shardRoutings == null) { synchronized (activeShardsByWeightMutex) { shardRoutings = shardsOrderedByWeight(activeShards, weightedRouting, nodes, defaultWeight); - activeShardsByWeight = MapBuilder.newMapBuilder(activeShardsByWeight).put(key, shardRoutings).immutableMap(); + activeShardsByWeight = new MapBuilder().put(key, shardRoutings).immutableMap(); } } return shardRoutings; @@ -811,7 +813,7 @@ private List getInitializingShardsByWeight(WeightedRouting weighte if (shardRoutings == null) { synchronized (initializingShardsByWeightMutex) { shardRoutings = shardsOrderedByWeight(activeShards, weightedRouting, nodes, defaultWeight); - initializingShardsByWeight = MapBuilder.newMapBuilder(initializingShardsByWeight).put(key, shardRoutings).immutableMap(); + initializingShardsByWeight = new MapBuilder().put(key, shardRoutings).immutableMap(); } } return shardRoutings; From d9368ab2c0f389b72cb6e2ebffe008420d29f118 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Sat, 17 Sep 2022 08:08:21 +0530 Subject: [PATCH 29/31] Add tests for shard routing in-memory store Signed-off-by: Anshu Agarwal --- .../routing/IndexShardRoutingTable.java | 8 +- .../structure/RoutingIteratorTests.java | 135 +++++++++++++++++- 2 files changed, 134 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java index 297100269d22a..f24d8d86a3b3b 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java +++ b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java @@ -237,6 +237,10 @@ public List assignedShards() { return this.assignedShards; } + public Map> getActiveShardsByWeight() { + return activeShardsByWeight; + } + public ShardIterator shardsRandomIt() { return new PlainShardIterator(shardId, shuffler.shuffle(shards)); } @@ -772,10 +776,10 @@ public int shardsMatchingPredicateCount(Predicate predicate) { * * @opensearch.internal */ - static class WeightedRoutingKey { + public static class WeightedRoutingKey { private final WeightedRouting weightedRouting; - WeightedRoutingKey(WeightedRouting weightedRouting) { + public WeightedRoutingKey(WeightedRouting weightedRouting) { this.weightedRouting = weightedRouting; } diff --git a/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java b/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java index 75587be4762d7..ff314a429759b 100644 --- a/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java +++ b/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java @@ -40,6 +40,7 @@ import org.opensearch.cluster.metadata.Metadata; import org.opensearch.cluster.node.DiscoveryNodes; import org.opensearch.cluster.routing.GroupShardsIterator; +import org.opensearch.cluster.routing.IndexShardRoutingTable; import org.opensearch.cluster.routing.OperationRouting; import org.opensearch.cluster.routing.PlainShardIterator; import org.opensearch.cluster.routing.RotationShardShuffler; @@ -504,7 +505,7 @@ public void testReplicaShardPreferenceIters() throws Exception { } } - public void testWeightedRouting() { + public void testWeightedRoutingWithDifferentWeights() { TestThreadPool threadPool = null; try { Settings.Builder settings = Settings.builder() @@ -555,12 +556,12 @@ public void testWeightedRouting() { .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), 1); assertEquals(2, shardIterator.size()); - ShardRouting shardRouting = shardIterator.nextOrNull(); - assertNotNull(shardRouting); - assertFalse(shardRouting.currentNodeId().equals("node3")); - shardRouting = shardIterator.nextOrNull(); - assertNotNull(shardRouting); - assertFalse(shardRouting.currentNodeId().equals("node3")); + ShardRouting shardRouting; + while (shardIterator.remaining() > 0) { + shardRouting = shardIterator.nextOrNull(); + assertNotNull(shardRouting); + assertFalse(Arrays.asList("node3").contains(shardRouting.currentNodeId())); + } weights = Map.of("zone1", 1.0, "zone2", 1.0, "zone3", 1.0); weightedRouting = new WeightedRouting("zone", weights); @@ -577,6 +578,11 @@ public void testWeightedRouting() { .shard(0) .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), 1); assertEquals(1, shardIterator.size()); + while (shardIterator.remaining() > 0) { + shardRouting = shardIterator.nextOrNull(); + assertNotNull(shardRouting); + assertFalse(Arrays.asList("node2", "node1").contains(shardRouting.currentNodeId())); + } weights = Map.of("zone1", 0.0, "zone2", 0.0, "zone3", 0.0); weightedRouting = new WeightedRouting("zone", weights); @@ -589,4 +595,119 @@ public void testWeightedRouting() { terminate(threadPool); } } + + public void testWeightedRoutingInMemoryStore() { + TestThreadPool threadPool = null; + try { + Settings.Builder settings = Settings.builder() + .put("cluster.routing.allocation.node_concurrent_recoveries", 10) + .put("cluster.routing.allocation.awareness.attributes", "zone"); + AllocationService strategy = createAllocationService(settings.build()); + + Metadata metadata = Metadata.builder() + .put(IndexMetadata.builder("test").settings(settings(Version.CURRENT)).numberOfShards(1).numberOfReplicas(2)) + .build(); + + RoutingTable routingTable = RoutingTable.builder().addAsNew(metadata.index("test")).build(); + + ClusterState clusterState = ClusterState.builder(ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)) + .metadata(metadata) + .routingTable(routingTable) + .build(); + + threadPool = new TestThreadPool("testThatOnlyNodesSupport"); + ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool); + + Map node1Attributes = new HashMap<>(); + node1Attributes.put("zone", "zone1"); + Map node2Attributes = new HashMap<>(); + node2Attributes.put("zone", "zone2"); + Map node3Attributes = new HashMap<>(); + node3Attributes.put("zone", "zone3"); + clusterState = ClusterState.builder(clusterState) + .nodes( + DiscoveryNodes.builder() + .add(newNode("node1", unmodifiableMap(node1Attributes))) + .add(newNode("node2", unmodifiableMap(node2Attributes))) + .add(newNode("node3", unmodifiableMap(node3Attributes))) + .localNodeId("node1") + ) + .build(); + clusterState = strategy.reroute(clusterState, "reroute"); + + clusterState = startInitializingShardsAndReroute(strategy, clusterState); + clusterState = startInitializingShardsAndReroute(strategy, clusterState); + + Map weights = Map.of("zone1", 1.0, "zone2", 1.0, "zone3", 0.0); + WeightedRouting weightedRouting = new WeightedRouting("zone", weights); + + IndexShardRoutingTable indexShardRoutingTable = clusterState.routingTable().index("test").shard(0); + + assertNull( + indexShardRoutingTable.getActiveShardsByWeight().get(new IndexShardRoutingTable.WeightedRoutingKey(weightedRouting)) + ); + ShardIterator shardIterator = clusterState.routingTable() + .index("test") + .shard(0) + .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), 1); + assertEquals(2, shardIterator.size()); + ShardRouting shardRouting; + while (shardIterator.remaining() > 0) { + shardRouting = shardIterator.nextOrNull(); + assertNotNull(shardRouting); + assertFalse(Arrays.asList("node3").contains(shardRouting.currentNodeId())); + } + + // Make iterator call with same WeightedRouting instance + assertNotNull( + indexShardRoutingTable.getActiveShardsByWeight().get(new IndexShardRoutingTable.WeightedRoutingKey(weightedRouting)) + ); + shardIterator = clusterState.routingTable() + .index("test") + .shard(0) + .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), 1); + assertEquals(2, shardIterator.size()); + while (shardIterator.remaining() > 0) { + shardRouting = shardIterator.nextOrNull(); + assertNotNull(shardRouting); + assertFalse(Arrays.asList("node3").contains(shardRouting.currentNodeId())); + } + + // Make iterator call with new instance of WeightedRouting but same weights + Map weights1 = Map.of("zone1", 1.0, "zone2", 1.0, "zone3", 0.0); + weightedRouting = new WeightedRouting("zone", weights1); + assertNotNull( + indexShardRoutingTable.getActiveShardsByWeight().get(new IndexShardRoutingTable.WeightedRoutingKey(weightedRouting)) + ); + shardIterator = clusterState.routingTable() + .index("test") + .shard(0) + .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), 1); + assertEquals(2, shardIterator.size()); + while (shardIterator.remaining() > 0) { + shardRouting = shardIterator.nextOrNull(); + assertNotNull(shardRouting); + assertFalse(Arrays.asList("node3").contains(shardRouting.currentNodeId())); + } + + // Make iterator call with different weights + Map weights2 = Map.of("zone1", 1.0, "zone2", 0.0, "zone3", 1.0); + weightedRouting = new WeightedRouting("zone", weights2); + assertNull( + indexShardRoutingTable.getActiveShardsByWeight().get(new IndexShardRoutingTable.WeightedRoutingKey(weightedRouting)) + ); + shardIterator = clusterState.routingTable() + .index("test") + .shard(0) + .activeInitializingShardsWeightedIt(weightedRouting, clusterState.nodes(), 1); + assertEquals(2, shardIterator.size()); + while (shardIterator.remaining() > 0) { + shardRouting = shardIterator.nextOrNull(); + assertNotNull(shardRouting); + assertFalse(Arrays.asList("node2").contains(shardRouting.currentNodeId())); + } + } finally { + terminate(threadPool); + } + } } From 01239bfd30be680f207a0fe1a87af0f6c2dc6120 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Sat, 17 Sep 2022 11:05:11 +0530 Subject: [PATCH 30/31] Add java docs and some code refactoring Signed-off-by: Anshu Agarwal --- .../cluster/routing/IndexShardRoutingTable.java | 10 +++++++++- .../opensearch/cluster/routing/WeightedRouting.java | 2 -- .../cluster/routing/OperationRoutingTests.java | 3 ++- .../cluster/structure/RoutingIteratorTests.java | 5 +---- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java index f24d8d86a3b3b..fbd8050fd467c 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java +++ b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java @@ -772,7 +772,7 @@ public int shardsMatchingPredicateCount(Predicate predicate) { } /** - * Key for WeightedRouting + * Key for WeightedRouting Shard Iterator * * @opensearch.internal */ @@ -799,6 +799,10 @@ public int hashCode() { } } + /** + * * + * Gets active shard routing from memory if available, else calculates and put it in memory. + */ private List getActiveShardsByWeight(WeightedRouting weightedRouting, DiscoveryNodes nodes, double defaultWeight) { WeightedRoutingKey key = new WeightedRoutingKey(weightedRouting); List shardRoutings = activeShardsByWeight.get(key); @@ -811,6 +815,10 @@ private List getActiveShardsByWeight(WeightedRouting weightedRouti return shardRoutings; } + /** + * * + * Gets initializing shard routing from memory if available, else calculates and put it in memory. + */ private List getInitializingShardsByWeight(WeightedRouting weightedRouting, DiscoveryNodes nodes, double defaultWeight) { WeightedRoutingKey key = new WeightedRoutingKey(weightedRouting); List shardRoutings = initializingShardsByWeight.get(key); diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRouting.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRouting.java index 02ed0a110a42e..df2d8d595eaab 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRouting.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRouting.java @@ -50,9 +50,7 @@ public void writeTo(StreamOutput out) throws IOException { public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - WeightedRouting that = (WeightedRouting) o; - if (!attributeName.equals(that.attributeName)) return false; return weights.equals(that.weights); } diff --git a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java index aa32e78549125..87cab4a006a63 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java @@ -698,6 +698,7 @@ public void testAdaptiveReplicaSelectionWithZoneAwarenessIgnored() throws Except Set selectedNodes = new HashSet<>(numShards); ResponseCollectorService collector = new ResponseCollectorService(clusterService); Map outstandingRequests = new HashMap<>(); + GroupShardsIterator groupIterator = opRouting.searchShards( state, indexNames, @@ -847,7 +848,7 @@ public void testWeightedOperationRouting() throws Exception { selectedNodes.add(shardRouting.currentNodeId()); } } - // tests no nodes are assigned to nodes in zone c + // tests no shards are assigned to nodes in zone c for (String nodeID : selectedNodes) { // No shards are assigned to nodes in zone c since its weight is 0 assertFalse(nodeID.contains("c")); diff --git a/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java b/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java index ff314a429759b..8f5aa1b764551 100644 --- a/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java +++ b/server/src/test/java/org/opensearch/cluster/structure/RoutingIteratorTests.java @@ -427,13 +427,11 @@ public void testShardsAndPreferNodeRouting() { clusterState = startInitializingShardsAndReroute(strategy, clusterState); clusterState = startInitializingShardsAndReroute(strategy, clusterState); - TestThreadPool threadPool = new TestThreadPool("testThatOnlyNodesSupport"); - ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool); - OperationRouting operationRouting = new OperationRouting( Settings.EMPTY, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) ); + GroupShardsIterator shardIterators = operationRouting.searchShards( clusterState, new String[] { "test" }, @@ -475,7 +473,6 @@ public void testShardsAndPreferNodeRouting() { } else { assertThat(it.nextOrNull().currentNodeId(), equalTo("node1")); } - terminate(threadPool); } public void testReplicaShardPreferenceIters() throws Exception { From dbbb2638a1d74289d3054e43c33c425962a613da Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Mon, 19 Sep 2022 10:04:57 +0530 Subject: [PATCH 31/31] Add null check for discovery nodes and single mutex for weighted shard iterator Signed-off-by: Anshu Agarwal --- .../cluster/routing/IndexShardRoutingTable.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java index fbd8050fd467c..9026e7068e9fe 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java +++ b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java @@ -85,8 +85,7 @@ public class IndexShardRoutingTable implements Iterable { private volatile Map activeShardsByAttributes = emptyMap(); private volatile Map initializingShardsByAttributes = emptyMap(); private final Object shardsByAttributeMutex = new Object(); - private final Object activeShardsByWeightMutex = new Object(); - private final Object initializingShardsByWeightMutex = new Object(); + private final Object shardsByWeightMutex = new Object(); private volatile Map> activeShardsByWeight = emptyMap(); private volatile Map> initializingShardsByWeight = emptyMap(); @@ -357,10 +356,12 @@ private List> calculateShardWeight( List> shardsWithWeights = new ArrayList<>(); for (ShardRouting shard : shards) { DiscoveryNode node = nodes.get(shard.currentNodeId()); - String attVal = node.getAttributes().get(weightedRouting.attributeName()); - // If weight for a zone is not defined, considering it as 1 by default - Double weight = weightedRouting.weights().getOrDefault(attVal, defaultWeight); - shardsWithWeights.add(new WeightedRoundRobin.Entity<>(weight, shard)); + if (node != null) { + String attVal = node.getAttributes().get(weightedRouting.attributeName()); + // If weight for a zone is not defined, considering it as 1 by default + Double weight = weightedRouting.weights().getOrDefault(attVal, defaultWeight); + shardsWithWeights.add(new WeightedRoundRobin.Entity<>(weight, shard)); + } } return shardsWithWeights; } @@ -807,7 +808,7 @@ private List getActiveShardsByWeight(WeightedRouting weightedRouti WeightedRoutingKey key = new WeightedRoutingKey(weightedRouting); List shardRoutings = activeShardsByWeight.get(key); if (shardRoutings == null) { - synchronized (activeShardsByWeightMutex) { + synchronized (shardsByWeightMutex) { shardRoutings = shardsOrderedByWeight(activeShards, weightedRouting, nodes, defaultWeight); activeShardsByWeight = new MapBuilder().put(key, shardRoutings).immutableMap(); } @@ -823,7 +824,7 @@ private List getInitializingShardsByWeight(WeightedRouting weighte WeightedRoutingKey key = new WeightedRoutingKey(weightedRouting); List shardRoutings = initializingShardsByWeight.get(key); if (shardRoutings == null) { - synchronized (initializingShardsByWeightMutex) { + synchronized (shardsByWeightMutex) { shardRoutings = shardsOrderedByWeight(activeShards, weightedRouting, nodes, defaultWeight); initializingShardsByWeight = new MapBuilder().put(key, shardRoutings).immutableMap(); }