You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/09/09 00:48:16 UTC
[GitHub] [lucene-solr] murblanc opened a new pull request #1845: SOLR-14613: Autoscaling replacement using placement plugins
murblanc opened a new pull request #1845:
URL: https://github.com/apache/lucene-solr/pull/1845
Previous [PR1684](https://github.com/apache/lucene-solr/pull/1684/) was too large and too slow.
This new PR takes into account (most) comments made on the old PR.
This code is untested! It wasn't actually run in its current form. It is still work in progress but I want to make sure it is now acceptable and possibly start parallelizing work on it... (post merge?)
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul edited a comment on pull request #1845: SOLR-14613: Autoscaling replacement using placement plugins
Posted by GitBox <gi...@apache.org>.
noblepaul edited a comment on pull request #1845:
URL: https://github.com/apache/lucene-solr/pull/1845#issuecomment-693216668
>I need more guidance.
Add your APIs [here](https://github.com/apache/lucene-solr/blob/master/solr/core/src/java/org/apache/solr/handler/ClusterAPI.java)
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul commented on pull request #1845: SOLR-14613: Autoscaling replacement using placement plugins
Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1845:
URL: https://github.com/apache/lucene-solr/pull/1845#issuecomment-689320009
Looks good to me.
My only displeasure is the presence of `SolrCollection` `ReplicaType` etc. But, I can live with that
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] murblanc merged pull request #1845: SOLR-14613: Autoscaling replacement using placement plugins
Posted by GitBox <gi...@apache.org>.
murblanc merged pull request #1845:
URL: https://github.com/apache/lucene-solr/pull/1845
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on pull request #1845: SOLR-14613: Autoscaling replacement using placement plugins
Posted by GitBox <gi...@apache.org>.
murblanc commented on pull request #1845:
URL: https://github.com/apache/lucene-solr/pull/1845#issuecomment-691968650
I plan to commit this code soon, so please comment quickly if needed...
Note that this code is **disabled** by default until a user updates `/clusterprops.json` to use a placement plugin, so risk is low and limited to a few touch points: picking strategy in `Assign.createAssignStrategy()`, changes to `CollectionHandlerApi` and related classes/files for `/clusterprops.json` manipulation.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul commented on a change in pull request #1845: SOLR-14613: Autoscaling replacement using placement plugins
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1845:
URL: https://github.com/apache/lucene-solr/pull/1845#discussion_r488397392
##########
File path: solr/solrj/src/resources/apispec/cluster.Commands.json
##########
@@ -141,6 +141,21 @@
}
}
}
- }
+ },
+ "set-placement-plugin": {
Review comment:
No, we do not need any more apispecs. I'm planning to eliminate the existing ones. @murblanc please remove this change. Annotations take care of this automatically
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] thelabdude edited a comment on pull request #1845: SOLR-14613: Autoscaling replacement using placement plugins
Posted by GitBox <gi...@apache.org>.
thelabdude edited a comment on pull request #1845:
URL: https://github.com/apache/lucene-solr/pull/1845#issuecomment-690613460
Building on @noblepaul 's concern, I do think we need to reconcile the newly added interfaces in the `org.apache.solr.cluster.api` package (under solrj) with the very similar interfaces in the `org.apache.solr.cluster.placement` package added in this PR. I do realize the `org.apache.solr.cluster.api` package was introduced while the work on this PR was already underway. My intent here is to move forward with a plan we’re all comfortable with and not rehash the past.
To recap my understanding from the previous PR comments, the intent of having the Node, SolrCollection, Shard, Replica interfaces in `org.apache.solr.cluster.placement` is to avoid exposing SolrCloud internals to the plugin writer.
While I agree that is a laudable goal in general esp. for plugins, this particular framework actually needs to have access to internal information about the cluster. In other words, placing replicas is a very internal (and core) concern. The fact we're exposing this as a pluggable implementation is really for operational convenience.
Moreover, I do believe implementing a placement strategy requires most of the metadata present in collections, shards, replicas, and nodes, so I don’t know if the cost of having two representations of the same domain objects in two different places is worth the benefit it provides? I think the community needs to decide this is how we want to move forward.
So referring back to Ilan's stated goals:
1. placement plugin writing is easy,
2. implementation for this placement API is efficient, and
3. plugins do not break when Solr is refactored
I’d actually argue that goal `#1` is subjective and hard to measure. For instance, do we consider the SamplePluginAffinityReplicaPlacement impl easy to write? It seems to require a fair amount of knowledge of how Solr’s various objects interact.
I believe @murblanc has done a great job with goal `#2` in this PR.
For goal `#3`, my sense is that any refactoring probably cannot break the interfaces defined in `org.apache.solr.cluster.api` without fundamentally changing the architecture of SolrCloud, which would likely break assumptions made in the `org.apache.solr.cluster.placement` as well.
Lastly, from our slack conversation, I was only suggesting that instead of the plugin impl introducing the `AzWithNodes` that we simply formalize this as a `DataCenter` in the API, which basically does what `AzWithNodes` is doing in the code, not a big deal though ...
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1845: SOLR-14613: Autoscaling replacement using placement plugins
Posted by GitBox <gi...@apache.org>.
murblanc commented on a change in pull request #1845:
URL: https://github.com/apache/lucene-solr/pull/1845#discussion_r488557460
##########
File path: solr/solrj/src/resources/apispec/cluster.Commands.json
##########
@@ -141,6 +141,21 @@
}
}
}
- }
+ },
+ "set-placement-plugin": {
Review comment:
Can you please point me @noblepaul or @sigram to existing code in which some of the commands on a path (here `/api/cluster`) use annotations and some use apispec?
I haven't found such a mix, and given existing `/api/cluster` commands (`add-role`, `remove-role`, `set-property`, `set-obj-property`) are defined in the apispec json file, that's where I've added the two new ones.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on pull request #1845: SOLR-14613: Autoscaling replacement using placement plugins
Posted by GitBox <gi...@apache.org>.
murblanc commented on pull request #1845:
URL: https://github.com/apache/lucene-solr/pull/1845#issuecomment-690736516
Thanks @thelabdude for your long and useful comment. Let me try to give my take on this.
By saying writing plugins should be easy in point 1, I meant the boilerplate code should not get in the way and force more code lines than really necessary. It's the ability to write for example things such as `for (Replica replica : shard.replicas())`, `Collections.shuffle(nodes, new Random())` and `replica.getShard().getState()`. Such compact statements do not require any explanation and are natural for any Java programmer.
My thinking is that once the interfaces in `org.apache.solr.cluster.placement` are understood, given an understanding of the corresponding _concepts_ in Solr, writing a placement plugin is relatively accessible. If you look at `SamplePluginAffinityReplicaPlacement`, most of the complexity is not Solr related but for implementing the _business logic_ of the placement decisions (sorting, filtering etc).
For point 2 I didn't want the API to get in the way of efficiency. The current implementation of the API is definitely not optimized (no multithreading etc) but this can be changed without impact on the API or on existing plugins. I believe we reached a good place. I too prefer how the attribute fetching looks now (@noblepaul's contribution) than what I initially proposed.
Point 3 is very important. Any internal Solr interface is relatively easy to change, we have the code using it and adapt it as the interface is modified. Once we start handing these interfaces to external code (external to the lucene-solr github repo really), then changing (or not changing) them is a lot more complex and painful.
My assumption here is that placement code might be implemented by outside users to suit their specific needs, and that code might not be contributed back to the project (as opposed to the plugin I wrote and that will be the default one and a possible starting point for custom ones). Therefore, we want to be able to maintain these interfaces unchanged even if internal implementation changes. Of course if internal _concepts_ change then the interfaces will likely have to change. For example if the notion of shard leader goes away (imagine...) then of course that part of the API (be it defined on the `Replica` or on the `Shard`) doesn't really make sense anymore.
Take as example the ongoing discussions about configuration. The plugin writer should not have to change the code based on how and where we decide placement plugin configuration should live.
Last, the cluster abstractions for the placement plugins do not necessarily represent the existing cluster! In the initial (current) proposal they do (see `SimpleClusterAbstractionsImpl`), but soon we'll want to provide a forward view of how the cluster will (likely) look after known past assignment decisions are applied (these things are quite slow to happen due to the structure of `Overseer`). We had a similar separate mechanism in Autoscaling in 8x (the notion of `Session`), here we can keep the plugin writer completely agnostic of that fact, and placement decisions will simply become better (esp. under high load) once we change the internal implementation. This is to say the interfaces should really focus on the _concepts_ at play and not the current internal implementation of these concepts, as the implementations of these interfaces will drift away from their internal counterparts quite soon.
If we want to recreate a simulation environment, focusing on concepts rather than implementation simplifies things a lot as well. I guess everybody agrees on the last points made though.
All this being said, it would be better to unify cluster abstractions (and possibly other abstractions) that are to be used by external code and have a single set of abstractions (interfaces). External uses of such interfaces include placement code (this PR), event processing (see [SOLR-14749](https://issues.apache.org/jira/browse/SOLR-14749)) and possibly other external code that needs to interact with the cluster. The interfaces defined here were used to write the plugin, and were changed in the process to simplify the plugin code. I believe if we make them evolve to adapt to event processing we'll have a pretty good coverage of potential uses.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul commented on a change in pull request #1845: SOLR-14613: Autoscaling replacement using placement plugins
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1845:
URL: https://github.com/apache/lucene-solr/pull/1845#discussion_r488658448
##########
File path: solr/solrj/src/resources/apispec/cluster.Commands.json
##########
@@ -141,6 +141,21 @@
}
}
}
- }
+ },
+ "set-placement-plugin": {
Review comment:
I'll give patch
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on pull request #1845: SOLR-14613: Autoscaling replacement using placement plugins
Posted by GitBox <gi...@apache.org>.
murblanc commented on pull request #1845:
URL: https://github.com/apache/lucene-solr/pull/1845#issuecomment-693315053
> > I need more guidance.
>
> Add your APIs [here](https://github.com/apache/lucene-solr/blob/master/solr/core/src/java/org/apache/solr/handler/ClusterAPI.java)
Thanks!
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] thelabdude commented on pull request #1845: SOLR-14613: Autoscaling replacement using placement plugins
Posted by GitBox <gi...@apache.org>.
thelabdude commented on pull request #1845:
URL: https://github.com/apache/lucene-solr/pull/1845#issuecomment-690613460
Building on @noblepaul 's concern, I do think we need to reconcile the newly added interfaces in the `org.apache.solr.cluster.api` package (under solrj) with the very similar interfaces in the `org.apache.solr.cluster.placement` package added in this PR. I do realize the `org.apache.solr.cluster.api` package was introduced while the work on this PR was already underway. My intent here is to move forward with a plan we’re all comfortable with and not rehash the past.
To recap my understanding from the previous PR comments, the intent of having the Node, SolrCollection, Shard, Replica interfaces in `org.apache.solr.cluster.placement` is to avoid exposing SolrCloud internals to the plugin writer.
While I agree that is a laudable goal in general esp. for plugins, this particular framework actually needs to have access to internal information about the cluster. In other words, placing replicas is a very internal (and core) concern. The fact we're exposing this as a pluggable implementation is really for operational convenience.
Moreover, I do believe implementing a placement strategy requires most of the metadata present in collections, shards, replicas, and nodes, so I don’t know if the cost of having two representations of the same domain objects in two different places is worth the benefit it provides? I think the community needs to decide this is how we want to move forward.
So referring back to Ilan's stated goals:
1. placement plugin writing is easy,
2. implementation for this placement API is efficient, and
3. plugins do not break when Solr is refactored
I’d actually argue that goal #1 is subjective and hard to measure. For instance, do we consider the SamplePluginAffinityReplicaPlacement impl easy to write? It seems to require a fair amount of knowledge of how Solr’s various objects interact.
I believe @murblanc has done a great job with goal #2 in this PR.
For goal #3, my sense is that any refactoring probably cannot break the interfaces defined in `org.apache.solr.cluster.api` without fundamentally changing the architecture of SolrCloud, which would likely break assumptions made in the `org.apache.solr.cluster.placement` as well.
Lastly, from our slack conversation, I was only suggesting that instead of the plugin impl introducing the `AzWithNodes` that we simply formalize this as a `DataCenter` in the API, which basically does what `AzWithNodes` is doing in the code, not a big deal though ...
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] sigram commented on a change in pull request #1845: SOLR-14613: Autoscaling replacement using placement plugins
Posted by GitBox <gi...@apache.org>.
sigram commented on a change in pull request #1845:
URL: https://github.com/apache/lucene-solr/pull/1845#discussion_r486531558
##########
File path: solr/core/src/java/org/apache/solr/cluster/placement/Cluster.java
##########
@@ -0,0 +1,50 @@
+/*
+ * 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;
+
+import java.io.IOException;
+import java.util.Optional;
+import java.util.Set;
+
+/**
+ * <p>A representation of the (initial) cluster state, providing information on which nodes are part of the cluster and a way
+ * to get to more detailed info.
+ */
+public interface Cluster {
+ /**
+ * @return current set of live nodes. Never <code>null</code>, never empty (Solr wouldn't call the plugin if empty
+ * since no useful work could then be done).
+ */
+ Set<Node> getLiveNodes();
+
+ /**
+ * <p>Returns info about the given collection if one exists. Because it is not expected for plugins to request info about
+ * a large number of collections, requests can only be made one by one.
+ *
+ * <p>This is also the reason we do not return a {@link java.util.Map} or {@link Set} of {@link SolrCollection}'s here: it would be
+ * wasteful to fetch all data and fill such a map when plugin code likely needs info about at most one or two collections.
+ */
+ Optional<SolrCollection> getCollection(String collectionName) throws IOException;
+
+ /**
+ * <p>Allows getting names of all {@link SolrCollection}'s present in the cluster.
+ *
+ * <p><b>WARNING:</b> this call will be extremely inefficient on large clusters. Usage is discouraged.
+ */
+ Set<String> getAllCollectionNames();
Review comment:
IIRC at some point we've considered using an Iterator<String> here instead.
##########
File path: solr/core/src/java/org/apache/solr/cluster/placement/Node.java
##########
@@ -0,0 +1,25 @@
+/*
+ * 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;
+
+/**
+ * Representation of a SolrCloud node or server in the SolrCloud cluster.
+ */
+public interface Node {
Review comment:
So ... given that there's already a `SolrNode` interface in master, which already provides isolation from implementation details, shouldn't we use that here? The same applies to `SolrCollection` and `ShardReplica`.
##########
File path: solr/core/src/java/org/apache/solr/cluster/placement/SolrCollection.java
##########
@@ -0,0 +1,81 @@
+/*
+ * 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;
+
+import java.util.Iterator;
+
+/**
+ * Represents a Collection in SolrCloud (unrelated to {@link java.util.Collection} that uses the nicer name).
+ */
+public interface SolrCollection {
Review comment:
See my other comments about merging this with the existing `SolrCollection`.
##########
File path: solr/solrj/src/resources/apispec/cluster.Commands.json
##########
@@ -141,6 +141,21 @@
}
}
}
- }
+ },
+ "set-placement-plugin": {
Review comment:
@noblepaul do we still need these awful json apispecs if we use the V2 API annotations?
##########
File path: solr/core/src/java/org/apache/solr/cluster/placement/Replica.java
##########
@@ -0,0 +1,56 @@
+/*
+ * 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;
+
+/**
+ * An instantiation (or one of the copies) of a given {@link Shard} of a given {@link SolrCollection}.
+ * Objects of this type are returned by the Solr framework to the plugin, they are not directly built by the plugin. When the
+ * plugin wants to add a replica it goes through appropriate method in {@link PlacementPlanFactory}).
+ */
+public interface Replica {
Review comment:
This should be merged with the existing `ShardReplica` to avoid creating separate abstractions for each subsystem.
##########
File path: solr/core/src/java/org/apache/solr/cluster/placement/ReplicaPlacement.java
##########
@@ -0,0 +1,45 @@
+/*
+ * 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;
+
+/**
+ * <p>Placement decision for a single {@link Replica}. Note this placement decision is used as part of a {@link PlacementPlan},
+ * it does not directly lead to the plugin code getting a corresponding {@link Replica} instance, nor does it require the
+ * plugin to provide a {@link Shard} instance (the plugin code gets such instances for existing replicas and shards in the
+ * cluster but does not create them directly for adding new replicas for new or existing shards).
+ *
+ * <p>Captures the {@link Shard} (via the shard name), {@link Node} and {@link Replica.ReplicaType} of a Replica to be created.
+ *
+ * <p>TODO: discuss (before merge) if this interface really needs to allow access to the data captured in an instance or if calling {@link PlacementPlanFactory#createReplicaPlacement(String, Node, Replica.ReplicaType)} is sufficient for plugin code.
+ */
+public interface ReplicaPlacement {
+ /**
+ * @return the name of the {@link Shard} for which the replica should be created
+ */
+ String getShardName();
Review comment:
Should we also have the collection name here for completeness?
##########
File path: solr/core/src/java/org/apache/solr/cluster/placement/PlacementRequest.java
##########
@@ -0,0 +1,70 @@
+/*
+ * 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;
+
+import java.util.Set;
+
+/**
+ * A cluster related placement request that Solr asks a {@link PlacementPlugin} plugin to resolve and compute a
+ * {@link PlacementPlan} placing one or more {@link Replica}'s of one or more {@link Shard}'s of an existing {@link SolrCollection}.
+ * The shard might or might not already exist, plugin code can easily find out by calling {@link SolrCollection#getShard(String)}
+ * with the shard name(s) returned by {@link #getShardNames()}.
+ *
+ * <p>The set of {@link Node}s on which the replicas should be placed
+ * is specified (defaults to being equal to the set returned by {@link Cluster#getLiveNodes()}).
+ */
+public interface PlacementRequest {
+ /**
+ * The {@link SolrCollection} to add {@link Replica}(s) to.
+ */
+ SolrCollection getCollection();
+
+ /**
+ * <p>Shard name(s) for which new replicas placement should be computed. The shard(s) might exist or not (that's why this
+ * method returns a {@link Set} of {@link String}'s and not directly a set of {@link Shard} instances).
+ *
+ * <p>Note the Collection API allows specifying the shard name or a {@code _route_} parameter. The Solr implementation will
+ * convert either specification into the relevant shard name so the plugin code doesn't have to worry about this.
+ */
+ Set<String> getShardNames();
+
+ /**
+ * <p>Replicas should only be placed on nodes in the set returned by this method.
+ *
+ * <p>When Collection API calls do not specify a specific set of target nodes, replicas can be placed on any live node of
+ * the cluster. In such cases, this set will be equal to the set of all live nodes. The plugin placement code does not
+ * need to worry (or care) if a set of nodes was explicitly specified or not.
+ *
+ * @return never {@code null} and never empty set (if that set was to be empty for any reason, no placement would be
+ * possible and the Solr infrastructure driving the plugin code would detect the error itself rather than calling the plugin).
+ */
+ Set<Node> getTargetNodes();
+
+ /**
+ * Returns the number of replica to create that is returned by the corresponding method {@link #getCountNrtReplicas()},
+ * {@link #getCountTlogReplicas()} or {@link #getCountPullReplicas()}. Might delete the other three.
+ */
+ int getCountReplicasToCreate(Replica.ReplicaType replicaType);
Review comment:
I slightly prefer this method, as it allows us to modify available replica types without changing the interface.
##########
File path: solr/core/src/java/org/apache/solr/cluster/placement/Shard.java
##########
@@ -0,0 +1,57 @@
+/*
+ * 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;
+
+import java.util.Iterator;
+
+/**
+ * Shard in a {@link SolrCollection}, i.e. a subset of the data indexed in that collection.
+ */
+public interface Shard {
Review comment:
Similar to the other top-level abstractions, this interface should be merged with the existing `Shard` interface, after resolving the main differences (the use of iterators vs. `SimpleMap`, what getters we absolutely need in this interface, etc).
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul commented on a change in pull request #1845: SOLR-14613: Autoscaling replacement using placement plugins
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1845:
URL: https://github.com/apache/lucene-solr/pull/1845#discussion_r488397392
##########
File path: solr/solrj/src/resources/apispec/cluster.Commands.json
##########
@@ -141,6 +141,21 @@
}
}
}
- }
+ },
+ "set-placement-plugin": {
Review comment:
NO, we do not need any more apispecs. I'm planning to eliminate the existing ones
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on pull request #1845: SOLR-14613: Autoscaling replacement using placement plugins
Posted by GitBox <gi...@apache.org>.
murblanc commented on pull request #1845:
URL: https://github.com/apache/lucene-solr/pull/1845#issuecomment-692633810
I need more guidance. I've implemented the new commands the way existing ones under the same path are implemented. I don't know how to "switch to annotations". Can you please point me to developer documentation that would help me 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul commented on pull request #1845: SOLR-14613: Autoscaling replacement using placement plugins
Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1845:
URL: https://github.com/apache/lucene-solr/pull/1845#issuecomment-692477612
@murblanc
Please switch to annotations before you commit this
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1845: SOLR-14613: Autoscaling replacement using placement plugins
Posted by GitBox <gi...@apache.org>.
murblanc commented on a change in pull request #1845:
URL: https://github.com/apache/lucene-solr/pull/1845#discussion_r488566052
##########
File path: solr/solrj/src/resources/apispec/cluster.Commands.json
##########
@@ -141,6 +141,21 @@
}
}
}
- }
+ },
+ "set-placement-plugin": {
Review comment:
Or, put differently (if there's no simple way to use annotations for the two new commands): when the existing 4 commands are migrated, migrating with them the two new ones is likely not going to make the task any harder.
Keeping the new definitions in apispecs would therefore make sense for now for the sake of simplicity.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul commented on pull request #1845: SOLR-14613: Autoscaling replacement using placement plugins
Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1845:
URL: https://github.com/apache/lucene-solr/pull/1845#issuecomment-693216668
>I need more guidance.
#1878 I'm committing that soon
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org