[GitHub] [lucene-solr] sigram commented on a change in pull request #1758: SOLR-14749: Provide a clean API for cluster-level event processing, Initial draft.
sigram commented on a change in pull request #1758:
URL: https://github.com/apache/lucene-solr/pull/1758#discussion_r499170285
##
File path:
solr/core/src/java/org/apache/solr/cluster/events/impl/CollectionsRepairEventListener.java
##
@@ -0,0 +1,185 @@
+/*
+ * 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.events.impl;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.cloud.SolrCloudManager;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.cloud.api.collections.Assign;
+import org.apache.solr.cluster.events.ClusterEvent;
+import org.apache.solr.cluster.events.ClusterEventListener;
+import org.apache.solr.cluster.events.NodesDownEvent;
+import org.apache.solr.cluster.events.ReplicasDownEvent;
+import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.Replica;
+import org.apache.solr.common.cloud.ReplicaPosition;
+import org.apache.solr.core.CoreContainer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This is an illustration how to re-implement the combination of 8x
+ * NodeLostTrigger and AutoAddReplicasPlanAction to maintain the collection's
replication factor.
+ * NOTE: there's no support for 'waitFor' yet.
+ * NOTE 2: this functionality would be probably more reliable when executed
also as a
+ * periodically scheduled check - both as a reactive (listener) and proactive
(scheduled) measure.
+ */
+public class CollectionsRepairEventListener implements ClusterEventListener {
Review comment:
Not yet - there's a unit test. I implemented this class to illustrate
that the events API is sufficient to bring back the "autoAddReplicas"
functionality that we removed in master.
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] sigram commented on a change in pull request #1758: SOLR-14749: Provide a clean API for cluster-level event processing, Initial draft.
sigram commented on a change in pull request #1758:
URL: https://github.com/apache/lucene-solr/pull/1758#discussion_r499169806
##
File path: solr/core/src/java/org/apache/solr/cluster/events/ClusterEvent.java
##
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.cluster.events;
+
+import org.apache.solr.common.MapWriter;
+
+import java.io.IOException;
+import java.time.Instant;
+
+/**
+ * Cluster-level event.
+ */
+public interface ClusterEvent extends MapWriter {
Review comment:
Makes sense, I'll remove 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] sigram commented on a change in pull request #1758: SOLR-14749: Provide a clean API for cluster-level event processing, Initial draft.
sigram commented on a change in pull request #1758:
URL: https://github.com/apache/lucene-solr/pull/1758#discussion_r494131970
##
File path: solr/core/src/java/org/apache/solr/core/CoreContainer.java
##
@@ -889,7 +896,37 @@ public void load() {
ContainerPluginsApi containerPluginsApi = new ContainerPluginsApi(this);
containerHandlers.getApiBag().registerObject(containerPluginsApi.readAPI);
containerHandlers.getApiBag().registerObject(containerPluginsApi.editAPI);
+
+ // create the ClusterEventProducer
+ CustomContainerPlugins.ApiInfo clusterEventProducerInfo =
customContainerPlugins.getPlugin(ClusterEventProducer.PLUGIN_NAME);
+ if (clusterEventProducerInfo != null) {
+clusterEventProducer = (ClusterEventProducer)
clusterEventProducerInfo.getInstance();
+ } else {
+clusterEventProducer = new ClusterEventProducerImpl(this);
+ }
+ // init ClusterSingleton-s
+ Map singletons = new ConcurrentHashMap<>();
+ if (clusterEventProducer instanceof ClusterSingleton) {
+singletons.put(ClusterEventProducer.PLUGIN_NAME, (ClusterSingleton)
clusterEventProducer);
+ }
+
+ // register ClusterSingleton handlers
+ // XXX register also other ClusterSingleton-s from packages - how?
+ containerHandlers.keySet().forEach(handlerName -> {
Review comment:
The purpose of this code is to build a registry of existing
`ClusterSingleton` implementations (perhaps this should go to a dedicated
registry class). We don't have a dependency injection framework, so we need to
somewhere perform the discovery and registration ourselves.
And we need a registry in order to manage the `ClusterSingleton` lifecycle
together with the Overseer leader life-cycle.
##
File path:
solr/core/src/java/org/apache/solr/handler/admin/ContainerPluginsApi.java
##
@@ -64,15 +64,15 @@ public ContainerPluginsApi(CoreContainer coreContainer) {
public class Read {
@EndPoint(method = METHOD.GET,
-path = "/cluster/plugin",
+path = "/cluster/plugins",
Review comment:
Right, but this is for 9.0 so we can break back-compat if it's justified
- and I think it is because the singular name here doesn't make sense, as it is
a location where multiple plugin configurations are defined.
In any case we can provide a back-compat shim for 9.0 to also accept
`plugin` singular.
##
File path:
solr/core/src/java/org/apache/solr/handler/admin/ContainerPluginsApi.java
##
@@ -64,15 +64,15 @@ public ContainerPluginsApi(CoreContainer coreContainer) {
public class Read {
@EndPoint(method = METHOD.GET,
-path = "/cluster/plugin",
+path = "/cluster/plugins",
permission = PermissionNameProvider.Name.COLL_READ_PERM)
public void list(SolrQueryRequest req, SolrQueryResponse rsp) throws
IOException {
- rsp.add(PLUGIN, plugins(zkClientSupplier));
+ rsp.add(PLUGINS, plugins(zkClientSupplier));
Review comment:
See above.
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] sigram commented on a change in pull request #1758: SOLR-14749: Provide a clean API for cluster-level event processing, Initial draft.
sigram commented on a change in pull request #1758:
URL: https://github.com/apache/lucene-solr/pull/1758#discussion_r494134314
##
File path:
solr/core/src/java/org/apache/solr/handler/admin/ContainerPluginsApi.java
##
@@ -64,15 +64,15 @@ public ContainerPluginsApi(CoreContainer coreContainer) {
public class Read {
@EndPoint(method = METHOD.GET,
-path = "/cluster/plugin",
+path = "/cluster/plugins",
Review comment:
Right, but this is for 9.0 so we can break back-compat if it's justified
- and I think it is because the singular name here doesn't make sense, as it is
a location where multiple plugin configurations are defined.
In any case we can provide a back-compat shim for 9.0 to also accept
`plugin` singular.
##
File path:
solr/core/src/java/org/apache/solr/handler/admin/ContainerPluginsApi.java
##
@@ -64,15 +64,15 @@ public ContainerPluginsApi(CoreContainer coreContainer) {
public class Read {
@EndPoint(method = METHOD.GET,
-path = "/cluster/plugin",
+path = "/cluster/plugins",
permission = PermissionNameProvider.Name.COLL_READ_PERM)
public void list(SolrQueryRequest req, SolrQueryResponse rsp) throws
IOException {
- rsp.add(PLUGIN, plugins(zkClientSupplier));
+ rsp.add(PLUGINS, plugins(zkClientSupplier));
Review comment:
See above.
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] sigram commented on a change in pull request #1758: SOLR-14749: Provide a clean API for cluster-level event processing, Initial draft.
sigram commented on a change in pull request #1758:
URL: https://github.com/apache/lucene-solr/pull/1758#discussion_r494131970
##
File path: solr/core/src/java/org/apache/solr/core/CoreContainer.java
##
@@ -889,7 +896,37 @@ public void load() {
ContainerPluginsApi containerPluginsApi = new ContainerPluginsApi(this);
containerHandlers.getApiBag().registerObject(containerPluginsApi.readAPI);
containerHandlers.getApiBag().registerObject(containerPluginsApi.editAPI);
+
+ // create the ClusterEventProducer
+ CustomContainerPlugins.ApiInfo clusterEventProducerInfo =
customContainerPlugins.getPlugin(ClusterEventProducer.PLUGIN_NAME);
+ if (clusterEventProducerInfo != null) {
+clusterEventProducer = (ClusterEventProducer)
clusterEventProducerInfo.getInstance();
+ } else {
+clusterEventProducer = new ClusterEventProducerImpl(this);
+ }
+ // init ClusterSingleton-s
+ Map singletons = new ConcurrentHashMap<>();
+ if (clusterEventProducer instanceof ClusterSingleton) {
+singletons.put(ClusterEventProducer.PLUGIN_NAME, (ClusterSingleton)
clusterEventProducer);
+ }
+
+ // register ClusterSingleton handlers
+ // XXX register also other ClusterSingleton-s from packages - how?
+ containerHandlers.keySet().forEach(handlerName -> {
Review comment:
The purpose of this code is to build a registry of existing
`ClusterSingleton` implementations (perhaps this should go to a dedicated
registry class). We don't have a dependency injection framework, so we need to
somewhere perform the discovery and registration ourselves.
And we need a registry in order to manage the `ClusterSingleton` lifecycle
together with the Overseer leader life-cycle.
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] sigram commented on a change in pull request #1758: SOLR-14749: Provide a clean API for cluster-level event processing, Initial draft.
sigram commented on a change in pull request #1758:
URL: https://github.com/apache/lucene-solr/pull/1758#discussion_r491867642
##
File path:
solr/core/src/java/org/apache/solr/cluster/events/CollectionsAddedEvent.java
##
@@ -0,0 +1,32 @@
+/*
+ * 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.events;
+
+import java.util.Collection;
+
+/**
+ * Event generated when some collections have been added.
+ */
+public interface CollectionsAddedEvent extends ClusterEvent {
+
+ @Override
+ default EventType getType() {
+return EventType.COLLECTIONS_ADDED;
+ }
+
+ Collection getCollectionNames();
Review comment:
Ok.
##
File path:
solr/core/src/java/org/apache/solr/cluster/events/ClusterEventListener.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.events;
+
+import java.util.Set;
+
+/**
+ * Components that want to be notified of cluster-wide events should use this.
+ *
+ * XXX should this work only for ClusterSingleton-s? some types of events may
be
+ * XXX difficult (or pointless) to propagate to every node.
+ */
+public interface ClusterEventListener {
+
+ /**
+ * The types of events that this listener can process.
+ */
+ Set getEventTypes();
Review comment:
Ok.
##
File path:
solr/core/src/java/org/apache/solr/cluster/events/ClusterEventProducer.java
##
@@ -0,0 +1,71 @@
+/*
+ * 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.events;
+
+import org.apache.solr.cloud.ClusterSingleton;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * Component that produces {@link ClusterEvent} instances.
+ */
+public interface ClusterEventProducer extends ClusterSingleton {
+
+ String PLUGIN_NAME = "clusterEventProducer";
+
+ /**
+ * Returns a modifiable map of event types and listeners to process events
+ * of a given type.
+ */
+ Map> getEventListeners();
Review comment:
`EnumMap` is a concrete class, if we specify just a `Map` here then
implementations are free to use whatever impl they want.
##
File path:
solr/core/src/java/org/apache/solr/cluster/events/ClusterPropertiesChangedEvent.java
##
@@ -0,0 +1,35 @@
+/*
+ * 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
[GitHub] [lucene-solr] sigram commented on a change in pull request #1758: SOLR-14749: Provide a clean API for cluster-level event processing, Initial draft.
sigram commented on a change in pull request #1758:
URL: https://github.com/apache/lucene-solr/pull/1758#discussion_r491870338
##
File path:
solr/core/src/java/org/apache/solr/cluster/events/ClusterPropertiesChangedEvent.java
##
@@ -0,0 +1,35 @@
+/*
+ * 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.events;
+
+import java.util.Map;
+
+/**
+ * Event generated when {@link
org.apache.solr.common.cloud.ZkStateReader#CLUSTER_PROPS} is modified.
+ */
+public interface ClusterPropertiesChangedEvent extends ClusterEvent {
+
+ @Override
+ default EventType getType() {
+return EventType.CLUSTER_PROPERTIES_CHANGED;
+ }
+
+ Map getOldClusterProperties();
Review comment:
Ok, makes sense.
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] sigram commented on a change in pull request #1758: SOLR-14749: Provide a clean API for cluster-level event processing, Initial draft.
sigram commented on a change in pull request #1758:
URL: https://github.com/apache/lucene-solr/pull/1758#discussion_r491870172
##
File path:
solr/core/src/java/org/apache/solr/cluster/events/ClusterEventProducer.java
##
@@ -0,0 +1,71 @@
+/*
+ * 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.events;
+
+import org.apache.solr.cloud.ClusterSingleton;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * Component that produces {@link ClusterEvent} instances.
+ */
+public interface ClusterEventProducer extends ClusterSingleton {
+
+ String PLUGIN_NAME = "clusterEventProducer";
+
+ /**
+ * Returns a modifiable map of event types and listeners to process events
+ * of a given type.
+ */
+ Map> getEventListeners();
Review comment:
`EnumMap` is a concrete class, if we specify just a `Map` here then
implementations are free to use whatever impl they want.
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] sigram commented on a change in pull request #1758: SOLR-14749: Provide a clean API for cluster-level event processing, Initial draft.
sigram commented on a change in pull request #1758:
URL: https://github.com/apache/lucene-solr/pull/1758#discussion_r491869081
##
File path:
solr/core/src/java/org/apache/solr/cluster/events/ClusterEventListener.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.events;
+
+import java.util.Set;
+
+/**
+ * Components that want to be notified of cluster-wide events should use this.
+ *
+ * XXX should this work only for ClusterSingleton-s? some types of events may
be
+ * XXX difficult (or pointless) to propagate to every node.
+ */
+public interface ClusterEventListener {
+
+ /**
+ * The types of events that this listener can process.
+ */
+ Set getEventTypes();
Review comment:
Ok.
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] sigram commented on a change in pull request #1758: SOLR-14749: Provide a clean API for cluster-level event processing, Initial draft.
sigram commented on a change in pull request #1758:
URL: https://github.com/apache/lucene-solr/pull/1758#discussion_r491867642
##
File path:
solr/core/src/java/org/apache/solr/cluster/events/CollectionsAddedEvent.java
##
@@ -0,0 +1,32 @@
+/*
+ * 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.events;
+
+import java.util.Collection;
+
+/**
+ * Event generated when some collections have been added.
+ */
+public interface CollectionsAddedEvent extends ClusterEvent {
+
+ @Override
+ default EventType getType() {
+return EventType.COLLECTIONS_ADDED;
+ }
+
+ Collection getCollectionNames();
Review comment:
Ok.
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] sigram commented on a change in pull request #1758: SOLR-14749: Provide a clean API for cluster-level event processing, Initial draft.
sigram commented on a change in pull request #1758:
URL: https://github.com/apache/lucene-solr/pull/1758#discussion_r484517789
##
File path: solr/core/src/java/org/apache/solr/cluster/events/ScheduledEvent.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.events;
+
+/**
+ *
+ */
+public interface ScheduledEvent extends ClusterEvent {
Review comment:
> For some context, almost all devops engineers I've interacted with do
not trust Solr to carry out these things in an unsupervised manner
IIUIC this objection was to the scheduler part. This is now removed from
this PR - changes here are related purely to the facility to generate
cluster-state related events and the API for registering listeners to these
events.
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] sigram commented on a change in pull request #1758: SOLR-14749: Provide a clean API for cluster-level event processing, Initial draft.
sigram commented on a change in pull request #1758:
URL: https://github.com/apache/lucene-solr/pull/1758#discussion_r473848530
##
File path:
solr/core/src/java/org/apache/solr/cluster/events/ClusterSingleton.java
##
@@ -0,0 +1,14 @@
+package org.apache.solr.cluster.events;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+/**
+ * Intended for {@link org.apache.solr.core.CoreContainer} plugins that should
be
+ * enabled only one instance per cluster.
+ * Implementation detail: currently these plugins are instantiated on the
+ * Overseer leader, and closed when the current node loses its leadership.
+ */
+@Retention(RetentionPolicy.RUNTIME)
+public @interface ClusterSingleton {
Review comment:
@chatman once again, this PR is NOT only about the autoscaling (even
though that was the original motivation). The whole point of this PR is for
Solr to provide a minimal facility that allows plugins to react to cluster-wide
events - and ONE particular application would be to re-implement the
autoscaling triggers.
Without this API each package (autoscaling or any other package that needs
it) would have to re-implement the notion of cluster-wide event monitoring and
generation.
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] sigram commented on a change in pull request #1758: SOLR-14749: Provide a clean API for cluster-level event processing, Initial draft.
sigram commented on a change in pull request #1758:
URL: https://github.com/apache/lucene-solr/pull/1758#discussion_r473852648
##
File path: solr/core/src/java/org/apache/solr/cluster/events/ScheduledEvent.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.events;
+
+/**
+ *
+ */
+public interface ScheduledEvent extends ClusterEvent {
Review comment:
> plugged into the autoscaling framework
This PR is NOT just about the autoscaling. The goal is to define a generic
API for cluster-level events. Autoscaling plugins will be just one consumer of
this API.
> IOW, ScheduledEvent should not be a first class concept/interface in
"solr-core"
I disagree. Scheduling various maintenance tasks is a common need (not just
in autoscaling), and it fits with the event API. Sure, we can define a separate
API just for scheduling, but it would be still something that belongs to
solr-core.
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] sigram commented on a change in pull request #1758: SOLR-14749: Provide a clean API for cluster-level event processing, Initial draft.
sigram commented on a change in pull request #1758:
URL: https://github.com/apache/lucene-solr/pull/1758#discussion_r473852648
##
File path: solr/core/src/java/org/apache/solr/cluster/events/ScheduledEvent.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.events;
+
+/**
+ *
+ */
+public interface ScheduledEvent extends ClusterEvent {
Review comment:
> plugged into the autoscaling framework
This PR is NOT just about the autoscaling. The goal is to define a generic
API for cluster-level events. Autoscaling plugins will be just one consumer of
this API.
> IOW, ScheduledEvent should not be a first class concept/interface in
"solr-core"
I disagree. Scheduling various maintenance tasks is a common need (not just
in autoscaling), and it fits with the event API. Sure, we can define a separate
API just for scheduling, but it would be still something that belongs to
solr-core.
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] sigram commented on a change in pull request #1758: SOLR-14749: Provide a clean API for cluster-level event processing, Initial draft.
sigram commented on a change in pull request #1758:
URL: https://github.com/apache/lucene-solr/pull/1758#discussion_r473848530
##
File path:
solr/core/src/java/org/apache/solr/cluster/events/ClusterSingleton.java
##
@@ -0,0 +1,14 @@
+package org.apache.solr.cluster.events;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+/**
+ * Intended for {@link org.apache.solr.core.CoreContainer} plugins that should
be
+ * enabled only one instance per cluster.
+ * Implementation detail: currently these plugins are instantiated on the
+ * Overseer leader, and closed when the current node loses its leadership.
+ */
+@Retention(RetentionPolicy.RUNTIME)
+public @interface ClusterSingleton {
Review comment:
@chatman once again, this PR is NOT about the autoscaling (even though
that was the original motivation). The whole point of this PR is for Solr to
provide a minimal facility that allows plugins to react to cluster-wide events
- and ONE particular application would be to re-implement the autoscaling
triggers.
Without this API each package (autoscaling or any other package that needs
it) would have to re-implement the notion of cluster-wide event monitoring and
generation.
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] sigram commented on a change in pull request #1758: SOLR-14749: Provide a clean API for cluster-level event processing, Initial draft.
sigram commented on a change in pull request #1758:
URL: https://github.com/apache/lucene-solr/pull/1758#discussion_r473682999
##
File path:
solr/core/src/java/org/apache/solr/cluster/events/ClusterSingleton.java
##
@@ -0,0 +1,14 @@
+package org.apache.solr.cluster.events;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+/**
+ * Intended for {@link org.apache.solr.core.CoreContainer} plugins that should
be
+ * enabled only one instance per cluster.
+ * Implementation detail: currently these plugins are instantiated on the
+ * Overseer leader, and closed when the current node loses its leadership.
+ */
+@Retention(RetentionPolicy.RUNTIME)
+public @interface ClusterSingleton {
Review comment:
@chatman this event API originated from the "triggers" API in 8x but can
equally well be used outside the autoscaling - so in the above diagram it
doesn't depend on autoscaling, it's adjacent to 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] sigram commented on a change in pull request #1758: SOLR-14749: Provide a clean API for cluster-level event processing, Initial draft.
sigram commented on a change in pull request #1758:
URL: https://github.com/apache/lucene-solr/pull/1758#discussion_r472436918
##
File path: solr/core/src/java/org/apache/solr/cluster/events/ScheduledEvent.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.events;
+
+/**
+ *
+ */
+public interface ScheduledEvent extends ClusterEvent {
Review comment:
Right, generally speaking any maintenance or optimization task that
needs to be periodically invoked. Again, we could define it as a separate API
but I think it fits the event model nicely.
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] sigram commented on a change in pull request #1758: SOLR-14749: Provide a clean API for cluster-level event processing, Initial draft.
sigram commented on a change in pull request #1758:
URL: https://github.com/apache/lucene-solr/pull/1758#discussion_r472435099
##
File path: solr/core/src/java/org/apache/solr/cloud/events/ScheduledEvent.java
##
@@ -0,0 +1,25 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.cloud.events;
+
+/**
+ *
+ */
+public interface ScheduledEvent extends ClusterEvent {
+ String getScheduleName();
+ Object getScheduleParam(String key);
Review comment:
@chatman even in 8x the scheduled events were used for things other than
autoscaling, namely for inactive shard maintenance. Solr still needs this
functionality in order to avoid implementing it over and over again in
individual plugins.
I don't have a strong opinion whether it must be a part of the event API
here, but it seems to me that it's convenient to model scheduled events in this
uniform way.
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] sigram commented on a change in pull request #1758: SOLR-14749: Provide a clean API for cluster-level event processing, Initial draft.
sigram commented on a change in pull request #1758:
URL: https://github.com/apache/lucene-solr/pull/1758#discussion_r472058053
##
File path: solr/core/src/java/org/apache/solr/cloud/events/ScheduledEvent.java
##
@@ -0,0 +1,25 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.cloud.events;
+
+/**
+ *
+ */
+public interface ScheduledEvent extends ClusterEvent {
+ String getScheduleName();
+ Object getScheduleParam(String key);
Review comment:
Thinking more about it, we should use a strongly-typed Schedule.
Scheduled event is something that Solr generates on a predefined schedule ;)
It's basically a general-purpose scheduler that is available via this API.
There's a need for some periodic maintenance tasks, and currently it was
implemented as a part of the autoscaling triggers (ScheduledTrigger, which in
turn invoked eg. inactive shard cleanup and collection repair tasks).
I'm open to other suggestions - we could make it a separate API, but
modelling it as a cluster event has benefits too (uniform way to register for
and to process events, and it avoids adding yet another API).
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] sigram commented on a change in pull request #1758: SOLR-14749: Provide a clean API for cluster-level event processing, Initial draft.
sigram commented on a change in pull request #1758:
URL: https://github.com/apache/lucene-solr/pull/1758#discussion_r472053144
##
File path: solr/core/src/java/org/apache/solr/cloud/events/NodeDownEvent.java
##
@@ -0,0 +1,24 @@
+/*
+ * 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.cloud.events;
+
+/**
+ *
+ */
+public interface NodeDownEvent extends ClusterEvent {
Review comment:
+1.
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
