[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1417: SOLR-12847: Auto-create a policy rule that corresponds to maxShardsPerNode
shalinmangar commented on a change in pull request #1417: SOLR-12847: Auto-create a policy rule that corresponds to maxShardsPerNode URL: https://github.com/apache/lucene-solr/pull/1417#discussion_r406737685 ## File path: solr/solrj/src/java/org/apache/solr/common/params/CollectionAdminParams.java ## @@ -127,4 +127,8 @@ /** Option to follow aliases when deciding the target of a collection admin command. */ String FOLLOW_ALIASES = "followAliases"; + + /** Prefix for automatically created config elements. */ + String AUTO_PREFIX = ".auto_"; Review comment: We use a suffix ".AUTOCREATED" for configsets, maybe we can use the same here? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1417: SOLR-12847: Auto-create a policy rule that corresponds to maxShardsPerNode
shalinmangar commented on a change in pull request #1417: SOLR-12847:
Auto-create a policy rule that corresponds to maxShardsPerNode
URL: https://github.com/apache/lucene-solr/pull/1417#discussion_r406736277
##
File path:
solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
##
@@ -407,14 +466,81 @@ public void call(ClusterState clusterState, ZkNodeProps
message, NamedList resul
.assignPullReplicas(numPullReplicas)
.onNodes(nodeList)
.build();
- Assign.AssignStrategyFactory assignStrategyFactory = new
Assign.AssignStrategyFactory(cloudManager);
+ // if Strategy.POLICY is in use AND maxShardsPerNode was specified then
for back-compat it needs to
+ // be translated into a rule (see SOLR-12847).
+ // Modify the autoscaling rules as needed BEFORE actually calling
assign, setting configToRestore as
+ // a side-effect to restore the original config if the creation fails
+ if ((strategy == Assign.AssignStrategyFactory.Strategy.POLICY) &&
+ (maxReplicasPerNode != null && maxReplicasPerNode !=
Integer.MAX_VALUE)) {
+maybeAddMaxReplicasRule(cloudManager, maxReplicasPerNode,
docCollection, configToRestore);
Review comment:
I would have preferred to do this outside of this method instead of adding a
side effect here. That way the addition of the clause as well as its cleanup
can be in one place.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1417: SOLR-12847: Auto-create a policy rule that corresponds to maxShardsPerNode
shalinmangar commented on a change in pull request #1417: SOLR-12847:
Auto-create a policy rule that corresponds to maxShardsPerNode
URL: https://github.com/apache/lucene-solr/pull/1417#discussion_r406735984
##
File path:
solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
##
@@ -407,14 +466,81 @@ public void call(ClusterState clusterState, ZkNodeProps
message, NamedList resul
.assignPullReplicas(numPullReplicas)
.onNodes(nodeList)
.build();
- Assign.AssignStrategyFactory assignStrategyFactory = new
Assign.AssignStrategyFactory(cloudManager);
+ // if Strategy.POLICY is in use AND maxShardsPerNode was specified then
for back-compat it needs to
+ // be translated into a rule (see SOLR-12847).
+ // Modify the autoscaling rules as needed BEFORE actually calling
assign, setting configToRestore as
+ // a side-effect to restore the original config if the creation fails
+ if ((strategy == Assign.AssignStrategyFactory.Strategy.POLICY) &&
+ (maxReplicasPerNode != null && maxReplicasPerNode !=
Integer.MAX_VALUE)) {
+maybeAddMaxReplicasRule(cloudManager, maxReplicasPerNode,
docCollection, configToRestore);
+ }
Assign.AssignStrategy assignStrategy =
assignStrategyFactory.create(clusterState, docCollection);
replicaPositions = assignStrategy.assign(cloudManager, assignRequest);
sessionWrapper.set(PolicyHelper.getLastSessionWrapper(true));
}
return replicaPositions;
}
+ /**
+ * Optionally add a policy that implements 'maxShardsPerNode' and set the
collection to use this policy.
+ * @param cloudManager SolrCloudManager
+ * @param maxReplicasPerNode max number of replicas per node
+ * @param docCollection collection (its properties may be modified as a
side-effect)
+ * @param configToRestore if AutoScalingConfig has been changed as a result
of this method the original
+ *value is set here in order to restore it in case
of failures
+ */
+ public static void maybeAddMaxReplicasRule(SolrCloudManager cloudManager,
int maxReplicasPerNode, DocCollection docCollection,
+
AtomicReference configToRestore) throws IOException,
InterruptedException {
+AutoScalingConfig initialConfig =
cloudManager.getDistribStateManager().getAutoScalingConfig();
+Policy policy = initialConfig.getPolicy();
+String policyName = docCollection.getPolicyName();
+if (policyName == null) {
+ policyName = CollectionAdminParams.AUTO_PREFIX + docCollection.getName();
+ docCollection.getProperties().put(Policy.POLICY, policyName);
+}
+Map> policies = policy.getPolicies();
+List clauses = policies.get(policyName);
+maxReplicasPerNode++; // only < supported
+Clause c = Clause.create("{'replica': '<" + maxReplicasPerNode +"' ,
'shard': '#ANY', 'node': '#ANY'}");
+boolean alreadyExists = false;
+if (clauses != null) {
+ for (Clause clause : clauses) {
+if (clause.equals(c)) {
+ alreadyExists = true;
+ break;
+}
+if (clause.getReplica() != null) {
+ throw new Assign.AssignmentException("Both an existing policy=" +
policyName + " and " +
Review comment:
This block will always execute if were to add a default policy like the one
on SOLR-12845. We should test if there is a similar clause in the policy where
similar means that only the value of N in `replica < N` is different than
maxShardsPerNode
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1417: SOLR-12847: Auto-create a policy rule that corresponds to maxShardsPerNode
shalinmangar commented on a change in pull request #1417: SOLR-12847: Auto-create a policy rule that corresponds to maxShardsPerNode URL: https://github.com/apache/lucene-solr/pull/1417#discussion_r406736732 ## File path: solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java ## @@ -159,6 +165,36 @@ public void call(ClusterState state, ZkNodeProps message, NamedList results) thr }); } + // remove auto-created policy if this is the only collection that uses it Review comment: Maybe we should disallow people from using these auto created policies in the first place? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
