You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ab...@apache.org on 2021/02/10 14:49:01 UTC
[lucene-solr] 01/01: SOLR-15131: Use collection properties instead
of plugin configurationC for per-collection configs.
This is an automated email from the ASF dual-hosted git repository.
ab pushed a commit to branch jira/solr-15131
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit 76c7057c8d625b1b6ab8b4cf2ed2fc78252257a6
Author: Andrzej Bialecki <ab...@apache.org>
AuthorDate: Wed Feb 10 15:48:02 2021 +0100
SOLR-15131: Use collection properties instead of plugin configurationC for
per-collection configs.
---
.../cloud/api/collections/CreateCollectionCmd.java | 1 +
.../org/apache/solr/cluster/SolrCollection.java | 2 +-
.../impl/SimpleClusterAbstractionsImpl.java | 36 ++++++++++++++-
.../placement/plugins/AffinityPlacementConfig.java | 28 +-----------
.../plugins/AffinityPlacementFactory.java | 51 +++++++++-------------
.../impl/PlacementPluginIntegrationTest.java | 8 +++-
.../plugins/AffinityPlacementFactoryTest.java | 5 ++-
.../src/replica-placement-plugins.adoc | 44 ++++++++-----------
8 files changed, 89 insertions(+), 86 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
index d9de9a5..678b7e4 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
@@ -137,6 +137,7 @@ public class CreateCollectionCmd implements OverseerCollectionMessageHandler.Cmd
ZkStateReader zkStateReader = ocmh.zkStateReader;
+ // this also creates the collection zk node as a side-effect
OverseerCollectionMessageHandler.createConfNode(stateManager, configName, collectionName);
Map<String,String> collectionParams = new HashMap<>();
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 ea7ea45..62130c7 100644
--- a/solr/core/src/java/org/apache/solr/cluster/SolrCollection.java
+++ b/solr/core/src/java/org/apache/solr/cluster/SolrCollection.java
@@ -62,7 +62,7 @@ public interface SolrCollection {
/**
* <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.
+ * property was set. Properties are set through the Collection API. See for example {@code MODIFYCOLLECTION} 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
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 e26a374..55cbf5c 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
@@ -27,6 +27,7 @@ import org.apache.solr.cluster.*;
import org.apache.solr.common.cloud.ClusterState;
import org.apache.solr.common.cloud.DocCollection;
import org.apache.solr.common.cloud.Slice;
+import org.apache.solr.common.params.CollectionAdminParams;
import org.apache.solr.common.util.Pair;
import javax.annotation.Nonnull;
@@ -180,9 +181,19 @@ class SimpleClusterAbstractionsImpl {
}
@Override
+ public String toString() {
+ return "SolrCollectionImpl{" +
+ "collectionName='" + collectionName + '\'' +
+ ", shards=" + shards.keySet() +
+ '}';
+ }
+
+ @Override
public String getCustomProperty(String customPropertyName) {
- return docCollection.getStr(customPropertyName);
+ return docCollection.getStr(CollectionAdminParams.PROPERTY_PREFIX + customPropertyName);
}
+
+
}
@@ -292,6 +303,17 @@ class SimpleClusterAbstractionsImpl {
public int hashCode() {
return Objects.hash(shardName, collection, shardState);
}
+
+ @Override
+ public String toString() {
+ return "ShardImpl{" +
+ "shardName='" + shardName + '\'' +
+ ", collection=" + collection +
+ ", shardState=" + shardState +
+ ", replicas=" + replicas.size() +
+ ", leader=" + leader +
+ '}';
+ }
}
@@ -432,5 +454,17 @@ class SimpleClusterAbstractionsImpl {
public int hashCode() {
return Objects.hash(replicaName, coreName, shard, replicaType, replicaState, node);
}
+
+ @Override
+ public String toString() {
+ return "ReplicaImpl{" +
+ "replicaName='" + replicaName + '\'' +
+ ", coreName='" + coreName + '\'' +
+ ", shard=" + shard.getShardName() +
+ ", replicaType=" + replicaType +
+ ", replicaState=" + replicaState +
+ ", node=" + node +
+ '}';
+ }
}
}
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementConfig.java b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementConfig.java
index d9579bc..bc660ea 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementConfig.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementConfig.java
@@ -20,14 +20,13 @@ package org.apache.solr.cluster.placement.plugins;
import org.apache.solr.cluster.placement.PlacementPluginConfig;
import org.apache.solr.common.annotation.JsonProperty;
-import java.util.Map;
-import java.util.Objects;
-
/**
* Configuration bean for {@link AffinityPlacementFactory}.
*/
public class AffinityPlacementConfig implements PlacementPluginConfig {
+ public static final String WITH_COLLECTION_PROPERTY = "placement.affinity.withCollection";
+
public static final long DEFAULT_MINIMAL_FREE_DISK_GB = 20L;
public static final long DEFAULT_PRIORITIZED_FREE_DISK_GB = 100L;
@@ -51,15 +50,6 @@ public class AffinityPlacementConfig implements PlacementPluginConfig {
public long prioritizedFreeDiskGB;
/**
- * This property defines an additional constraint that primary collections (keys) should be
- * located on the same nodes as the secondary collections (values). The plugin will assume
- * that the secondary collection replicas are already in place and ignore candidate nodes where
- * they are not already present.
- */
- @JsonProperty
- public Map<String, String> withCollection;
-
- /**
* Zero-arguments public constructor required for deserialization - don't use.
*/
public AffinityPlacementConfig() {
@@ -72,21 +62,7 @@ public class AffinityPlacementConfig implements PlacementPluginConfig {
* @param prioritizedFreeDiskGB prioritized free disk GB.
*/
public AffinityPlacementConfig(long minimalFreeDiskGB, long prioritizedFreeDiskGB) {
- this(minimalFreeDiskGB, prioritizedFreeDiskGB, Map.of());
- }
-
- /**
- * Configuration for the {@link AffinityPlacementFactory}.
- * @param minimalFreeDiskGB minimal free disk GB.
- * @param prioritizedFreeDiskGB prioritized free disk GB.
- * @param withCollection configuration of co-located collections: keys are
- * primary collection names and values are secondary
- * collection names.
- */
- public AffinityPlacementConfig(long minimalFreeDiskGB, long prioritizedFreeDiskGB, Map<String, String> withCollection) {
this.minimalFreeDiskGB = minimalFreeDiskGB;
this.prioritizedFreeDiskGB = prioritizedFreeDiskGB;
- Objects.requireNonNull(withCollection);
- this.withCollection = withCollection;
}
}
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactory.java b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactory.java
index eaec4ab..d129b5b 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactory.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactory.java
@@ -149,7 +149,7 @@ public class AffinityPlacementFactory implements PlacementPluginFactory<Affinity
@Override
public PlacementPlugin createPluginInstance() {
- return new AffinityPlacementPlugin(config.minimalFreeDiskGB, config.prioritizedFreeDiskGB, config.withCollection);
+ return new AffinityPlacementPlugin(config.minimalFreeDiskGB, config.prioritizedFreeDiskGB);
}
@Override
@@ -173,29 +173,14 @@ public class AffinityPlacementFactory implements PlacementPluginFactory<Affinity
private final long prioritizedFreeDiskGB;
- // primary to secondary (1:1)
- private final Map<String, String> withCollections;
- // secondary to primary (1:N)
- private final Map<String, Set<String>> colocatedWith;
-
private final Random replicaPlacementRandom = new Random(); // ok even if random sequence is predictable.
/**
* The factory has decoded the configuration for the plugin instance and passes it the parameters it needs.
*/
- private AffinityPlacementPlugin(long minimalFreeDiskGB, long prioritizedFreeDiskGB, Map<String, String> withCollections) {
+ private AffinityPlacementPlugin(long minimalFreeDiskGB, long prioritizedFreeDiskGB) {
this.minimalFreeDiskGB = minimalFreeDiskGB;
this.prioritizedFreeDiskGB = prioritizedFreeDiskGB;
- Objects.requireNonNull(withCollections, "withCollections must not be null");
- this.withCollections = withCollections;
- if (withCollections.isEmpty()) {
- colocatedWith = Map.of();
- } else {
- colocatedWith = new HashMap<>();
- withCollections.forEach((primary, secondary) ->
- colocatedWith.computeIfAbsent(secondary, s -> new HashSet<>())
- .add(primary));
- }
// We make things reproducible in tests by using test seed if any
String seed = System.getProperty("tests.seed");
@@ -280,25 +265,31 @@ public class AffinityPlacementFactory implements PlacementPluginFactory<Affinity
private void verifyDeleteCollection(DeleteCollectionRequest deleteCollectionRequest, PlacementContext placementContext) throws PlacementModificationException, InterruptedException {
Cluster cluster = placementContext.getCluster();
- Set<String> colocatedCollections = colocatedWith.getOrDefault(deleteCollectionRequest.getCollection().getName(), Set.of());
- for (String primaryName : colocatedCollections) {
- try {
- if (cluster.getCollection(primaryName) != null) {
- // still exists
- throw new PlacementModificationException("colocated collection " + primaryName +
- " of " + deleteCollectionRequest.getCollection().getName() + " still present");
- }
- } catch (IOException e) {
- throw new PlacementModificationException("failed to retrieve colocated collection information", e);
+ Set<String> colocatedWith = new HashSet<>();
+ for (SolrCollection collection : cluster.collections()) {
+ String withCollection = collection.getCustomProperty(AffinityPlacementConfig.WITH_COLLECTION_PROPERTY);
+ if (withCollection != null && withCollection.equals(deleteCollectionRequest.getCollection().getName())) {
+ colocatedWith.add(collection.getName());
}
}
+ if (!colocatedWith.isEmpty()) {
+ // still exists
+ throw new PlacementModificationException("primary colocated collections: " + colocatedWith +
+ " of the secondary collection " + deleteCollectionRequest.getCollection().getName() + " still present!");
+ }
}
private void verifyDeleteReplicas(DeleteReplicasRequest deleteReplicasRequest, PlacementContext placementContext) throws PlacementModificationException, InterruptedException {
Cluster cluster = placementContext.getCluster();
SolrCollection secondaryCollection = deleteReplicasRequest.getCollection();
- Set<String> colocatedCollections = colocatedWith.get(secondaryCollection.getName());
- if (colocatedCollections == null) {
+ Set<String> colocatedCollections = new HashSet<>();
+ for (SolrCollection collection : cluster.collections()) {
+ String withCollection = collection.getCustomProperty(AffinityPlacementConfig.WITH_COLLECTION_PROPERTY);
+ if (withCollection != null && withCollection.equals(deleteReplicasRequest.getCollection().getName())) {
+ colocatedCollections.add(collection.getName());
+ }
+ }
+ if (colocatedCollections.isEmpty()) {
return;
}
Map<Node, Map<String, AtomicInteger>> secondaryNodeShardReplicas = new HashMap<>();
@@ -635,7 +626,7 @@ public class AffinityPlacementFactory implements PlacementPluginFactory<Affinity
private Set<Node> filterNodesWithCollection(Cluster cluster, PlacementRequest request, Set<Node> initialNodes) throws PlacementException {
// if there's a `withCollection` constraint for this collection then remove nodes
// that are not eligible
- String withCollectionName = withCollections.get(request.getCollection().getName());
+ String withCollectionName = request.getCollection().getCustomProperty(AffinityPlacementConfig.WITH_COLLECTION_PROPERTY);
if (withCollectionName == null) {
return initialNodes;
}
diff --git a/solr/core/src/test/org/apache/solr/cluster/placement/impl/PlacementPluginIntegrationTest.java b/solr/core/src/test/org/apache/solr/cluster/placement/impl/PlacementPluginIntegrationTest.java
index dd89bf9..1dc9798 100644
--- a/solr/core/src/test/org/apache/solr/cluster/placement/impl/PlacementPluginIntegrationTest.java
+++ b/solr/core/src/test/org/apache/solr/cluster/placement/impl/PlacementPluginIntegrationTest.java
@@ -41,6 +41,7 @@ import org.apache.solr.cloud.MiniSolrCloudCluster;
import org.apache.solr.cluster.placement.plugins.MinimizeCoresPlacementFactory;
import org.apache.solr.common.cloud.ClusterState;
import org.apache.solr.common.cloud.DocCollection;
+import org.apache.solr.common.params.CollectionAdminParams;
import org.apache.solr.core.CoreContainer;
import org.apache.solr.util.LogLevel;
@@ -241,7 +242,7 @@ public class PlacementPluginIntegrationTest extends SolrCloudTestCase {
PluginMeta plugin = new PluginMeta();
plugin.name = PlacementPluginFactory.PLUGIN_NAME;
plugin.klass = AffinityPlacementFactory.class.getName();
- plugin.config = new AffinityPlacementConfig(1, 2, Map.of(COLLECTION, SECONDARY_COLLECTION));
+ plugin.config = new AffinityPlacementConfig(1, 2);
V2Request req = new V2Request.Builder("/cluster/plugin")
.forceV2(true)
.POST()
@@ -264,6 +265,11 @@ public class PlacementPluginIntegrationTest extends SolrCloudTestCase {
.process(cluster.getSolrClient());
assertTrue(rsp.isSuccess());
cluster.waitForActiveCollection(COLLECTION, 2, 4);
+ rsp = CollectionAdminRequest.modifyCollection(COLLECTION,
+ Map.of(CollectionAdminParams.PROPERTY_PREFIX + AffinityPlacementConfig.WITH_COLLECTION_PROPERTY, SECONDARY_COLLECTION))
+ .process(cluster.getSolrClient());
+ assertEquals(0, rsp.getStatus());
+
// make sure the primary replicas were placed on the nodeset
DocCollection primary = cloudManager.getClusterStateProvider().getClusterState().getCollection(COLLECTION);
primary.forEachReplica((shard, replica) ->
diff --git a/solr/core/src/test/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactoryTest.java b/solr/core/src/test/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactoryTest.java
index 2fd02a0..877f0d5 100644
--- a/solr/core/src/test/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactoryTest.java
+++ b/solr/core/src/test/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactoryTest.java
@@ -57,8 +57,7 @@ public class AffinityPlacementFactoryTest extends SolrTestCaseJ4 {
public static void setupPlugin() {
AffinityPlacementConfig config = new AffinityPlacementConfig(
MINIMAL_FREE_DISK_GB,
- PRIORITIZED_FREE_DISK_GB,
- Map.of(primaryCollectionName, secondaryCollectionName));
+ PRIORITIZED_FREE_DISK_GB);
AffinityPlacementFactory factory = new AffinityPlacementFactory();
factory.configure(config);
plugin = factory.createPluginInstance();
@@ -661,6 +660,7 @@ public class AffinityPlacementFactoryTest extends SolrTestCaseJ4 {
collectionBuilder = Builders.newCollectionBuilder(primaryCollectionName);
collectionBuilder.initializeShardsReplicas(0, 0, 0, 0, clusterBuilder.getLiveNodeBuilders());
+ collectionBuilder.addCustomProperty(AffinityPlacementConfig.WITH_COLLECTION_PROPERTY, secondaryCollectionName);
clusterBuilder.addCollection(collectionBuilder);
PlacementContext placementContext = clusterBuilder.buildPlacementContext();
@@ -703,6 +703,7 @@ public class AffinityPlacementFactoryTest extends SolrTestCaseJ4 {
collectionBuilder = Builders.newCollectionBuilder(primaryCollectionName);
collectionBuilder.initializeShardsReplicas(2, 2, 0, 0, clusterBuilder.getLiveNodeBuilders());
+ collectionBuilder.addCustomProperty(AffinityPlacementConfig.WITH_COLLECTION_PROPERTY, secondaryCollectionName);
clusterBuilder.addCollection(collectionBuilder);
PlacementContext placementContext = clusterBuilder.buildPlacementContext();
diff --git a/solr/solr-ref-guide/src/replica-placement-plugins.adoc b/solr/solr-ref-guide/src/replica-placement-plugins.adoc
index 39369bc..6e481f0 100644
--- a/solr/solr-ref-guide/src/replica-placement-plugins.adoc
+++ b/solr/solr-ref-guide/src/replica-placement-plugins.adoc
@@ -72,11 +72,7 @@ curl -X POST -H 'Content-type: application/json' -d '{
"class": "org.apache.solr.cluster.placement.plugins.AffinityPlacementFactory",
"config": {
"minimalFreeDiskGB": 20,
- "prioritizedFreeDiskGB": 100,
- "withCollections": {
- "A_primary": "A_secondary",
- "B_primary": "B_secondary"
- }
+ "prioritizedFreeDiskGB": 100
}
}}'
http://localhost:8983/api/cluster/plugin
@@ -146,19 +142,25 @@ property are not configurable, and set respectively to `availability_zone` and `
This plugin supports enforcing additional constraint named `withCollection`, which causes
replicas of two paired collections to be placed on the same nodes.
-Users can define the collection pairs in the plugin configuration, in the `config/withCollection`
-element, which is a JSON map where keys are the primary collection names, and the values are the
-secondary collection names (currently only 1:1 mapping is supported - however, multiple primary
-collections may use the same secondary collection, which effectively relaxes this to N:1 mapping).
+Users can define the collection pairs in the primary collection properties using
+MODIFYCOLLECTION admin API like in the example below:
+
+[source,bash]
+----
+curl http://localhost:8983/solr/admin/collections?action=MODIFYCOLLECTION&collection=primary&property.placement.affinity.withCollection=secondary
+----
+
+The name of the property to set in the primary collection is `property.placement.affinity.withCollection`. Please note that the `property.` prefix is mandatory and indicates
+a custom collection property that is ignored by other parts of Solr.
-Unlike previous versions of Solr, this plugin does NOT automatically place replicas of the
+Unlike previous versions of Solr, this plugin does NOT automatically create replicas of the
secondary collection - those replicas are assumed to be already in place, and it's the
responsibility of the user to already place them on the right nodes (most likely simply by
using this plugin to create the secondary collection first, with large enough replication
factor to ensure that the target node set is populated with secondary replicas).
-When a request to compute placements is processed for the primary collection that has a
-key in the `withCollection` map, the set of candidate nodes is first filtered to eliminate nodes
+When a request to compute placements is processed for the primary collection that has
+the `withCollection` property, the set of candidate nodes is first filtered to eliminate nodes
that don't contain the replicas of the secondary collection. Please note that this may
result in an empty set, and an exception - in this case the sufficient number of secondary
replicas needs to be created first.
@@ -184,12 +186,6 @@ replicas to nodes with less than this amount of free disk space if that's an opt
not an option, replicas can still be assigned to nodes with less than this amount of free space).
Default value is 100.
-`withCollection`::
-(optional, map) this property defines additional constraints that primary collections (keys)
-must be located on the same nodes as the secondary collections (values). The plugin will
-assume that the secondary collection replicas are already in place and ignore candidate
-nodes where they are not already present. Default value is none.
-
=== Example configurations
This is a simple configuration that uses default values:
@@ -220,20 +216,18 @@ curl -X POST -H 'Content-type: application/json' -d '{
This configuration defines that collection `A_primary` must be co-located with
collection `Common_secondary`, and collection `B_primary` must be co-located also with the
-collection `Common_secondary`:
+collection `Common_secondary` (the example below assumes all collections already exist):
[source,bash]
----
curl -X POST -H 'Content-type: application/json' -d '{
"add":{
"name": ".placement-plugin",
- "class": "org.apache.solr.cluster.placement.plugins.AffinityPlacementFactory",
- "config": {
- "withCollection": {
- "A_primary": "Common_secondary",
- "B_primary": "Common_secondary"
- }
+ "class": "org.apache.solr.cluster.placement.plugins.AffinityPlacementFactory"
}
}}'
http://localhost:8983/api/cluster/plugin
+
+curl http://localhost:8983/solr/admin/collections?action=MODIFYCOLLECTION&collection=A_primary&property.placement.affinity.withCollection=Common_secondary
+curl http://localhost:8983/solr/admin/collections?action=MODIFYCOLLECTION&collection=B_primary&property.placement.affinity.withCollection=Common_secondary
----