[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r471388275
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/PlacementPlanFactory.java
##
@@ -0,0 +1,52 @@
+/*
+ * 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;
+
+/**
+ * Allows plugins to create {@link PlacementPlan}s telling the Solr layer
where to create replicas following the processing of
+ * a {@link PlacementRequest}. The Solr layer can (and will) check that the
{@link PlacementPlan} conforms to the {@link PlacementRequest} (and
+ * if it does not, the requested operation will fail).
+ */
+public interface PlacementPlanFactory {
+ /**
+ * Creates a {@link PlacementPlan} for adding a new collection and its
replicas.
+ *
+ * This is in support of {@link
org.apache.solr.cloud.api.collections.CreateCollectionCmd}.
+ */
+ PlacementPlan
createPlacementPlanNewCollection(CreateNewCollectionPlacementRequest request,
String CollectionName, Set replicaPlacements);
+
+ /**
+ * Creates a {@link PlacementPlan} for adding replicas to a given shard
of an existing collection.
+ *
+ * This is in support (directly or indirectly) of {@link
org.apache.solr.cloud.api.collections.AddReplicaCmd},
+ * {@link org.apache.solr.cloud.api.collections.CreateShardCmd}, {@link
org.apache.solr.cloud.api.collections.ReplaceNodeCmd},
+ * {@link org.apache.solr.cloud.api.collections.MoveReplicaCmd}, {@link
org.apache.solr.cloud.api.collections.SplitShardCmd},
+ * {@link org.apache.solr.cloud.api.collections.RestoreCmd} and {@link
org.apache.solr.cloud.api.collections.MigrateCmd}.
+ * (as well as of {@link
org.apache.solr.cloud.api.collections.CreateCollectionCmd} in the specific case
of
+ * {@link
org.apache.solr.common.params.CollectionAdminParams#WITH_COLLECTION} but this
should be removed shortly and
+ * the section in parentheses of this comment should be removed when the
{@code withCollection} javadoc link appears broken).
+ */
+ PlacementPlan createPlacementPlanAddReplicas(AddReplicasPlacementRequest
request, String CollectionName, Set replicaPlacements);
Review comment:
I meant above I'm unifying the creation requests as well as the
placement plans.
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r471387128
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/PlacementPlanFactory.java
##
@@ -0,0 +1,52 @@
+/*
+ * 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;
+
+/**
+ * Allows plugins to create {@link PlacementPlan}s telling the Solr layer
where to create replicas following the processing of
+ * a {@link PlacementRequest}. The Solr layer can (and will) check that the
{@link PlacementPlan} conforms to the {@link PlacementRequest} (and
+ * if it does not, the requested operation will fail).
+ */
+public interface PlacementPlanFactory {
+ /**
+ * Creates a {@link PlacementPlan} for adding a new collection and its
replicas.
+ *
+ * This is in support of {@link
org.apache.solr.cloud.api.collections.CreateCollectionCmd}.
+ */
+ PlacementPlan
createPlacementPlanNewCollection(CreateNewCollectionPlacementRequest request,
String CollectionName, Set replicaPlacements);
+
+ /**
+ * Creates a {@link PlacementPlan} for adding replicas to a given shard
of an existing collection.
+ *
+ * This is in support (directly or indirectly) of {@link
org.apache.solr.cloud.api.collections.AddReplicaCmd},
+ * {@link org.apache.solr.cloud.api.collections.CreateShardCmd}, {@link
org.apache.solr.cloud.api.collections.ReplaceNodeCmd},
+ * {@link org.apache.solr.cloud.api.collections.MoveReplicaCmd}, {@link
org.apache.solr.cloud.api.collections.SplitShardCmd},
+ * {@link org.apache.solr.cloud.api.collections.RestoreCmd} and {@link
org.apache.solr.cloud.api.collections.MigrateCmd}.
+ * (as well as of {@link
org.apache.solr.cloud.api.collections.CreateCollectionCmd} in the specific case
of
+ * {@link
org.apache.solr.common.params.CollectionAdminParams#WITH_COLLECTION} but this
should be removed shortly and
+ * the section in parentheses of this comment should be removed when the
{@code withCollection} javadoc link appears broken).
+ */
+ PlacementPlan createPlacementPlanAddReplicas(AddReplicasPlacementRequest
request, String CollectionName, Set replicaPlacements);
Review comment:
@sigram I will indeed unify these two requests to a single one. When
creating a new collection, the command currently first creates the collection
ZK node then does the Assign request, and there's already a DocCollection
instance available. Therefore no need to distinguish new collection creation
from adding replicas to an existing one. Given the plugin will have to deal
with the more complex task of adding replicas (compared to creating a new
collection, since positions of existing replicas have to be taken into
account), there will be no added work there (minor loss of efficiency for
computing placement for new collections, likely will not even be measurable).
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r471142386
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/AddReplicasPlacementRequest.java
##
@@ -0,0 +1,72 @@
+/*
+ * 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;
+
+/**
+ * Request passed by Solr to a {@link PlacementPlugin} to compute placement
for one or more {@link Replica}'s for 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 using {@link SolrCollection#getShards()}
+ * and verifying if the shard name(s) from {@link #getShardNames()} are there.
+ *
+ * As opposed to {@link CreateNewCollectionPlacementRequest}, 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()}).
+ *
+ * There is no extension between this interface and {@link
CreateNewCollectionPlacementRequest} in either direction
+ * or from a common ancestor for readability. An ancestor could make sense and
would be an "abstract interface" not intended
+ * to be implemented directly, but this does not exist in Java.
+ *
+ * Plugin code would likely treat the two types of requests differently
since here existing {@link Replica}'s must be taken
+ * into account for placement whereas in {@link
CreateNewCollectionPlacementRequest} no {@link Replica}'s are assumed to exist.
+ */
+public interface AddReplicasPlacementRequest extends PlacementRequest {
+ /**
+ * The {@link SolrCollection} to add {@link Replica}(s) to. The replicas are
to be added to a shard that might or might
+ * not yet exist when the plugin's {@link PlacementPlugin#computePlacement}
is called.
+ */
+ SolrCollection getCollection();
Review comment:
@noblepaul you requested a change here. Do you mind elaborating based on
my reply to your request?
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r469455229
##
File path: solr/core/src/java/org/apache/solr/cluster/placement/Cluster.java
##
@@ -0,0 +1,53 @@
+/*
+ * 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;
+
+/**
+ * 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.
+ *
+ * This instance can also be used as a {@link PropertyValueSource} if
{@link PropertyKey}'s need to be specified with
+ * a global cluster target.
+ */
+public interface Cluster extends PropertyValueSource {
+ /**
+ * @return current set of live nodes. Never null, never empty
(Solr wouldn't call the plugin if empty
+ * since no useful work could then be done).
+ */
+ Set getLiveNodes();
+
+ /**
+ * 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.
+ *
+ * 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 getCollection(String collectionName) throws
IOException;
+
+ /**
+ * Allows getting all {@link SolrCollection} present in the cluster.
+ *
+ * WARNING: this call might be extremely inefficient on large
clusters. Usage is discouraged.
+ */
+ Set getAllCollections();
Review comment:
Side note: I need (and plan to eventually make) SolrCloud to scale to
hundreds of thousand collections. Anything that’s in O(n) or worse in number of
collections will not fly for this scale. Implementations can be inefficient
(and can change) but let’s try to keep interfaces efficient.
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r469455229
##
File path: solr/core/src/java/org/apache/solr/cluster/placement/Cluster.java
##
@@ -0,0 +1,53 @@
+/*
+ * 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;
+
+/**
+ * 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.
+ *
+ * This instance can also be used as a {@link PropertyValueSource} if
{@link PropertyKey}'s need to be specified with
+ * a global cluster target.
+ */
+public interface Cluster extends PropertyValueSource {
+ /**
+ * @return current set of live nodes. Never null, never empty
(Solr wouldn't call the plugin if empty
+ * since no useful work could then be done).
+ */
+ Set getLiveNodes();
+
+ /**
+ * 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.
+ *
+ * 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 getCollection(String collectionName) throws
IOException;
+
+ /**
+ * Allows getting all {@link SolrCollection} present in the cluster.
+ *
+ * WARNING: this call might be extremely inefficient on large
clusters. Usage is discouraged.
+ */
+ Set getAllCollections();
Review comment:
Side note: I need (and plan to eventually make) SolrCloud to scale to
hundred of thousand collections. Anything that’s in O(n) or worse in number of
collections will not fly for this scale. Implementations can be inefficient
(and can change) but let’s try to keep interfaces efficient.
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r469452206
##
File path: solr/core/src/java/org/apache/solr/cluster/placement/Cluster.java
##
@@ -0,0 +1,53 @@
+/*
+ * 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;
+
+/**
+ * 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.
+ *
+ * This instance can also be used as a {@link PropertyValueSource} if
{@link PropertyKey}'s need to be specified with
+ * a global cluster target.
+ */
+public interface Cluster extends PropertyValueSource {
+ /**
+ * @return current set of live nodes. Never null, never empty
(Solr wouldn't call the plugin if empty
+ * since no useful work could then be done).
+ */
+ Set getLiveNodes();
+
+ /**
+ * 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.
+ *
+ * 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 getCollection(String collectionName) throws
IOException;
+
+ /**
+ * Allows getting all {@link SolrCollection} present in the cluster.
+ *
+ * WARNING: this call might be extremely inefficient on large
clusters. Usage is discouraged.
+ */
+ Set getAllCollections();
Review comment:
Ok. Can return names here.
Still didn’t get what’s the use case for this method though. I’d assume if
we need to fetch collection names we don’t know, we might want to fetch names
that verify a given pattern. Maybe make this method accept some form of
filtering? (Something that can be implemented efficiently if we ever want to,
not an “accept” function that forces iterating over all collection names
anyway).
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r469449162
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/PlacementPlugin.java
##
@@ -0,0 +1,41 @@
+/*
+ * 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;
+
+/**
+ * Implemented by external plugins to control replica placement and movement
on the search cluster (as well as other things
+ * such as cluster elasticity?) when cluster changes are required (initiated
elsewhere, most likely following a Collection
+ * API call).
+ */
+public interface PlacementPlugin {
Review comment:
Either constructor convention or - I prefer but not sure how popular it
is in the Solr codebase - when configuring a plugin what is added to the Solr
config is a plugin factory and that factory is called to get plugin instances.
One more interface but cleaner code.
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r469447213
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/PlacementPlugin.java
##
@@ -0,0 +1,41 @@
+/*
+ * 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;
+
+/**
+ * Implemented by external plugins to control replica placement and movement
on the search cluster (as well as other things
+ * such as cluster elasticity?) when cluster changes are required (initiated
elsewhere, most likely following a Collection
+ * API call).
+ */
+public interface PlacementPlugin {
Review comment:
Perfectly fine with me. The contract will be for the plugin to provide a
constructor accepting config to create a new Plugin instance, then that
instance will be called for each placement computation (and if the plugin
doesn’t care about config, no arg constructor would be fine). Multiple plugin
instances might be in use at the same time if so called by Solr (config changes
or other reasons).
This means the plugin instance must be reentrant and its member variables
can’t directly be used as if specific to a single computation. Not a big deal,
plugin implementor can delegate internally to an instance of another class if
they want that option.
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r468418051
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/plugins/SamplePluginMinimizeCores.java
##
@@ -0,0 +1,132 @@
+/*
+ * 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.plugins;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Set;
+import java.util.Map;
+
+import com.google.common.collect.Ordering;
+import com.google.common.collect.TreeMultimap;
+import org.apache.solr.cluster.placement.Cluster;
+import org.apache.solr.cluster.placement.CoresCountPropertyValue;
+import org.apache.solr.cluster.placement.CreateNewCollectionPlacementRequest;
+import org.apache.solr.cluster.placement.Node;
+import org.apache.solr.cluster.placement.PlacementException;
+import org.apache.solr.cluster.placement.PlacementPlugin;
+import org.apache.solr.cluster.placement.PropertyKey;
+import org.apache.solr.cluster.placement.PropertyKeyFactory;
+import org.apache.solr.cluster.placement.PropertyValue;
+import org.apache.solr.cluster.placement.PropertyValueFetcher;
+import org.apache.solr.cluster.placement.Replica;
+import org.apache.solr.cluster.placement.ReplicaPlacement;
+import org.apache.solr.cluster.placement.PlacementRequest;
+import org.apache.solr.cluster.placement.PlacementPlan;
+import org.apache.solr.cluster.placement.PlacementPlanFactory;
+import org.apache.solr.common.util.SuppressForbidden;
+
+/**
+ * Implements placing replicas to minimize number of cores per {@link Node},
while not placing two replicas of the same
+ * shard on the same node.
+ *
+ * TODO: code not tested and never run, there are no implementation yet for
used interfaces
+ */
+public class SamplePluginMinimizeCores implements PlacementPlugin {
+
+ @SuppressForbidden(reason = "Ordering.arbitrary() has no equivalent in
Comparator class. Rather reuse than copy.")
+ public PlacementPlan computePlacement(Cluster cluster, PlacementRequest
placementRequest, PropertyKeyFactory propertyFactory,
+PropertyValueFetcher propertyFetcher,
PlacementPlanFactory placementPlanFactory) throws PlacementException {
+// This plugin only supports Creating a collection.
+if (!(placementRequest instanceof CreateNewCollectionPlacementRequest)) {
+ throw new PlacementException("This toy plugin only supports creating
collections");
+}
+
+final CreateNewCollectionPlacementRequest reqCreateCollection =
(CreateNewCollectionPlacementRequest) placementRequest;
+
+final int totalReplicasPerShard =
reqCreateCollection.getNrtReplicationFactor() +
+reqCreateCollection.getTlogReplicationFactor() +
reqCreateCollection.getPullReplicationFactor();
+
+if (cluster.getLiveNodes().size() < totalReplicasPerShard) {
+ throw new PlacementException("Cluster size too small for number of
replicas per shard");
+}
+
+// Get number of cores on each Node
+TreeMultimap nodesByCores =
TreeMultimap.create(Comparator.naturalOrder(), Ordering.arbitrary());
+
+// Get the number of cores on each node and sort the nodes by increasing
number of cores
+for (Node node : cluster.getLiveNodes()) {
+ // TODO: redo this. It is potentially less efficient to call
propertyFetcher.getProperties() multiple times rather than once
+ final PropertyKey coresCountPropertyKey =
propertyFactory.createCoreCountKey(node);
+ Map propMap =
propertyFetcher.fetchProperties(Collections.singleton(coresCountPropertyKey));
+ PropertyValue returnedValue = propMap.get(coresCountPropertyKey);
+ if (returnedValue == null) {
+throw new PlacementException("Can't get number of cores in " + node);
Review comment:
Yes. This is a PoC implementation, not a real one. Used it to guide me
in building the interfaces.
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
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r468415587
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/ReplicaPlacement.java
##
@@ -0,0 +1,29 @@
+/*
+ * 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;
+
+/**
+ * Placement decision for a single {@link Replica}. Note this placement
decision is used as part of a {@link WorkOrder},
+ * 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).
+ *
+ * Captures the {@link Shard} (via the shard name), {@link Node} and {@link
Replica.ReplicaType} of a Replica to be created.
+ */
+public interface ReplicaPlacement {
Review comment:
Solr code will not be using these interfaces. This will be clearer once
I'm back from vacation and implement the solr side of things. Solr will only be
using the concrete implementations of the interfaces so will have access to
everything it needs.
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r468412029
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/plugins/SamplePluginMinimizeCores.java
##
@@ -0,0 +1,132 @@
+/*
+ * 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.plugins;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Set;
+import java.util.Map;
+
+import com.google.common.collect.Ordering;
+import com.google.common.collect.TreeMultimap;
+import org.apache.solr.cluster.placement.Cluster;
+import org.apache.solr.cluster.placement.CoresCountPropertyValue;
+import org.apache.solr.cluster.placement.CreateNewCollectionPlacementRequest;
+import org.apache.solr.cluster.placement.Node;
+import org.apache.solr.cluster.placement.PlacementException;
+import org.apache.solr.cluster.placement.PlacementPlugin;
+import org.apache.solr.cluster.placement.PropertyKey;
+import org.apache.solr.cluster.placement.PropertyKeyFactory;
+import org.apache.solr.cluster.placement.PropertyValue;
+import org.apache.solr.cluster.placement.PropertyValueFetcher;
+import org.apache.solr.cluster.placement.Replica;
+import org.apache.solr.cluster.placement.ReplicaPlacement;
+import org.apache.solr.cluster.placement.PlacementRequest;
+import org.apache.solr.cluster.placement.PlacementPlan;
+import org.apache.solr.cluster.placement.PlacementPlanFactory;
+import org.apache.solr.common.util.SuppressForbidden;
+
+/**
+ * Implements placing replicas to minimize number of cores per {@link Node},
while not placing two replicas of the same
+ * shard on the same node.
+ *
+ * TODO: code not tested and never run, there are no implementation yet for
used interfaces
+ */
+public class SamplePluginMinimizeCores implements PlacementPlugin {
+
+ @SuppressForbidden(reason = "Ordering.arbitrary() has no equivalent in
Comparator class. Rather reuse than copy.")
+ public PlacementPlan computePlacement(Cluster cluster, PlacementRequest
placementRequest, PropertyKeyFactory propertyFactory,
+PropertyValueFetcher propertyFetcher,
PlacementPlanFactory placementPlanFactory) throws PlacementException {
+// This plugin only supports Creating a collection.
+if (!(placementRequest instanceof CreateNewCollectionPlacementRequest)) {
+ throw new PlacementException("This toy plugin only supports creating
collections");
+}
+
+final CreateNewCollectionPlacementRequest reqCreateCollection =
(CreateNewCollectionPlacementRequest) placementRequest;
+
+final int totalReplicasPerShard =
reqCreateCollection.getNrtReplicationFactor() +
+reqCreateCollection.getTlogReplicationFactor() +
reqCreateCollection.getPullReplicationFactor();
+
+if (cluster.getLiveNodes().size() < totalReplicasPerShard) {
+ throw new PlacementException("Cluster size too small for number of
replicas per shard");
+}
+
+// Get number of cores on each Node
+TreeMultimap nodesByCores =
TreeMultimap.create(Comparator.naturalOrder(), Ordering.arbitrary());
Review comment:
Strong typing was one requirement (and strong typing is not only return
type, it's also nice names for methods to access these values), the other one
was efficiency: being able to fetch ALL needed properties from a remote Node at
once (i.e. not requesting metrics first and system properties second which
would force the Solr side framework to do two round trips - caching and
optimizations aside).
That's why we have the notion of property key, to be able to "accumulate"
the properties to fetch, then we have asingle interface per property, allowing
clean access to the value (for cores it's the call
coresCountPropertyValue.getCoresCount() line 85 below).
The "complexity" of the structure here is the plug-in implementation, not
the API, using sorted data structures to compute a placement quickly. This
should be compared (in complexity) to the previous approach of generating all
possible placements and comparing them.
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r468405452
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/AddReplicasPlacementRequest.java
##
@@ -0,0 +1,72 @@
+/*
+ * 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;
+
+/**
+ * Request passed by Solr to a {@link PlacementPlugin} to compute placement
for one or more {@link Replica}'s for 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 using {@link SolrCollection#getShards()}
+ * and verifying if the shard name(s) from {@link #getShardNames()} are there.
+ *
+ * As opposed to {@link CreateNewCollectionPlacementRequest}, 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()}).
+ *
+ * There is no extension between this interface and {@link
CreateNewCollectionPlacementRequest} in either direction
+ * or from a common ancestor for readability. An ancestor could make sense and
would be an "abstract interface" not intended
+ * to be implemented directly, but this does not exist in Java.
+ *
+ * Plugin code would likely treat the two types of requests differently
since here existing {@link Replica}'s must be taken
+ * into account for placement whereas in {@link
CreateNewCollectionPlacementRequest} no {@link Replica}'s are assumed to exist.
+ */
+public interface AddReplicasPlacementRequest extends PlacementRequest {
+ /**
+ * The {@link SolrCollection} to add {@link Replica}(s) to. The replicas are
to be added to a shard that might or might
+ * not yet exist when the plugin's {@link PlacementPlugin#computePlacement}
is called.
+ */
+ SolrCollection getCollection();
Review comment:
What's the value of returning the name rather than the object itself? If
the plug-in requests info on a collection it's to access that info I assume, so
returning an object saves an additional step of "get Collection From Name" or
similar call in plugin code.
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r468403391
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/PlacementPlanFactory.java
##
@@ -0,0 +1,52 @@
+/*
+ * 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;
+
+/**
+ * Allows plugins to create {@link PlacementPlan}s telling the Solr layer
where to create replicas following the processing of
+ * a {@link PlacementRequest}. The Solr layer can (and will) check that the
{@link PlacementPlan} conforms to the {@link PlacementRequest} (and
+ * if it does not, the requested operation will fail).
+ */
+public interface PlacementPlanFactory {
+ /**
+ * Creates a {@link PlacementPlan} for adding a new collection and its
replicas.
+ *
+ * This is in support of {@link
org.apache.solr.cloud.api.collections.CreateCollectionCmd}.
+ */
+ PlacementPlan
createPlacementPlanNewCollection(CreateNewCollectionPlacementRequest request,
String CollectionName, Set replicaPlacements);
+
+ /**
+ * Creates a {@link PlacementPlan} for adding replicas to a given shard
of an existing collection.
+ *
+ * This is in support (directly or indirectly) of {@link
org.apache.solr.cloud.api.collections.AddReplicaCmd},
+ * {@link org.apache.solr.cloud.api.collections.CreateShardCmd}, {@link
org.apache.solr.cloud.api.collections.ReplaceNodeCmd},
+ * {@link org.apache.solr.cloud.api.collections.MoveReplicaCmd}, {@link
org.apache.solr.cloud.api.collections.SplitShardCmd},
+ * {@link org.apache.solr.cloud.api.collections.RestoreCmd} and {@link
org.apache.solr.cloud.api.collections.MigrateCmd}.
+ * (as well as of {@link
org.apache.solr.cloud.api.collections.CreateCollectionCmd} in the specific case
of
+ * {@link
org.apache.solr.common.params.CollectionAdminParams#WITH_COLLECTION} but this
should be removed shortly and
+ * the section in parentheses of this comment should be removed when the
{@code withCollection} javadoc link appears broken).
+ */
+ PlacementPlan createPlacementPlanAddReplicas(AddReplicasPlacementRequest
request, String CollectionName, Set replicaPlacements);
Review comment:
That was my initial intention but then I switched to the factory
approach. We can expose all accessors in PlacementPlan and let the plug-in
build instances of it as it wishes. Maybe we should expose all accessors
anyway, but I thought rather than letting each plug-in implement these
interfaces again it's simpler to provide a factory. Also I saw no real added
value from the plug-in perspective to have its own instance type.
A provided factory allows to introduce constraints on acceptable parameters
in a programmer friendly way (what a method accepts, different method names),
if Solr side accepts any instance it would have to do some checks at runtime.
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r468398576
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/PlacementPlugin.java
##
@@ -0,0 +1,41 @@
+/*
+ * 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;
+
+/**
+ * Implemented by external plugins to control replica placement and movement
on the search cluster (as well as other things
+ * such as cluster elasticity?) when cluster changes are required (initiated
elsewhere, most likely following a Collection
+ * API call).
+ */
+public interface PlacementPlugin {
Review comment:
So what do you suggest here and how would the plug-in code deal with it?
And how different it would be from the plug-in storing/caching whatever
processing it has done in static variables?
I thought about this and we can introduce a concept of "Config session"
during which which the config doesn't change.
The plug-in would implement such a newConfigSession method that would accept
a config and return an instance of the compute plug-in. That instance would
then be called by the framework to do computation. When the framework sees a
change of config, it would call again newConfigSession. By using the
appropriate classes without storing references in funny places, garbage
collection would clean up old configs...
Just having a Solr callback into the existing plug-in passing config is not
elegant IMO: forces to synchronize access to config (JMM) and to deal with
first compute request coming before first config + race conditions between the
two.
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r468065465
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/PlacementPlugin.java
##
@@ -0,0 +1,41 @@
+/*
+ * 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;
+
+/**
+ * Implemented by external plugins to control replica placement and movement
on the search cluster (as well as other things
+ * such as cluster elasticity?) when cluster changes are required (initiated
elsewhere, most likely following a Collection
+ * API call).
+ */
+public interface PlacementPlugin {
Review comment:
I believe we should let the plug-in manage this type of requirements
rather than try to control it by the timing of when configs are passed. If
there are licences to check the plug-in should cache the result and only
confirm each time?
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r467947455
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/PlacementPlanFactory.java
##
@@ -0,0 +1,52 @@
+/*
+ * 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;
+
+/**
+ * Allows plugins to create {@link PlacementPlan}s telling the Solr layer
where to create replicas following the processing of
+ * a {@link PlacementRequest}. The Solr layer can (and will) check that the
{@link PlacementPlan} conforms to the {@link PlacementRequest} (and
+ * if it does not, the requested operation will fail).
+ */
+public interface PlacementPlanFactory {
+ /**
+ * Creates a {@link PlacementPlan} for adding a new collection and its
replicas.
+ *
+ * This is in support of {@link
org.apache.solr.cloud.api.collections.CreateCollectionCmd}.
+ */
+ PlacementPlan
createPlacementPlanNewCollection(CreateNewCollectionPlacementRequest request,
String CollectionName, Set replicaPlacements);
+
+ /**
+ * Creates a {@link PlacementPlan} for adding replicas to a given shard
of an existing collection.
+ *
+ * This is in support (directly or indirectly) of {@link
org.apache.solr.cloud.api.collections.AddReplicaCmd},
+ * {@link org.apache.solr.cloud.api.collections.CreateShardCmd}, {@link
org.apache.solr.cloud.api.collections.ReplaceNodeCmd},
+ * {@link org.apache.solr.cloud.api.collections.MoveReplicaCmd}, {@link
org.apache.solr.cloud.api.collections.SplitShardCmd},
+ * {@link org.apache.solr.cloud.api.collections.RestoreCmd} and {@link
org.apache.solr.cloud.api.collections.MigrateCmd}.
+ * (as well as of {@link
org.apache.solr.cloud.api.collections.CreateCollectionCmd} in the specific case
of
+ * {@link
org.apache.solr.common.params.CollectionAdminParams#WITH_COLLECTION} but this
should be removed shortly and
+ * the section in parentheses of this comment should be removed when the
{@code withCollection} javadoc link appears broken).
+ */
+ PlacementPlan createPlacementPlanAddReplicas(AddReplicasPlacementRequest
request, String CollectionName, Set replicaPlacements);
+
+ /**
+ * Creates a {@link ReplicaPlacement} needed to be passed to some/all {@link
PlacementPlan} factory methods.
+ */
+ ReplicaPlacement createReplicaPlacement(String shardName, Node node,
Replica.ReplicaType replicaType);
Review comment:
That’s the way for the plugin to build the replica placements it has
decided in order to pass the set to the appropriate PlacementPlanFactory method.
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r467945960
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/PlacementPlanFactory.java
##
@@ -0,0 +1,52 @@
+/*
+ * 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;
+
+/**
+ * Allows plugins to create {@link PlacementPlan}s telling the Solr layer
where to create replicas following the processing of
+ * a {@link PlacementRequest}. The Solr layer can (and will) check that the
{@link PlacementPlan} conforms to the {@link PlacementRequest} (and
+ * if it does not, the requested operation will fail).
+ */
+public interface PlacementPlanFactory {
+ /**
+ * Creates a {@link PlacementPlan} for adding a new collection and its
replicas.
+ *
+ * This is in support of {@link
org.apache.solr.cloud.api.collections.CreateCollectionCmd}.
+ */
+ PlacementPlan
createPlacementPlanNewCollection(CreateNewCollectionPlacementRequest request,
String CollectionName, Set replicaPlacements);
+
+ /**
+ * Creates a {@link PlacementPlan} for adding replicas to a given shard
of an existing collection.
+ *
+ * This is in support (directly or indirectly) of {@link
org.apache.solr.cloud.api.collections.AddReplicaCmd},
+ * {@link org.apache.solr.cloud.api.collections.CreateShardCmd}, {@link
org.apache.solr.cloud.api.collections.ReplaceNodeCmd},
+ * {@link org.apache.solr.cloud.api.collections.MoveReplicaCmd}, {@link
org.apache.solr.cloud.api.collections.SplitShardCmd},
+ * {@link org.apache.solr.cloud.api.collections.RestoreCmd} and {@link
org.apache.solr.cloud.api.collections.MigrateCmd}.
+ * (as well as of {@link
org.apache.solr.cloud.api.collections.CreateCollectionCmd} in the specific case
of
+ * {@link
org.apache.solr.common.params.CollectionAdminParams#WITH_COLLECTION} but this
should be removed shortly and
+ * the section in parentheses of this comment should be removed when the
{@code withCollection} javadoc link appears broken).
+ */
+ PlacementPlan createPlacementPlanAddReplicas(AddReplicasPlacementRequest
request, String CollectionName, Set replicaPlacements);
Review comment:
Is the move replica command picking up the destination or is the
destination specified in the API call? If the latter, there will be no call to
the placement plugin.
And if the former, the fact that no files are to be moved is relatively
transparent to the plugin. The plugin doesn’t do any work but just tells Solr
where to put things. Solr code would then either create or move depending on
what command it was executing.
The only difference could be move placement computation (if there’s one)
should take into account the lower load on the source node (since replicas will
be moved off of it).
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r467943083
##
File path: solr/core/src/java/org/apache/solr/cluster/placement/Cluster.java
##
@@ -0,0 +1,53 @@
+/*
+ * 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;
+
+/**
+ * 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.
+ *
+ * This instance can also be used as a {@link PropertyValueSource} if
{@link PropertyKey}'s need to be specified with
+ * a global cluster target.
+ */
+public interface Cluster extends PropertyValueSource {
+ /**
+ * @return current set of live nodes. Never null, never empty
(Solr wouldn't call the plugin if empty
+ * since no useful work could then be done).
+ */
+ Set getLiveNodes();
+
+ /**
+ * 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.
+ *
+ * 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 getCollection(String collectionName) throws
IOException;
+
+ /**
+ * Allows getting all {@link SolrCollection} present in the cluster.
+ *
+ * WARNING: this call might be extremely inefficient on large
clusters. Usage is discouraged.
+ */
+ Set getAllCollections();
Review comment:
I think there was no call to get only names (away from computer for a
week or more). The only call on cluster state returned DocCollection set or
map...
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r467455202
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/AddReplicasRequest.java
##
@@ -0,0 +1,62 @@
+/*
+ * 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;
+
+/**
+ * Request for creating one or more {@link Replica}'s for 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 using {@link SolrCollection#getShards()}
+ * and verifying if the shard name(s) from {@link #getShardNames()} are there.
+ *
+ * As opposed to {@link CreateNewCollectionRequest}, 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()}).
+ *
+ * There is no extension between this interface and {@link
CreateNewCollectionRequest} in either direction
+ * or from a common ancestor for readability. An ancestor could make sense and
would be an "abstract interface" not intended
+ * to be implemented directly, but this does not exist in Java.
+ *
+ * Plugin code would likely treat the two types of requests differently
since here existing {@link Replica}'s must be taken
+ * into account for placement whereas in {@link CreateNewCollectionRequest} no
{@link Replica}'s are assumed to exist.
+ */
+public interface AddReplicasRequest extends Request {
+ /**
+ * The {@link SolrCollection} to add {@link Replica}(s) to. The replicas are
to be added to a shard that might or might
+ * not yet exist when the plugin's {@link PlacementPlugin#computePlacement}
is called.
+ */
+ SolrCollection getCollection();
+
+ /**
+ * 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).
+ *
+ * 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 getShardNames();
+
+ /** Replicas should only be placed on nodes from the set returned by this
method. */
+ Set getTargetNodes();
Review comment:
And also `Request` -> `PlacementRequest`
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r467454703
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/AddReplicasRequest.java
##
@@ -0,0 +1,62 @@
+/*
+ * 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;
+
+/**
+ * Request for creating one or more {@link Replica}'s for 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 using {@link SolrCollection#getShards()}
+ * and verifying if the shard name(s) from {@link #getShardNames()} are there.
+ *
+ * As opposed to {@link CreateNewCollectionRequest}, 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()}).
+ *
+ * There is no extension between this interface and {@link
CreateNewCollectionRequest} in either direction
+ * or from a common ancestor for readability. An ancestor could make sense and
would be an "abstract interface" not intended
+ * to be implemented directly, but this does not exist in Java.
+ *
+ * Plugin code would likely treat the two types of requests differently
since here existing {@link Replica}'s must be taken
+ * into account for placement whereas in {@link CreateNewCollectionRequest} no
{@link Replica}'s are assumed to exist.
+ */
+public interface AddReplicasRequest extends Request {
+ /**
+ * The {@link SolrCollection} to add {@link Replica}(s) to. The replicas are
to be added to a shard that might or might
+ * not yet exist when the plugin's {@link PlacementPlugin#computePlacement}
is called.
+ */
+ SolrCollection getCollection();
+
+ /**
+ * 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).
+ *
+ * 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 getShardNames();
+
+ /** Replicas should only be placed on nodes from the set returned by this
method. */
+ Set getTargetNodes();
Review comment:
Used work order -> placement plan
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r467453739
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/PlacementPlugin.java
##
@@ -0,0 +1,41 @@
+/*
+ * 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;
+
+/**
+ * Implemented by external plugins to control replica placement and movement
on the search cluster (as well as other things
+ * such as cluster elasticity?) when cluster changes are required (initiated
elsewhere, most likely following a Collection
+ * API call).
+ */
+public interface PlacementPlugin {
Review comment:
I believe Solr side would keep the configuration (map) somewhere and
just pass a reference to it on every call to the plugin. So cost would be
minimal. If the plugin code needs to retrieve a config value, it will basically
do a `get` on that map, very low cost here as well.
I don't see how a plugin can keep state between invocations with a semantic
that's different from a static field, unless we create a notion of higher level
context shared between invocations (and different such higher level contexts
will not be shared). I suggest we skip that aspect for now and reconsider later
when we implement real plugins. For now I believe the model in which the plugin
is instantiated (no arg constructor, or constructor getting the config map or
something simple like that) before every call to the computePlacement method
would be sufficient.
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r467453245
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/ReplicaPlacement.java
##
@@ -0,0 +1,29 @@
+/*
+ * 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;
+
+/**
+ * Placement decision for a single {@link Replica}. Note this placement
decision is used as part of a {@link WorkOrder},
+ * 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).
+ *
+ * Captures the {@link Shard} (via the shard name), {@link Node} and {@link
Replica.ReplicaType} of a Replica to be created.
+ */
+public interface ReplicaPlacement {
Review comment:
Ok, added. Note that the interfaces in
`org.apache.solr.cluster.placement` only condition what the plugin code is
seeing, not what's present in the objects. The `Request` is present in the
`WorkOrder` instances and I added it to the interface, but even if it's not in
the interface, Solr side code would have it.
My thinking (by not putting it in the interface) is that plugin code is
creating these instances so it should know which instance is which and doesn't
need a method on that instance to find out.
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r467127445
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/PlacementPlugin.java
##
@@ -0,0 +1,41 @@
+/*
+ * 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;
+
+/**
+ * Implemented by external plugins to control replica placement and movement
on the search cluster (as well as other things
+ * such as cluster elasticity?) when cluster changes are required (initiated
elsewhere, most likely following a Collection
+ * API call).
+ */
+public interface PlacementPlugin {
Review comment:
I see a few options for passing config to the plugin:
- Pass the config map in the `computePlacement` call along with the rest of
the parameters
- Provide in one of the passed factories the ability of the plugin code to
call back into Solr and get the config
- Add a `configure` call on the plugin as you suggest (then the Solr infra
decides when to call this method)
- Pass the config when the plugin class is instantiated. This might be
equivalent to passing it in the `computePlacement` method if a new plugin class
instance is created for each new computation.
So the real choice is do we create a new plugin class instance per placement
computation or reuse a given one? Creating a new one for each call is likely a
simpler programming model for the plugin developer, it can use class member
variables freely and if it wants to keep some state it has to make it static...
With a new instance per computation, there would be no notion of
"configuration update". A separate call into the plugin as you suggest or
passing the config in `computePlacement` is equivalent (and the latter likely
easier to handle in plugin code).
I'm tempted to pass the config with each call to `computePlacement`
(assuming the saving of not passing it when the plugin doesn't need it are non
measurable). I'm also tempted to make it `Map` given it will
(most likely?) come from XML and the plugin code would have to deal with what
the config means anyway and what types to cast it to...
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r466860061
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/PlacementPlugin.java
##
@@ -0,0 +1,41 @@
+/*
+ * 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;
+
+/**
+ * Implemented by external plugins to control replica placement and movement
on the search cluster (as well as other things
+ * such as cluster elasticity?) when cluster changes are required (initiated
elsewhere, most likely following a Collection
+ * API call).
+ */
+public interface PlacementPlugin {
Review comment:
**Please ignore this comment**. I thought you were referring to how
various plugins get selected for doing work (currently hard coded in
`AssignStrategyFactory.create`)
> The configuration part I didn't really do yet. Unclear to me how the
`configure` method here would be used, since in order to get to it the plugin
has to be loaded already...
> I was thinking defining the plugin class or classes in some solr
configuration file (with the rest of the config). At least a single default
plugin implementation that would be used for all placement needs or a default +
other ones ones (with names that can then be selected by callers of the
Collection API passing a `placement` parameter as suggested in the changes to
`CollectionAdminRequest.java`).
> Are there similar examples in Solr code (loading plugins possibly on a per
collection basis) that I can get inspiration from or reuse?
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r467108068
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/PlacementException.java
##
@@ -0,0 +1,48 @@
+/*
+ * 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;
+
+/**
+ * Exception thrown by a {@link PlacementPlugin} when it is unable to compute
placement for whatever reason (except an
Review comment:
Skipping on this one for now. I believe we need to better identify what
problems plugins will be running into (that are not Solr side exceptions
bubbling up) and how the Solr side implementation could use such a reason
before being able to make useful design decisions.
If you do have specific use cases in mind, please let me know.
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r467102867
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/MetricPropertyValue.java
##
@@ -0,0 +1,30 @@
+/*
+ * 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;
+
+/**
+ * A {@link PropertyValue} representing a metric on the target {@link
PropertyValueSource}.
+ * Note there might be overlap with {@link SystemLoadPropertyValue} (only
applicable to {@link Node}'s), may need to clarify.
+ */
+public interface MetricPropertyValue extends PropertyValue {
+ /**
+ * Returns the metric value from the {@link PropertyValueSource} on which it
was retrieved.
+ * TODO: what type should the metric be? Maybe offer multiple getters for
different java types and have each metric implement the right one and throw
from the wrong ones? This avoids casting...
Review comment:
I've created the number and string property value types but I'm unclear
on the Map one so we need to discuss this further.
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r467083780
##
File path:
solr/core/src/java/org/apache/solr/cluster/placement/PropertyKeyFactory.java
##
@@ -0,0 +1,61 @@
+/*
+ * 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;
+
+/**
+ * Factory used by the plugin to create property keys to request property
values from Solr.
+ *
+ * Building of a {@link PropertyKey} requires specifying the target (context)
from which the value of that key should be
+ * obtained. This is done by specifying the appropriate {@link
PropertyValueSource}.
+ * For clarity, when only a single type of target is acceptable, the
corresponding subtype of {@link PropertyValueSource} is used instead
+ * (for example {@link Node}).
+ */
+public interface PropertyKeyFactory {
+ /**
+ * Returns a property key to request the number of cores on a {@link Node}.
+ */
+ PropertyKey createCoreCountKey(Node node);
+
+ /**
+ * Returns a property key to request disk related info on a {@link Node}.
+ */
+ PropertyKey createDiskInfoKey(Node node);
+
+ /**
+ * Returns a property key to request the value of a system property on a
{@link Node}.
+ * @param systemPropertyName the name of the system property to retrieve.
+ */
+ PropertyKey createSystemPropertyKey(Node node, String systemPropertyName);
+
+ /**
+ * Returns a property key to request the value of a metric.
+ *
+ * Not all metrics make sense everywhere, but metrics can be applied to
different objects. For example
+ * SEARCHER.searcher.indexCommitSize would make sense for a
given replica of a given shard of a given collection,
+ * and possibly in other contexts.
+ *
+ * @param metricSource The registry of the metric. For example a specific
{@link Replica}.
+ * @param metricName for example
SEARCHER.searcher.indexCommitSize.
+ */
+ PropertyKey createMetricKey(PropertyValueSource metricSource, String
metricName);
Review comment:
Is `solr.jetty` of any use or `solr.node` and `solr.jvm` are sufficient?
I see `SolrInfoBean.Group` entries other than `node`, `jvm`, `jetty` are:
- `collection`, `shard`, `cluster`: these can be inferred from
`PropertyValueSource` type,
- `core`: likely not needed for placement computation,
- `overseer`: doesn't seem to be used
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
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
