You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by il...@apache.org on 2020/11/24 22:15:23 UTC
[lucene-solr] branch jira/solr-15004 updated: Remove
PluginTestHelper,
use Builders instead. Add getShardNames() to SolrCollection
This is an automated email from the ASF dual-hosted git repository.
ilan pushed a commit to branch jira/solr-15004
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/jira/solr-15004 by this push:
new 2be41f3 Remove PluginTestHelper, use Builders instead. Add getShardNames() to SolrCollection
2be41f3 is described below
commit 2be41f3cecb285afad31ed9f359ddf5ea10f2eee
Author: Ilan Ginzburg <ig...@salesforce.com>
AuthorDate: Tue Nov 24 23:14:06 2020 +0100
Remove PluginTestHelper, use Builders instead. Add getShardNames() to SolrCollection
---
.../org/apache/solr/cluster/SolrCollection.java | 33 ++++---
.../impl/SimpleClusterAbstractionsImpl.java | 5 +
.../impl/AffinityPlacementFactoryTest.java | 107 ++++++---------------
.../solr/cluster/placement/impl/Builders.java | 9 +-
.../placement/impl/ClusterAbstractionsForTest.java | 9 +-
.../cluster/placement/impl/PluginTestHelper.java | 94 ------------------
6 files changed, 62 insertions(+), 195 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/cluster/SolrCollection.java b/solr/core/src/java/org/apache/solr/cluster/SolrCollection.java
index 23e79a4..d22560a 100644
--- a/solr/core/src/java/org/apache/solr/cluster/SolrCollection.java
+++ b/solr/core/src/java/org/apache/solr/cluster/SolrCollection.java
@@ -23,6 +23,7 @@ import org.apache.solr.cluster.placement.PlacementPlugin;
import org.apache.solr.cluster.placement.PlacementRequest;
import java.util.Iterator;
+import java.util.Set;
/**
* Represents a Collection in SolrCloud (unrelated to {@link java.util.Collection} that uses the nicer name).
@@ -55,20 +56,26 @@ public interface SolrCollection {
Iterable<Shard> shards();
/**
- * <p>Returns the value of a custom property name set on the {@link SolrCollection} or {@code null} when no such
- * property was set. Properties are set through the Collection API. See for example {@code COLLECTIONPROP} in the Solr reference guide.
- *
- * <p><b>{@link PlacementPlugin} related note:</b></p>
- * <p>Using custom properties in conjunction with ad hoc {@link PlacementPlugin} code allows customizing placement
- * decisions per collection.
- *
- * <p>For example if a collection is to be placed only on nodes using SSD storage and not rotating disks, it can be
- * identified as such using some custom property (collection property could for example be called "driveType" and have
- * value "ssd" in that case), and the placement plugin (implementing {@link PlacementPlugin}) would then
- * {@link AttributeFetcher#requestNodeSystemProperty(String)} for that property from all nodes and only place replicas
- * of this collection on {@link Node}'s for which
- * {@link AttributeValues#getDiskType(Node)} is non empty and equal to {@link org.apache.solr.cluster.placement.AttributeFetcher.DiskHardwareType#SSD}.
+ * @return a set of the names of the shards defined for this collection. This set is backed by an internal map so should
+ * not be modified.
*/
+ Set<String> getShardNames();
+
+ /**
+ * <p>Returns the value of a custom property name set on the {@link SolrCollection} or {@code null} when no such
+ * property was set. Properties are set through the Collection API. See for example {@code COLLECTIONPROP} in the Solr reference guide.
+ *
+ * <p><b>{@link PlacementPlugin} related note:</b></p>
+ * <p>Using custom properties in conjunction with ad hoc {@link PlacementPlugin} code allows customizing placement
+ * decisions per collection.
+ *
+ * <p>For example if a collection is to be placed only on nodes using SSD storage and not rotating disks, it can be
+ * identified as such using some custom property (collection property could for example be called "driveType" and have
+ * value "ssd" in that case), and the placement plugin (implementing {@link PlacementPlugin}) would then
+ * {@link AttributeFetcher#requestNodeSystemProperty(String)} for that property from all nodes and only place replicas
+ * of this collection on {@link Node}'s for which
+ * {@link AttributeValues#getDiskType(Node)} is non empty and equal to {@link org.apache.solr.cluster.placement.AttributeFetcher.DiskHardwareType#SSD}.
+ */
String getCustomProperty(String customPropertyName);
/*
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/impl/SimpleClusterAbstractionsImpl.java b/solr/core/src/java/org/apache/solr/cluster/placement/impl/SimpleClusterAbstractionsImpl.java
index 6ea2d24..47b8f6c 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/impl/SimpleClusterAbstractionsImpl.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/impl/SimpleClusterAbstractionsImpl.java
@@ -167,6 +167,11 @@ class SimpleClusterAbstractionsImpl {
}
@Override
+ public Set<String> getShardNames() {
+ return shards.keySet();
+ }
+
+ @Override
public String getCustomProperty(String customPropertyName) {
return docCollection.getStr(customPropertyName);
}
diff --git a/solr/core/src/test/org/apache/solr/cluster/placement/impl/AffinityPlacementFactoryTest.java b/solr/core/src/test/org/apache/solr/cluster/placement/impl/AffinityPlacementFactoryTest.java
index 3981b09..8f7e130 100644
--- a/solr/core/src/test/org/apache/solr/cluster/placement/impl/AffinityPlacementFactoryTest.java
+++ b/solr/core/src/test/org/apache/solr/cluster/placement/impl/AffinityPlacementFactoryTest.java
@@ -62,17 +62,12 @@ public class AffinityPlacementFactoryTest extends Assert {
testBasicPlacementInternal(true);
}
- @Test
- public void testBasicPlacementNewCollection2() throws Exception {
- testBasicInternal2(false);
- }
-
- @Test
- public void testBasicPlacementExistingCollection2() throws Exception {
- testBasicInternal2(true);
- }
-
- private void testBasicInternal2(boolean hasExistingCollection) throws Exception {
+ /**
+ * When this test places a replica for a new collection, it should pick the node with less cores.<p>
+ * <p>
+ * When it places a replica for an existing collection, it should pick the node with more cores that doesn't already have a replica for the shard.
+ */
+ private void testBasicPlacementInternal(boolean hasExistingCollection) throws Exception {
String collectionName = "testCollection";
Builders.ClusterBuilder clusterBuilder = Builders.newClusterBuilder().initializeNodes(2);
@@ -109,50 +104,6 @@ public class AffinityPlacementFactoryTest extends Assert {
assertEquals(hasExistingCollection ? liveNodes.get(1) : liveNodes.get(0), rp.getNode());
}
- /**
- * When this test places a replica for a new collection, it should pick the node with less cores.<p>
- * <p>
- * When it places a replica for an existing collection, it should pick the node with more cores that doesn't already have a replica for the shard.
- */
- private void testBasicPlacementInternal(boolean hasExistingCollection) throws Exception {
- String collectionName = "testCollection";
-
- Node node1 = new ClusterAbstractionsForTest.NodeImpl("node1");
- Node node2 = new ClusterAbstractionsForTest.NodeImpl("node2");
- Set<Node> liveNodes = Set.of(node1, node2);
-
- ClusterAbstractionsForTest.SolrCollectionImpl solrCollection;
- // Make sure new collections are not visible in the cluster state and existing ones are
- final Map<String, SolrCollection> clusterCollections;
- if (hasExistingCollection) {
- // An existing collection with a single replica on node 1. Note that new collections already exist by the time the plugin is called, but are empty
- solrCollection = PluginTestHelper.createCollection(collectionName, Map.of(), 1, 1, 0, 0, Set.of(node1));
- clusterCollections = Map.of(solrCollection.getName(), solrCollection);
- } else {
- // A new collection has the shards defined ok but no replicas
- solrCollection = PluginTestHelper.createCollection(collectionName, Map.of(), 1, 0, 0, 0, Set.of());
- clusterCollections = Map.of();
- }
-
- Cluster cluster = new ClusterAbstractionsForTest.ClusterImpl(liveNodes, clusterCollections);
- // Place a new replica for the (only) existing shard of the collection
- PlacementRequestImpl placementRequest = new PlacementRequestImpl(solrCollection, Set.of(solrCollection.shards().iterator().next().getShardName()), liveNodes, 1, 0, 0);
- // More cores on node2
- Map<Node, Integer> nodeToCoreCount = Map.of(node1, 1, node2, 10);
- // A lot of free disk on the two nodes
- final Map<Node, Long> nodeToFreeDisk = Map.of(node1, 100L, node2, 100L);
- AttributeValues attributeValues = new AttributeValuesImpl(nodeToCoreCount, Map.of(), nodeToFreeDisk, Map.of(), Map.of(), Map.of(), Map.of(), Map.of());
- AttributeFetcher attributeFetcher = new AttributeFetcherForTest(attributeValues);
- PlacementPlanFactory placementPlanFactory = new PlacementPlanFactoryImpl();
-
- PlacementPlan pp = plugin.computePlacement(cluster, placementRequest, attributeFetcher, placementPlanFactory);
-
-
- assertEquals(1, pp.getReplicaPlacements().size());
- ReplicaPlacement rp = pp.getReplicaPlacements().iterator().next();
- assertEquals(hasExistingCollection ? node2 : node1, rp.getNode());
- }
-
@Test
public void testAvailabilityZones() throws Exception {
String collectionName = "testCollection";
@@ -311,38 +262,34 @@ public class AffinityPlacementFactoryTest extends Assert {
int nrtReplicas, int tlogReplicas,
int pullReplicas) throws Exception {
- int REPLICAS_PER_SHARD = nrtReplicas + tlogReplicas + pullReplicas;
- int TOTAL_REPLICAS = numShards * REPLICAS_PER_SHARD;
-
- String collectionName = "testCollection";
+ String collectionName = "scaleCollection";
- final Set<Node> liveNodes = new HashSet<>();
- final Map<Node, Long> nodeToFreeDisk = new HashMap<>();
- final Map<Node, Integer> nodeToCoreCount = new HashMap<>();
+ Builders.ClusterBuilder clusterBuilder = Builders.newClusterBuilder().initializeNodes(numNodes);
+ LinkedList<Builders.NodeBuilder> nodeBuilders = clusterBuilder.getNodeBuilders();
for (int i = 0; i < numNodes; i++) {
- Node node = new ClusterAbstractionsForTest.NodeImpl("node_" + i);
- liveNodes.add(node);
- nodeToFreeDisk.put(node, Long.valueOf(numNodes));
- nodeToCoreCount.put(node, 0);
+ nodeBuilders.get(i).setCoreCount(0).setFreeDiskGB(Long.valueOf(numNodes));
}
- ClusterAbstractionsForTest.SolrCollectionImpl solrCollection =
- PluginTestHelper.createCollection(collectionName, Map.of(), numShards, 0, 0, 0, Set.of());
- Cluster cluster = new ClusterAbstractionsForTest.ClusterImpl(liveNodes, Map.of());
- PlacementRequestImpl placementRequest = new PlacementRequestImpl(solrCollection,
- // XXX awkward!
- // StreamSupport.stream(solrCollection.shards().spliterator(), false)
- // .map(Shard::getShardName).collect(Collectors.toSet()),
- solrCollection.getShardNames(),
- liveNodes, nrtReplicas, tlogReplicas, pullReplicas);
-
- AttributeValues attributeValues = new AttributeValuesImpl(nodeToCoreCount, Map.of(), nodeToFreeDisk, Map.of(), Map.of(), Map.of(), Map.of(), Map.of());
- AttributeFetcher attributeFetcher = new AttributeFetcherForTest(attributeValues);
- PlacementPlanFactory placementPlanFactory = new PlacementPlanFactoryImpl();
+ Builders.CollectionBuilder collectionBuilder = Builders.newCollectionBuilder(collectionName);
+ collectionBuilder.initializeShardsReplicas(numShards, 0, 0, 0, List.of());
+
+ Cluster cluster = clusterBuilder.build();
+ AttributeFetcher attributeFetcher = clusterBuilder.buildAttributeFetcher();
+
+ SolrCollection solrCollection = collectionBuilder.build();
+ List<Node> liveNodes = clusterBuilder.buildLiveNodes();
+
+ // Place replicas for all the shards of the (newly created since it has no replicas yet) collection
+ PlacementRequestImpl placementRequest = new PlacementRequestImpl(solrCollection, solrCollection.getShardNames(),
+ new HashSet<>(liveNodes), nrtReplicas, tlogReplicas, pullReplicas);
long start = System.nanoTime();
- PlacementPlan pp = plugin.computePlacement(cluster, placementRequest, attributeFetcher, placementPlanFactory);
+ PlacementPlan pp = plugin.computePlacement(cluster, placementRequest, attributeFetcher, new PlacementPlanFactoryImpl());
long end = System.nanoTime();
+
+ final int REPLICAS_PER_SHARD = nrtReplicas + tlogReplicas + pullReplicas;
+ final int TOTAL_REPLICAS = numShards * REPLICAS_PER_SHARD;
+
log.info("ComputePlacement: {} nodes, {} shards, {} total replicas, elapsed time {} ms.", numNodes, numShards, TOTAL_REPLICAS, TimeUnit.NANOSECONDS.toMillis(end - start)); //nowarn
assertEquals("incorrect number of calculated placements", TOTAL_REPLICAS,
pp.getReplicaPlacements().size());
diff --git a/solr/core/src/test/org/apache/solr/cluster/placement/impl/Builders.java b/solr/core/src/test/org/apache/solr/cluster/placement/impl/Builders.java
index 1d964b2..1880374 100644
--- a/solr/core/src/test/org/apache/solr/cluster/placement/impl/Builders.java
+++ b/solr/core/src/test/org/apache/solr/cluster/placement/impl/Builders.java
@@ -27,7 +27,7 @@ public class Builders {
ClusterBuilder initializeNodes(int countNodes) {
nodeBuilders = new LinkedList<>();
for (int n = 0; n < countNodes; n++) {
- nodeBuilders.add(new NodeBuilder().setNodeName("node" + n)); // Default name, can be changed
+ nodeBuilders.add(new NodeBuilder().setNodeName("node_" + n)); // Default name, can be changed
}
return this;
}
@@ -128,9 +128,10 @@ public class Builders {
Iterator<NodeBuilder> nodeIterator = nodes.iterator();
shardBuilders = new LinkedList<>();
+ int replicaNumber = 0;
- for (int s = 0; s < countShards; s++) {
- String shardName = "shard" + (s + 1);
+ for (int shardNumber = 1; shardNumber <= countShards; shardNumber++) {
+ String shardName = "shard" + shardNumber;
LinkedList<ReplicaBuilder> replicas = new LinkedList<>();
ReplicaBuilder leader = null;
@@ -146,7 +147,7 @@ public class Builders {
int count = tc.second();
String replicaPrefix = collectionName + "_" + shardName + "_replica_" + type.getSuffixChar();
for (int r = 0; r < count; r++) {
- String replicaName = replicaPrefix + r;
+ String replicaName = replicaPrefix + replicaNumber++;
String coreName = replicaName + "_c";
if (!nodeIterator.hasNext()) {
nodeIterator = nodes.iterator();
diff --git a/solr/core/src/test/org/apache/solr/cluster/placement/impl/ClusterAbstractionsForTest.java b/solr/core/src/test/org/apache/solr/cluster/placement/impl/ClusterAbstractionsForTest.java
index bd14d0d..059143b 100644
--- a/solr/core/src/test/org/apache/solr/cluster/placement/impl/ClusterAbstractionsForTest.java
+++ b/solr/core/src/test/org/apache/solr/cluster/placement/impl/ClusterAbstractionsForTest.java
@@ -156,10 +156,6 @@ class ClusterAbstractionsForTest {
this.shards = shards;
}
- Set<String> getShardNames() {
- return shards.keySet();
- }
-
@Override
public String getName() {
return collectionName;
@@ -182,6 +178,11 @@ class ClusterAbstractionsForTest {
}
@Override
+ public Set<String> getShardNames() {
+ return shards.keySet();
+ }
+
+ @Override
public String getCustomProperty(String customPropertyName) {
return customProperties.get(customPropertyName);
}
diff --git a/solr/core/src/test/org/apache/solr/cluster/placement/impl/PluginTestHelper.java b/solr/core/src/test/org/apache/solr/cluster/placement/impl/PluginTestHelper.java
deleted file mode 100644
index 61670da..0000000
--- a/solr/core/src/test/org/apache/solr/cluster/placement/impl/PluginTestHelper.java
+++ /dev/null
@@ -1,94 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.solr.cluster.placement.impl;
-
-import org.apache.solr.cluster.Node;
-import org.apache.solr.cluster.Replica;
-import org.apache.solr.cluster.Shard;
-import org.apache.solr.cluster.SolrCollection;
-
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.Map;
-import java.util.Set;
-
-public class PluginTestHelper {
-
- static ClusterAbstractionsForTest.SolrCollectionImpl createCollection(String name, Map<String, String> properties,
- int numShards, int nrtReplicas, int tlogReplicas, int pullReplicas, Set<Node> nodes) {
- ClusterAbstractionsForTest.SolrCollectionImpl solrCollection = new ClusterAbstractionsForTest.SolrCollectionImpl(name, properties);
- Map<String, Shard> shards = createShardsAndReplicas(solrCollection, numShards, nrtReplicas, tlogReplicas, pullReplicas, nodes);
- solrCollection.setShards(shards);
- return solrCollection;
- }
-
- /**
- * Builds the representation of shards for a collection, based on the number of shards and replicas for each to create.
- * The replicas are allocated to the provided nodes in a round robin way. The leader is set to the last replica of each shard.
- */
- static Map<String, Shard> createShardsAndReplicas(SolrCollection collection, int numShards,
- int nrtReplicas, int tlogReplicas, int pullReplicas,
- Set<Node> nodes) {
- Iterator<Node> nodeIterator = nodes.iterator();
-
- Map<String, Shard> shards = new HashMap<>();
-
- for (int s = 0; s < numShards; s++) {
- // "traditional" shard name
- String shardName = "shard" + (s + 1);
-
- ClusterAbstractionsForTest.ShardImpl shard = new ClusterAbstractionsForTest.ShardImpl(shardName, collection, Shard.ShardState.ACTIVE);
-
- Map<String, Replica> replicas = new HashMap<>();
-
- Replica leader = null;
- int totalReplicas = nrtReplicas + tlogReplicas + pullReplicas;
- for (int r = 0; r < totalReplicas; r++) {
- Replica.ReplicaType type;
- if (r < nrtReplicas) {
- type = Replica.ReplicaType.NRT;
- } else if (r < nrtReplicas + tlogReplicas) {
- type = Replica.ReplicaType.TLOG;
- } else {
- type = Replica.ReplicaType.PULL;
- }
- String replicaName = shardName + "_replica_" + type.getSuffixChar() + r;
- String coreName = replicaName + "_c";
- final Node node;
- if (!nodeIterator.hasNext()) {
- nodeIterator = nodes.iterator();
- }
- // If the nodes set is empty, this call will fail
- node = nodeIterator.next();
-
- Replica replica = new ClusterAbstractionsForTest.ReplicaImpl(replicaName, coreName, shard, type, Replica.ReplicaState.ACTIVE, node);
-
- replicas.put(replica.getReplicaName(), replica);
- if (replica.getType() == Replica.ReplicaType.NRT) {
- leader = replica;
- }
- }
-
- shard.setReplicas(replicas, leader);
-
- shards.put(shard.getShardName(), shard);
- }
-
- return shards;
- }
-}