[GitHub] [lucene-solr] noblepaul commented on a change in pull request #1962: SOLR-14749 Provide a clean API for cluster-level event processing
noblepaul commented on a change in pull request #1962:
URL: https://github.com/apache/lucene-solr/pull/1962#discussion_r515762968
##
File path:
solr/core/src/java/org/apache/solr/core/ClusterEventProducerFactory.java
##
@@ -0,0 +1,178 @@
+package org.apache.solr.core;
+
+import org.apache.solr.api.CustomContainerPlugins;
+import org.apache.solr.cluster.events.ClusterEvent;
+import org.apache.solr.cluster.events.ClusterEventListener;
+import org.apache.solr.cluster.events.ClusterEventProducer;
+import org.apache.solr.cluster.events.impl.DefaultClusterEventProducer;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * This class helps in handling the initial registration of plugin-based
listeners,
+ * when both the final {@link ClusterEventProducer} implementation and
listeners
+ * are configured using plugins.
+ */
+public class ClusterEventProducerFactory implements ClusterEventProducer {
+ private Map>
initialListeners = new HashMap<>();
+ private CustomContainerPlugins.PluginRegistryListener initialPluginListener;
+ private final CoreContainer cc;
+ private boolean created = false;
+
+ public ClusterEventProducerFactory(CoreContainer cc) {
+this.cc = cc;
+initialPluginListener = new
CustomContainerPlugins.PluginRegistryListener() {
+ @Override
+ public void added(CustomContainerPlugins.ApiInfo plugin) {
+if (plugin == null || plugin.getInstance() == null) {
+ return;
+}
+Object instance = plugin.getInstance();
+if (instance instanceof ClusterEventListener) {
+ registerListener((ClusterEventListener) instance);
+}
+ }
+
+ @Override
+ public void deleted(CustomContainerPlugins.ApiInfo plugin) {
+if (plugin == null || plugin.getInstance() == null) {
+ return;
+}
+Object instance = plugin.getInstance();
+if (instance instanceof ClusterEventListener) {
+ unregisterListener((ClusterEventListener) instance);
+}
+ }
+
+ @Override
+ public void modified(CustomContainerPlugins.ApiInfo old,
CustomContainerPlugins.ApiInfo replacement) {
+added(replacement);
+deleted(old);
+ }
+};
+ }
+
+ /**
+ * This method returns an initial plugin registry listener that helps to
capture the
+ * freshly loaded listener plugins before the final cluster event producer
is created.
+ * @return initial listener
+ */
+ public CustomContainerPlugins.PluginRegistryListener
getPluginRegistryListener() {
+return initialPluginListener;
+ }
+
+ /**
+ * Create a {@link ClusterEventProducer} based on the current plugin
configurations.
+ * NOTE: this method can only be called once because it has side-effects,
such as
+ * transferring the initially collected listeners to the resulting
producer's instance, and
+ * installing a {@link
org.apache.solr.api.CustomContainerPlugins.PluginRegistryListener}.
+ * Calling this method more than once will result in an exception.
+ * @param plugins current plugin configurations
+ * @return configured instance of cluster event producer (with side-effects,
see above)
+ */
+ public ClusterEventProducer create(CustomContainerPlugins plugins) {
+if (created) {
+ throw new RuntimeException("this factory can be called only once!");
+}
+final ClusterEventProducer clusterEventProducer;
+CustomContainerPlugins.ApiInfo clusterEventProducerInfo =
plugins.getPlugin(ClusterEventProducer.PLUGIN_NAME);
+if (clusterEventProducerInfo != null) {
+ // the listener in ClusterSingletons already registered it
+ clusterEventProducer = (ClusterEventProducer)
clusterEventProducerInfo.getInstance();
+} else {
+ // create the default impl
+ clusterEventProducer = new DefaultClusterEventProducer(cc);
Review comment:
NO , please.
I am -1 on enabling this by default
##
File path:
solr/core/src/java/org/apache/solr/core/ClusterEventProducerFactory.java
##
@@ -0,0 +1,178 @@
+package org.apache.solr.core;
+
+import org.apache.solr.api.CustomContainerPlugins;
+import org.apache.solr.cluster.events.ClusterEvent;
+import org.apache.solr.cluster.events.ClusterEventListener;
+import org.apache.solr.cluster.events.ClusterEventProducer;
+import org.apache.solr.cluster.events.impl.DefaultClusterEventProducer;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * This class helps in handling the initial registration of plugin-based
listeners,
+ * when both the final {@link ClusterEventProducer} implementation and
listeners
+ * are configured using plugins.
+ */
+public class ClusterEventProducerFactory implements ClusterEventProducer {
+ private Map>
initialListeners = new HashMap<>();
+ private CustomContainerPlugins.PluginRegistryListener initialPluginListener;
+ private final
[GitHub] [lucene-solr] noblepaul commented on a change in pull request #1962: SOLR-14749 Provide a clean API for cluster-level event processing
noblepaul commented on a change in pull request #1962:
URL: https://github.com/apache/lucene-solr/pull/1962#discussion_r513938423
##
File path:
solr/core/src/java/org/apache/solr/core/ClusterEventProducerFactory.java
##
@@ -0,0 +1,178 @@
+package org.apache.solr.core;
+
+import org.apache.solr.api.CustomContainerPlugins;
+import org.apache.solr.cluster.events.ClusterEvent;
+import org.apache.solr.cluster.events.ClusterEventListener;
+import org.apache.solr.cluster.events.ClusterEventProducer;
+import org.apache.solr.cluster.events.impl.DefaultClusterEventProducer;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * This class helps in handling the initial registration of plugin-based
listeners,
+ * when both the final {@link ClusterEventProducer} implementation and
listeners
+ * are configured using plugins.
+ */
+public class ClusterEventProducerFactory implements ClusterEventProducer {
+ private Map>
initialListeners = new HashMap<>();
+ private CustomContainerPlugins.PluginRegistryListener initialPluginListener;
+ private final CoreContainer cc;
+ private boolean created = false;
+
+ public ClusterEventProducerFactory(CoreContainer cc) {
+this.cc = cc;
+initialPluginListener = new
CustomContainerPlugins.PluginRegistryListener() {
+ @Override
+ public void added(CustomContainerPlugins.ApiInfo plugin) {
+if (plugin == null || plugin.getInstance() == null) {
+ return;
+}
+Object instance = plugin.getInstance();
+if (instance instanceof ClusterEventListener) {
+ registerListener((ClusterEventListener) instance);
+}
+ }
+
+ @Override
+ public void deleted(CustomContainerPlugins.ApiInfo plugin) {
+if (plugin == null || plugin.getInstance() == null) {
+ return;
+}
+Object instance = plugin.getInstance();
+if (instance instanceof ClusterEventListener) {
+ unregisterListener((ClusterEventListener) instance);
+}
+ }
+
+ @Override
+ public void modified(CustomContainerPlugins.ApiInfo old,
CustomContainerPlugins.ApiInfo replacement) {
+added(replacement);
+deleted(old);
+ }
+};
+ }
+
+ /**
+ * This method returns an initial plugin registry listener that helps to
capture the
+ * freshly loaded listener plugins before the final cluster event producer
is created.
+ * @return initial listener
+ */
+ public CustomContainerPlugins.PluginRegistryListener
getPluginRegistryListener() {
+return initialPluginListener;
+ }
+
+ /**
+ * Create a {@link ClusterEventProducer} based on the current plugin
configurations.
+ * NOTE: this method can only be called once because it has side-effects,
such as
+ * transferring the initially collected listeners to the resulting
producer's instance, and
+ * installing a {@link
org.apache.solr.api.CustomContainerPlugins.PluginRegistryListener}.
+ * Calling this method more than once will result in an exception.
+ * @param plugins current plugin configurations
+ * @return configured instance of cluster event producer (with side-effects,
see above)
+ */
+ public ClusterEventProducer create(CustomContainerPlugins plugins) {
+if (created) {
+ throw new RuntimeException("this factory can be called only once!");
+}
+final ClusterEventProducer clusterEventProducer;
+CustomContainerPlugins.ApiInfo clusterEventProducerInfo =
plugins.getPlugin(ClusterEventProducer.PLUGIN_NAME);
+if (clusterEventProducerInfo != null) {
+ // the listener in ClusterSingletons already registered it
+ clusterEventProducer = (ClusterEventProducer)
clusterEventProducerInfo.getInstance();
+} else {
+ // create the default impl
+ clusterEventProducer = new DefaultClusterEventProducer(cc);
Review comment:
> AFAIK there is no mechanism in plugins to require the presence of
another plugin.
There is no reason why we can't implement it. I think we should. Everything
should be initialized as lazily as possible.
The add command for a `ClusterEventListener` has to do an extra check to see
if a `ClusterEventProducer` is registered. If not, just register 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] noblepaul commented on a change in pull request #1962: SOLR-14749 Provide a clean API for cluster-level event processing
noblepaul commented on a change in pull request #1962:
URL: https://github.com/apache/lucene-solr/pull/1962#discussion_r512566935
##
File path:
solr/core/src/java/org/apache/solr/core/ClusterEventProducerFactory.java
##
@@ -0,0 +1,178 @@
+package org.apache.solr.core;
+
+import org.apache.solr.api.CustomContainerPlugins;
+import org.apache.solr.cluster.events.ClusterEvent;
+import org.apache.solr.cluster.events.ClusterEventListener;
+import org.apache.solr.cluster.events.ClusterEventProducer;
+import org.apache.solr.cluster.events.impl.DefaultClusterEventProducer;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * This class helps in handling the initial registration of plugin-based
listeners,
+ * when both the final {@link ClusterEventProducer} implementation and
listeners
+ * are configured using plugins.
+ */
+public class ClusterEventProducerFactory implements ClusterEventProducer {
+ private Map>
initialListeners = new HashMap<>();
+ private CustomContainerPlugins.PluginRegistryListener initialPluginListener;
+ private final CoreContainer cc;
+ private boolean created = false;
+
+ public ClusterEventProducerFactory(CoreContainer cc) {
+this.cc = cc;
+initialPluginListener = new
CustomContainerPlugins.PluginRegistryListener() {
+ @Override
+ public void added(CustomContainerPlugins.ApiInfo plugin) {
+if (plugin == null || plugin.getInstance() == null) {
+ return;
+}
+Object instance = plugin.getInstance();
+if (instance instanceof ClusterEventListener) {
+ registerListener((ClusterEventListener) instance);
+}
+ }
+
+ @Override
+ public void deleted(CustomContainerPlugins.ApiInfo plugin) {
+if (plugin == null || plugin.getInstance() == null) {
+ return;
+}
+Object instance = plugin.getInstance();
+if (instance instanceof ClusterEventListener) {
+ unregisterListener((ClusterEventListener) instance);
+}
+ }
+
+ @Override
+ public void modified(CustomContainerPlugins.ApiInfo old,
CustomContainerPlugins.ApiInfo replacement) {
+added(replacement);
+deleted(old);
+ }
+};
+ }
+
+ /**
+ * This method returns an initial plugin registry listener that helps to
capture the
+ * freshly loaded listener plugins before the final cluster event producer
is created.
+ * @return initial listener
+ */
+ public CustomContainerPlugins.PluginRegistryListener
getPluginRegistryListener() {
+return initialPluginListener;
+ }
+
+ /**
+ * Create a {@link ClusterEventProducer} based on the current plugin
configurations.
+ * NOTE: this method can only be called once because it has side-effects,
such as
+ * transferring the initially collected listeners to the resulting
producer's instance, and
+ * installing a {@link
org.apache.solr.api.CustomContainerPlugins.PluginRegistryListener}.
+ * Calling this method more than once will result in an exception.
+ * @param plugins current plugin configurations
+ * @return configured instance of cluster event producer (with side-effects,
see above)
+ */
+ public ClusterEventProducer create(CustomContainerPlugins plugins) {
+if (created) {
+ throw new RuntimeException("this factory can be called only once!");
+}
+final ClusterEventProducer clusterEventProducer;
+CustomContainerPlugins.ApiInfo clusterEventProducerInfo =
plugins.getPlugin(ClusterEventProducer.PLUGIN_NAME);
+if (clusterEventProducerInfo != null) {
+ // the listener in ClusterSingletons already registered it
+ clusterEventProducer = (ClusterEventProducer)
clusterEventProducerInfo.getInstance();
+} else {
+ // create the default impl
+ clusterEventProducer = new DefaultClusterEventProducer(cc);
Review comment:
This is a core , opt-in functionality
core = part of core
opt-in =. no traces if user does not explicitly add an event producer
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] noblepaul commented on a change in pull request #1962: SOLR-14749 Provide a clean API for cluster-level event processing
noblepaul commented on a change in pull request #1962:
URL: https://github.com/apache/lucene-solr/pull/1962#discussion_r512365317
##
File path:
solr/core/src/java/org/apache/solr/core/ClusterEventProducerFactory.java
##
@@ -0,0 +1,178 @@
+package org.apache.solr.core;
+
+import org.apache.solr.api.CustomContainerPlugins;
+import org.apache.solr.cluster.events.ClusterEvent;
+import org.apache.solr.cluster.events.ClusterEventListener;
+import org.apache.solr.cluster.events.ClusterEventProducer;
+import org.apache.solr.cluster.events.impl.DefaultClusterEventProducer;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * This class helps in handling the initial registration of plugin-based
listeners,
+ * when both the final {@link ClusterEventProducer} implementation and
listeners
+ * are configured using plugins.
+ */
+public class ClusterEventProducerFactory implements ClusterEventProducer {
+ private Map>
initialListeners = new HashMap<>();
+ private CustomContainerPlugins.PluginRegistryListener initialPluginListener;
+ private final CoreContainer cc;
+ private boolean created = false;
+
+ public ClusterEventProducerFactory(CoreContainer cc) {
+this.cc = cc;
+initialPluginListener = new
CustomContainerPlugins.PluginRegistryListener() {
+ @Override
+ public void added(CustomContainerPlugins.ApiInfo plugin) {
+if (plugin == null || plugin.getInstance() == null) {
+ return;
+}
+Object instance = plugin.getInstance();
+if (instance instanceof ClusterEventListener) {
+ registerListener((ClusterEventListener) instance);
+}
+ }
+
+ @Override
+ public void deleted(CustomContainerPlugins.ApiInfo plugin) {
+if (plugin == null || plugin.getInstance() == null) {
+ return;
+}
+Object instance = plugin.getInstance();
+if (instance instanceof ClusterEventListener) {
+ unregisterListener((ClusterEventListener) instance);
+}
+ }
+
+ @Override
+ public void modified(CustomContainerPlugins.ApiInfo old,
CustomContainerPlugins.ApiInfo replacement) {
+added(replacement);
+deleted(old);
+ }
+};
+ }
+
+ /**
+ * This method returns an initial plugin registry listener that helps to
capture the
+ * freshly loaded listener plugins before the final cluster event producer
is created.
+ * @return initial listener
+ */
+ public CustomContainerPlugins.PluginRegistryListener
getPluginRegistryListener() {
+return initialPluginListener;
+ }
+
+ /**
+ * Create a {@link ClusterEventProducer} based on the current plugin
configurations.
+ * NOTE: this method can only be called once because it has side-effects,
such as
+ * transferring the initially collected listeners to the resulting
producer's instance, and
+ * installing a {@link
org.apache.solr.api.CustomContainerPlugins.PluginRegistryListener}.
+ * Calling this method more than once will result in an exception.
+ * @param plugins current plugin configurations
+ * @return configured instance of cluster event producer (with side-effects,
see above)
+ */
+ public ClusterEventProducer create(CustomContainerPlugins plugins) {
+if (created) {
+ throw new RuntimeException("this factory can be called only once!");
+}
+final ClusterEventProducer clusterEventProducer;
+CustomContainerPlugins.ApiInfo clusterEventProducerInfo =
plugins.getPlugin(ClusterEventProducer.PLUGIN_NAME);
+if (clusterEventProducerInfo != null) {
+ // the listener in ClusterSingletons already registered it
+ clusterEventProducer = (ClusterEventProducer)
clusterEventProducerInfo.getInstance();
+} else {
+ // create the default impl
+ clusterEventProducer = new DefaultClusterEventProducer(cc);
Review comment:
There is no need to have a `ClusterEventProducer` if the user does not
register it.
##
File path: solr/core/src/java/org/apache/solr/core/CoreContainer.java
##
@@ -252,6 +253,11 @@ public CoreLoadFailure(CoreDescriptor cd, Exception
loadFailure) {
getZkController().getOverseer() != null &&
!getZkController().getOverseer().isClosed(),
(r) -> this.runAsync(r));
+
+ private final ClusterEventProducerFactory clusterEventProducerFactory = new
ClusterEventProducerFactory(this);
Review comment:
Does it have to be a field? It can be a local variable as well
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
