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/08/10 08:32:54 UTC
[GitHub] [lucene-solr] noblepaul opened a new pull request #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
noblepaul opened a new pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694
A few notes before anyone who starts reviewing this
- This was created after I saw similar attempt as a part #1684 . I believe this has to receive a more wider input and review irrespective of whether devs are interested in autoscaling or not
- This is a WIP PR
- The concrete implementations are for demo purposes. Can be omitted, if required. Anything outside the `o.a.s.cluster.api` package is optional and will be removed
- The interfaces are designed to be minimal to avoid overload. We can and will add more methods later. Let's not add a lot
----------------------------------------------------------------
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] gus-asf commented on a change in pull request #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
gus-asf commented on a change in pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#discussion_r460560248
##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/sdk/SolrCluster.java
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.common.cloud.sdk;
+
+import org.apache.solr.common.util.SimpleMap;
+
+/**
+ * Represents a Solr cluster
+ */
+public interface SolrCluster {
+ /** collections in the cluster */
+ SimpleMap<SolrCollection> collections();
Review comment:
Also aliases... and the Alias class returned should list the collections provided and routing info if routed. (also law of Demeter etc...)
##########
File path: solr/solrj/src/java/org/apache/solr/common/util/LinkedSimpleHashMap.java
##########
@@ -0,0 +1,27 @@
+/*
+ * 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.common.util;
+
+import java.util.LinkedHashMap;
+
+public class LinkedSimpleHashMap<T> extends LinkedHashMap<CharSequence, T> implements SimpleMap<T> {
Review comment:
this class appears unused...
##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/sdk/SolrNode.java
##########
@@ -0,0 +1,33 @@
+/*
+ * 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.common.cloud.sdk;
+
+import org.apache.solr.common.util.SimpleMap;
+
+/**
+ * A read only view of a Solr node
+ */
+public interface SolrNode {
+
+ /** The node name */
+ String name();
+
+ String baseUrl(boolean isV2);
+
+ SimpleMap<ShardReplica> cores();
Review comment:
Possibly good to also answer the question of "what collections are participating in this node" and what shards of Collection X are on this node.
##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/sdk/SolrCollection.java
##########
@@ -0,0 +1,33 @@
+/*
+ * 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.common.cloud.sdk;
+
+import org.apache.solr.common.util.SimpleMap;
+
+/** Represents a collection in Solr */
+public interface SolrCollection {
Review comment:
Also I've definitely had cases where I wanted a list of nodes where this collection is hosted.
##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/sdk/SolrCluster.java
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.common.cloud.sdk;
+
+import org.apache.solr.common.util.SimpleMap;
+
+/**
+ * Represents a Solr cluster
+ */
+public interface SolrCluster {
Review comment:
Since we have our own package namespace, prepending solr isn't really needed unless we think we might also model non-solr clusters.
##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java
##########
@@ -38,7 +43,7 @@
* {@link ZkStateReader#getClusterState()}.
* @lucene.experimental
*/
-public class ClusterState implements JSONWriter.Writable {
+public class ClusterState implements JSONWriter.Writable , SolrCluster {
Review comment:
Little worried that this design allows for casting of the SolrCluster reference...
##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/sdk/SolrCluster.java
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.common.cloud.sdk;
+
+import org.apache.solr.common.util.SimpleMap;
+
+/**
+ * Represents a Solr cluster
+ */
+public interface SolrCluster {
+ /** collections in the cluster */
+ SimpleMap<SolrCollection> collections();
+
+ /** nodes in the cluster */
+ SimpleMap<SolrNode> nodes();
+
+ /** Get a {@link SolrNode} by name. returns null if no such node exists */
+ SolrNode getNode(String node);
+
+ /**
+ * Name of the node in which the overseer is running
+ */
+ String overseerNode();
+
+ /**
+ * The name of the node in which this method is invoked from. returns null, if this is not invoked from a
+ * Solr node
+ */
+ String thisNode();
+
Review comment:
I've wondered sometimes if clusters should have name or id of some sort but that's probably another topic.
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664094746
>I also feel this PR has a lot of overlap with the one I started to define an API for Autoscaling plugins #1684
Yes, totally.That ticket is trying to do something more than the scope of that ticket. I want to limit the scope of that ticket and have wider discussion on how a simple set of interfaces to represent the existing Solr cluster/cloud
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-667104786
@murblanc I shall try to provide some sample implementation in another 2-3 days
----------------------------------------------------------------
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] chatman commented on pull request #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
chatman commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664610279
----------------------------------------------------------------
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 merged pull request #1694: SOLR-14680: Provide simple interfaces to our cloud classes (only API)
Posted by GitBox <gi...@apache.org>.
noblepaul merged pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
murblanc commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664333081
We also need to guarantee some immutability of the instances passed to plugins. Computing placement if the underlying data (cluster state) can change during the computation would be tricky. So these abstractions are read only so plugins can’t use them to change Solr state but the instances passed to plugins are immutable. That would make it possibly a bit harder to only have the internal classes implement these 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 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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
murblanc commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-663999287
I also feel this PR has a lot of overlap with the one I started to define an API for Autoscaling plugins (https://github.com/apache/lucene-solr/pull/1684).
Unless we want to pursue two distinct options for these interfaces and decide later (perfectly OK if that’s the idea) I suggest we use the other PR that has also examples of how these interfaces are used (makes it easier to reason about them and decide what goes where).
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664094145
> Can we start simple by defining the external interface without the required changes to internal classes first? Will make a smaller PR easier to discuss.
Feel free to ignore the implementing classes. That was to demonstrate one way of implementing these interfaces. I'm happy to focus on the interfaces inside the `sdk` package
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
murblanc commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664312074
Thanks Noble.
So what’s your guidance regarding the Autoscaling PR? Shall we assume the Autoscaling plugins need to use these interfaces or shall I continue building cluster/node/collection/shard/replica abstractions that make plugin development easier?
If the first option, there will be a few requirements on these interfaces (and their implementations), for example allowing them to be keys in maps (variable values or snitches are per node or collection or replica etc, they need to be stored somewhere so the plugin can access them). Ideally also as stated elsewhere, all these interfaces implement a common one (could be empty) so that snitch/variable target can be passed easily.
The Autoscaling plugin work can also define similar abstractions on top of the interfaces defined here.
I do see value for all plugins of all types to share a single set of abstractions but obviously this puts more constraints on these abstractions...
----------------------------------------------------------------
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] gus-asf edited a comment on pull request #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
gus-asf edited a comment on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664018065
These interfaces look like a good way to begin. Thoughts: This is an excellent point at which to consider and then stick to a preferred set of terminology. My favorites are:
- **Machines** are the operating system level container or hardware and have
- **Nodes** which are running solr processes on machines which have
- **Collections** which are logical indexes associated with a particular schema and have
- **Shards** which split a collection on a routing value to allow scaling and have
- **Replicas** which are duplicate copies implementing a shard often hosted on separate nodes.
We should strive to apply only ONE word for each level throughout the interfaces to make them as simple and easy to understand as possible.
These 5 terms all have nice clear english words that communicate their function to some extent. Core however only really means "central thing" and I have always thought it was a very confusing word to use here since it has almost no memnonic value. Slice is also good for the shard level, but I think shard is no worse and more commonly used in documentation. Although core has been used many times in the past, I don't think anyone is going to have trouble finding what they want via the terms I'm suggesting.
The Cluster level concept usually ignores Machines and speaks only of nodes... I could be convinced either way on whether or not that's a good thing.
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#discussion_r460627824
##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/sdk/SolrCollection.java
##########
@@ -0,0 +1,33 @@
+/*
+ * 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.common.cloud.sdk;
+
+import org.apache.solr.common.util.SimpleMap;
+
+/** Represents a collection in Solr */
+public interface SolrCollection {
Review comment:
That should be a utility method outside
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
noblepaul edited a comment on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664402636
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our cloud classes (only API)
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#discussion_r467845512
##########
File path: solr/solrj/src/java/org/apache/solr/cluster/api/SolrNode.java
##########
@@ -0,0 +1,39 @@
+/*
+ * 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.api;
+
+import org.apache.solr.common.util.SimpleMap;
+
+/** A read only view of a Solr node */
+public interface SolrNode {
+
+ /** The node name */
+ String name();
+
+ /**Base http url for this node
+ *
+ * @param isV2 if true gives the /api endpoint , else /solr endpoint
+ */
+ String baseUrl(boolean isV2);
Review comment:
done
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
murblanc commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664230886
Many methods returning other objects are returning names of other abstractions being defined here (Shard, Collection, Node etc.).
I’d think an instance of the abstraction should be returned instead, and that instance would have a getName() method (i.e have SolrCollection getCollection() rather than String getCollection() on a Shard for example).
What’s the rationale for returning names rather than objects?
----------------------------------------------------------------
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] janhoy commented on a change in pull request #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#discussion_r462911335
##########
File path: solr/solrj/src/java/org/apache/solr/cluster/api/Config.java
##########
@@ -0,0 +1,40 @@
+/*
+ * 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.api;
+
+import java.io.InputStream;
+import java.util.Set;
+import java.util.function.Consumer;
+
+import org.apache.solr.common.SolrException;
+
+public interface Config {
+
+ String name();
+
+ /**set of files in the config */
Review comment:
Ok, agree
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our cloud classes (only API)
Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-671305266
I intend to merge this 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
[GitHub] [lucene-solr] noblepaul commented on pull request #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-671136445
Opening another PR
----------------------------------------------------------------
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] gus-asf edited a comment on pull request #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
gus-asf edited a comment on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664018065
These interfaces look like a good way to begin. Thoughts: This is an excellent point at which to consider and then stick to a preferred set of terminology. My favorites are:
- **Machines** are the operating system level container or hardware and have
- **Nodes** which are running solr processes on machines which have portions of
- **Collections** which are logical indexes associated with a particular schema and have
- **Shards** which split a collection on a routing value to allow scaling and have
- **Replicas** which are duplicate copies implementing a shard often hosted on separate nodes.
We should strive to apply only ONE word for each level throughout the interfaces to make them as simple and easy to understand as possible.
These 5 terms all have nice clear english words that communicate their function to some extent. Core however only really means "central thing" and I have always thought it was a very confusing word to use here since it has almost no memnonic value. Slice is also good for the shard level, but I think shard is no worse and more commonly used in documentation. Although core has been used many times in the past, I don't think anyone is going to have trouble finding what they want via the terms I'm suggesting.
The Cluster level concept usually ignores Machines and speaks only of nodes... I could be convinced either way on whether or not that's a good thing.
For fun I drew a near worst case diagaram (omitting back references) :)
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
noblepaul edited a comment on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664095650
>These interfaces look like a good way to begin. Thoughts: This is an excellent point at which to consider and then stick to a preferred set of terminology. My favorites are:
There is no way to have a "machine" abstraction in Solr. We can start with a `Node` as an atomic unit. I wanted to carefully avoid the class names already taken up so that existing code can be left as it is
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664244951
>I’d think an instance of the abstraction should be returned instead,
I though about it and decided against it. We should always try to keep the methods light and avoid creating unnecessary Objects.
for instance, the `Shard#leader()` returns the name of the leader replica. A String is lightweight object and we do not need to create a `ShardReplica` object unnecessarily.
> I see two cases where we need interfaces such as defined here:
Both. over time , we will move as much of our code to use these interfaces instead of concrete classes.
I do not see a reason why external plugins cannot use these interfaces. This is not to say that external plugins may need some more methods. In that case, just compose your own interfaces using these as building blocks.
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#discussion_r460627654
##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/sdk/SolrCluster.java
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.common.cloud.sdk;
+
+import org.apache.solr.common.util.SimpleMap;
+
+/**
+ * Represents a Solr cluster
+ */
+public interface SolrCluster {
+ /** collections in the cluster */
+ SimpleMap<SolrCollection> collections();
Review comment:
Yes, I wanted to include aliases. Where do you propose it to be there.
I would say , There should be no `Alias` class/interface.
It should be as simple as
`SolrCluster#collections#get(String name , include alias) `
and
` SolrCollection#aliases() returns List<String>`
##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/sdk/SolrCluster.java
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.common.cloud.sdk;
+
+import org.apache.solr.common.util.SimpleMap;
+
+/**
+ * Represents a Solr cluster
+ */
+public interface SolrCluster {
+ /** collections in the cluster */
+ SimpleMap<SolrCollection> collections();
Review comment:
Yes, I wanted to include aliases. Where do you propose it to be there?
I would say , There should be no `Alias` class/interface.
It should be as simple as
`SolrCluster#collections#get(String name , include alias) `
and
` SolrCollection#aliases() returns List<String>`
----------------------------------------------------------------
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] gus-asf edited a comment on pull request #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
gus-asf edited a comment on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664078634
While marking things internal and external is good, a stronger possibility is to code more defensively and make "new plugins" get instantiated/injected with implementations of a facade that return instances of the interfaces. If those implementations can't just be cast to internal types we then have a much higher barrier to abuse (reflection).
As for package names I agree we need to think carefully about them. It is a question what the goal is too. If we're reworking the internals, sdk won't sound right at all. If we are building something only meant to be consumed by an outer layer of plugins to insulate the plugins from internal refactoring, something in that vein sounds fine (or maybe o.a.s.cloud.plugin.api?) Settling on what our goals are will help us decide what name is best.
----------------------------------------------------------------
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] chatman commented on a change in pull request #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
chatman commented on a change in pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#discussion_r460623863
##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java
##########
@@ -38,7 +43,7 @@
* {@link ZkStateReader#getClusterState()}.
* @lucene.experimental
*/
-public class ClusterState implements JSONWriter.Writable {
+public class ClusterState implements JSONWriter.Writable , SolrCluster {
Review comment:
Let's rename Solr luster to Cluster.
----------------------------------------------------------------
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] gus-asf commented on pull request #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
gus-asf commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664125851
> > These interfaces look like a good way to begin. Thoughts: This is an excellent point at which to consider and then stick to a preferred set of terminology. My favorites are:
>
> There is no way to have a "machine" abstraction in Solr. We can start of a `Node` as an atomic unit. I wanted to carefully avoid the class names already taken up so that existing code can be left as it is
Yeah machine is a difficult notion as it's a real world thing entirely outside java. It probably has to be optional and provided by sysprop or env var. and even so might be reducible to a property on node. Certainly detecting it would be fraught with peril. But machine designations (and rack designations) might be important to some folks.
----------------------------------------------------------------
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] dsmiley commented on a change in pull request #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#discussion_r462508983
##########
File path: solr/solrj/src/java/org/apache/solr/cluster/api/Config.java
##########
@@ -0,0 +1,40 @@
+/*
+ * 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.api;
+
+import java.io.InputStream;
+import java.util.Set;
+import java.util.function.Consumer;
+
+import org.apache.solr.common.SolrException;
+
+public interface Config {
+
+ String name();
+
+ /**set of files in the config */
Review comment:
A one-line variant is acceptable: https://google.github.io/styleguide/javaguide.html#s7.1.1-javadoc-multi-line
The only problem I see with the javadoc here is that it's missing a leading space, and it should capitalize the first letter.
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
murblanc commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-666429803
Machine vs Node: environment variables (rack, availability zone etc) are "machine" abstractions but can be considered at the node level. Free disk is clearly a machine abstraction (two nodes on same machine might be sharing the same free disk and skewing decisions based on it) but I don't see how this can be solved (even a single node might be sharing space with other processes).
Therefore I don't see how a machine abstraction helps.
----------------------------------------------------------------
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] gus-asf edited a comment on pull request #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
gus-asf edited a comment on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664018065
These interfaces look like a good way to begin. Thoughts: This is an excellent point at which to consider and then stick to a preferred set of terminology. My favorites are:
- **Machines** are the operating system level container or hardware and have
- **Nodes** which are running solr processes on machines which have portions of
- **Collections** which are logical indexes associated with a particular schema and have
- **Shards** which split a collection on a routing value to allow scaling and have
- **Replicas** which are duplicate copies implementing a shard often hosted on separate nodes.
We should strive to apply only ONE word for each level throughout the interfaces to make them as simple and easy to understand as possible.
These 5 terms all have nice clear english words that communicate their function to some extent. Core however only really means "central thing" and I have always thought it was a very confusing word to use here since it has almost no memnonic value. Slice is also good for the shard level, but I think shard is no worse and more commonly used in documentation. Although core has been used many times in the past, I don't think anyone is going to have trouble finding what they want via the terms I'm suggesting.
The Cluster level concept usually ignores Machines and speaks only of nodes... I could be convinced either way on whether or not that's a good thing.
For fun I drew a near worst case diagaram (omitting back references and convenience rollups) :)
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#discussion_r460622100
##########
File path: solr/solrj/src/java/org/apache/solr/common/util/LinkedSimpleHashMap.java
##########
@@ -0,0 +1,27 @@
+/*
+ * 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.common.util;
+
+import java.util.LinkedHashMap;
+
+public class LinkedSimpleHashMap<T> extends LinkedHashMap<CharSequence, T> implements SimpleMap<T> {
Review comment:
yes, this was to demonstrate how to implement a default impl of `SimpleMap`
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
murblanc commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-666431559
Would love to see sample plug-in code accessing cluster config. How the instances are obtained and how the cluster is explored (nodes, collections, shards, replicas).
----------------------------------------------------------------
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 edited a comment on pull request #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
murblanc edited a comment on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664048888
Snitches have a target (at least node or replica, but one could imagine Cluster, collection or shard if such snitches do not already exist).
Would be helpful for all these classes to implement a common interface so they can be passed when a notch target is needed in Autoscaling plugins.
(See in https://github.com/apache/lucene-solr/pull/1684/files#diff-fc8bc5eb94b9e48a24c2ec768733e72b)
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#discussion_r460622520
##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java
##########
@@ -38,7 +43,7 @@
* {@link ZkStateReader#getClusterState()}.
* @lucene.experimental
*/
-public class ClusterState implements JSONWriter.Writable {
+public class ClusterState implements JSONWriter.Writable , SolrCluster {
Review comment:
We can have multiple implementations of `SolrCluster` .The idea of existing classes implementing these interfaces is to have readily available implementations
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#discussion_r460626596
##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/sdk/SolrCluster.java
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.common.cloud.sdk;
+
+import org.apache.solr.common.util.SimpleMap;
+
+/**
+ * Represents a Solr cluster
+ */
+public interface SolrCluster {
Review comment:
I know. Imagine this will also be used outside of Solr as a part of SolrJ. So if you read some client code,
`Cluster cluster;`
is less readable compared to
`SolrCluster solrCluster`;
Eventually, I would wish to replace a lot of SolrJ code/API with a standard set of 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 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 closed pull request #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
noblepaul closed pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694
----------------------------------------------------------------
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] chatman commented on pull request #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
chatman commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664052755
As I suggested in the other PR on autoscaling, I feel we should change the package name:
https://github.com/apache/lucene-solr/pull/1684#issuecomment-664051353
Also, WDYT about marking the concrete classes DocCollection, Replica etc. with @lucene.internal to make it clear that they shouldn't be used directly from the plugins? Alternatively, we can mark these new interfaces in this PR with a new annotation (e.g. @solr.external) that signals to plugin writers that these will be stable for their use across versions.
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
murblanc commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-663994405
Can we start simple by defining the external interface without the required changes to internal classes first? Will make a smaller PR easier to discuss.
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
murblanc commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664608589
EagerlyFetchedSolrCluster would then guarantee immutability? Must also be careful not too fetch too much. Autoscaling for example might not always need all details on everything.
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
noblepaul edited a comment on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664094145
> Can we start simple by defining the external interface without the required changes to internal classes first? Will make a smaller PR easier to discuss.
Feel free to ignore the implementing classes. That was to demonstrate one way of implementing these interfaces. I'm happy to focus on the interfaces inside the `sdk` package
I have updated the description. Only the SDK is important. Implementation can/will be removed
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
murblanc commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664240038
I see two cases where we need interfaces such as defined here:
- internal code. Coding to interfaces rather than the actual implementation makes for better structured code usually with less implementation leaks,
- external “plugins”.
What’s the intention? If only the later, then there’s really no need to have the internal classes implement these interfaces, a wrapper is ok.
I believe addressing both points with a single interface is complicated (and to a point counterproductive as it ties internal and external views).
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
murblanc commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664048888
Snitches have a target (at least node or replica, but one could imagine Cluster, collection or shard if such snitches do not already exist).
Would be helpful for all these classes to implement a common interface so they can be passed when a notch target is needed in Autoscaling plugins.
----------------------------------------------------------------
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] gus-asf edited a comment on pull request #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
gus-asf edited a comment on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664018065
These interfaces look like a good way to begin. Thoughts: This is an excellent point at which to consider and then stick to a preferred set of terminology. My favorites are:
- **Machines** are the operating system level container or hardware and have
- **Nodes** which are running solr processes on machines which have portions of
- **Collections** which are logical indexes associated with a particular schema and have
- **Shards** which split a collection on a routing value to allow scaling and have
- **Replicas** which are duplicate copies implementing a shard often hosted on separate nodes.
We should strive to apply only ONE word for each level throughout the interfaces to make them as simple and easy to understand as possible.
These 5 terms all have nice clear english words that communicate their function to some extent. Core however only really means "central thing" and I have always thought it was a very confusing word to use here since it has almost no memnonic value. Slice is also good for the shard level, but I think shard is no worse and more commonly used in documentation. Although core has been used many times in the past, I don't think anyone is going to have trouble finding what they want via the terms I'm suggesting.
The Cluster level concept usually ignores Machines and speaks only of nodes... I could be convinced either way on whether or not that's a good thing.
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#discussion_r460627654
##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/sdk/SolrCluster.java
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.common.cloud.sdk;
+
+import org.apache.solr.common.util.SimpleMap;
+
+/**
+ * Represents a Solr cluster
+ */
+public interface SolrCluster {
+ /** collections in the cluster */
+ SimpleMap<SolrCollection> collections();
Review comment:
Yes, I wanted to include aliases. Where do you propose it to be there?
I would say , There should be no `Alias` class/interface.
It should be as simple as
`SolrCluster#collections#get(String name , boolean includeAlias) `
and
` SolrCollection#aliases() returns List<String>`
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#discussion_r460624050
##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/sdk/SolrNode.java
##########
@@ -0,0 +1,33 @@
+/*
+ * 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.common.cloud.sdk;
+
+import org.apache.solr.common.util.SimpleMap;
+
+/**
+ * A read only view of a Solr node
+ */
+public interface SolrNode {
+
+ /** The node name */
+ String name();
+
+ String baseUrl(boolean isV2);
+
+ SimpleMap<ShardReplica> cores();
Review comment:
I would say, that is something that can be easily computed from the `cores()` method and can be provided as a helper method elsewhere
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#discussion_r462931220
##########
File path: solr/solrj/src/java/org/apache/solr/cluster/api/Config.java
##########
@@ -0,0 +1,40 @@
+/*
+ * 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.api;
+
+import java.io.InputStream;
+import java.util.Set;
+import java.util.function.Consumer;
+
+import org.apache.solr.common.SolrException;
+
+public interface Config {
Review comment:
I don't like both names , `Config` & `ConfigSet`
`configset` suggests it is a set of configs? in reality it's a single configuration which has multiple files
----------------------------------------------------------------
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] gus-asf commented on pull request #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
gus-asf commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664078634
While marking things internal and external is good, a stronger possibility to code more defensively and make "new plugins" get instantiated/injected with implementations of a facade that return instances of the interfaces. If those implementations can't just be cast to internal types we then have a much higher barrier to abuse (reflection).
As for package names I agree we need to think carefully about them. It is a question what the goal is too. If we're reworking the internals, sdk won't sound right at all. If we are building something only meant to be consumed by an outer layer of plugins to insulate the plugins from internal refactoring, something in that vein sounds fine (or maybe o.a.s.cloud.plugin.api?) Settling on what our goals are will help us decide what name is best.
----------------------------------------------------------------
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] janhoy commented on a change in pull request #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#discussion_r462323818
##########
File path: solr/solrj/src/java/org/apache/solr/cluster/api/SolrNode.java
##########
@@ -0,0 +1,39 @@
+/*
+ * 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.api;
+
+import org.apache.solr.common.util.SimpleMap;
+
+/** A read only view of a Solr node */
+public interface SolrNode {
+
+ /** The node name */
+ String name();
+
+ /**Base http url for this node
+ *
+ * @param isV2 if true gives the /api endpoint , else /solr endpoint
+ */
+ String baseUrl(boolean isV2);
Review comment:
Instead of boolean isV2, what about an Enum, that could then evolve?
##########
File path: solr/solrj/src/java/org/apache/solr/cluster/api/Config.java
##########
@@ -0,0 +1,40 @@
+/*
+ * 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.api;
+
+import java.io.InputStream;
+import java.util.Set;
+import java.util.function.Consumer;
+
+import org.apache.solr.common.SolrException;
+
+public interface Config {
+
+ String name();
+
+ /**set of files in the config */
Review comment:
General : Please format Javadocs properly with text starting on line 2 and capital letter, even if it takes more lines.
##########
File path: solr/solrj/src/java/org/apache/solr/cluster/api/Config.java
##########
@@ -0,0 +1,40 @@
+/*
+ * 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.api;
+
+import java.io.InputStream;
+import java.util.Set;
+import java.util.function.Consumer;
+
+import org.apache.solr.common.SolrException;
+
+public interface Config {
Review comment:
Use `ConfigSet` instead? `Config` is soooo overloaded.
----------------------------------------------------------------
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] gus-asf commented on pull request #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
gus-asf commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664018065
These interfaces look like a good way to begin. Thoughts: This is an excellent point at which to consider and then stick to a preferred set of terminology. My favorites are:
- **Machines** are the operating system level container or hardware and have
- **Nodes** which are running solr processes on machines which have
- **Collections** which are logical indexes associated with a particular schema and have
- **Shards** which split a collection on a routing value to allow scaling and have
- **Replicas** which are duplicate copies implementing a shard often hosted on separate nodes.
We should strive to apply only ONE word for each level throughout the interfaces to make them as simple and easy to understand as possible.
These 5 terms all have nice clear english words that communicate their function to some extent. Core however only really means "central thing" and I have always thought it was a very confusing word to use here since it has almost no memnonic value. Slice is also good for the shard level, but I think shard is no worse and more commonly used in documentation. Although core has been used many times in the past, I don't think anyone is going to have trouble finding what they want via the terms I'm suggesting.
----------------------------------------------------------------
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 #1694: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes
Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1694:
URL: https://github.com/apache/lucene-solr/pull/1694#issuecomment-664095650
>These interfaces look like a good way to begin. Thoughts: This is an excellent point at which to consider and then stick to a preferred set of terminology. My favorites are:
There is no way to have a "machine" abstraction in Solr. We can start of a `Node` as an atomic unit. I wanted to carefully avoid the class names already taken up so that existing code can be left as it is
----------------------------------------------------------------
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