You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2021/08/17 14:05:26 UTC
[GitHub] [solr] gus-asf opened a new pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
gus-asf opened a new pull request #265:
URL: https://github.com/apache/solr/pull/265
https://issues.apache.org/jira/browse/SOLR-15590
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] markrmiller commented on pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
markrmiller commented on pull request #265:
URL: https://github.com/apache/solr/pull/265#issuecomment-907034437
> Seems like it should have generated a ticket with Jetty at least since while the spec doesn't actually state that the call to init() > must have been completed, that sure sounds like something the spec authors wouldn't have thought necessary to say...
I doubt it was ever actually their issue. I could care less about the webapp spec, webapp's are terrible, but filter init / process is fairly fundamental and unlikely to be gotten wrong by any container out there, other than perhaps a short lived high level blunder. Plenty of things up the stack they would be toast on if this was a standard problem.
More likely someone was looking at tests when they looked at that particular race. If you hotwire a filter into Jetty, you can mostly certainly do it in a way that will not follow any specs or rules, or sanity if you so choose or fall into.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gus-asf closed pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
gus-asf closed pull request #265:
URL: https://github.com/apache/solr/pull/265
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] sonatype-lift[bot] commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r690499793
##########
File path: solr/core/src/java/org/apache/solr/servlet/CoreService.java
##########
@@ -0,0 +1,451 @@
+/*
+ * 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.servlet;
+
+import com.codahale.metrics.jvm.ClassLoadingGaugeSet;
+import com.codahale.metrics.jvm.GarbageCollectorMetricSet;
+import com.codahale.metrics.jvm.MemoryUsageGaugeSet;
+import com.codahale.metrics.jvm.ThreadStatesGaugeSet;
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.http.client.HttpClient;
+import org.apache.lucene.util.Version;
+import org.apache.solr.cloud.ZkController;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrInfoBean.Group;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.metrics.AltBufferPoolMetricSet;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.OperatingSystemMetricSet;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricManager.ResolutionStrategy;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.servlet.RateLimitManager.Builder;
+import org.apache.solr.util.StartupLoggingUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
+import javax.naming.NoInitialContextException;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
+import javax.servlet.UnavailableException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.Set;
+import java.util.WeakHashMap;
+import java.util.concurrent.CountDownLatch;
+
+import static org.apache.solr.servlet.SolrCoreUtils.loadNodeConfig;
+import static org.apache.solr.servlet.SolrDispatchFilter.PROPERTIES_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLRHOME_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_INSTALL_DIR_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_LEVEL;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_MUTECONSOLE;
+
+/**
+ * A service that can provide access to solr cores. This allows us to have multiple filters and
+ * servlets that depend on SolrCore and CoreContainer, while still only having one CoreContainer per
+ * instance of solr.
+ */
+public class CoreService implements ServletContextListener {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private final String metricTag = SolrMetricProducer.getUniqueMetricTag(this, null);
+ private CoreContainer cores;
+ private Properties extraProperties;
+ private HttpClient httpClient;
+ private SolrMetricManager metricManager;
+ private RateLimitManager rateLimitManager;
+ private final CountDownLatch init = new CountDownLatch(1);
+ private String registryName;
+
+ // AFAIK the only reason we need this is to support JettySolrRunner for tests. In tests we might have
+ // multiple CoreContainers in the same JVM, but I *think* that doesn't happen in a real server.
+ private static final Map<ContextInitializationKey, ServiceHolder> services =
+ Collections.synchronizedMap(new WeakHashMap<>());
+
+ // todo: dependency injection instead, but CDI is not jetty native so for now this method and the associated
+ // map will have to suffice. Note that this relies on ServletContext.equals() not implementing anything significantly
+ // different thant Object.equals for its .equals method (I've found no implementation that even immplements it).
+ public static ServiceHolder serviceForContext(ServletContext ctx) throws InterruptedException {
+ ContextInitializationKey key = new ContextInitializationKey(ctx);
+ return services.computeIfAbsent(key, ServiceHolder::new);
+ }
+
+ @Override
+ public void contextInitialized(ServletContextEvent sce) {
+ init(sce.getServletContext());
+ }
+
+ @Override
+ public void contextDestroyed(ServletContextEvent sce) {
+ close();
+ }
+
+ CoreContainer getCoreContainer() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return cores;
+ }
+ HttpClient getHttpClient() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return httpClient;
+ }
+
+ private static void waitForCoreContainer(CoreContainerProvider provider, CountDownLatch latch) throws UnavailableException {
+ CoreContainer cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ try {
+ latch.await();
+ } catch (InterruptedException e) { //well, no wait then
+ }
+ cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ final String msg = "Error processing the request. CoreContainer is either not initialized or shutting down.";
+ log.error(msg);
+ throw new UnavailableException(msg);
+ }
+ }
+ }
+ public void close() {
+ CoreContainer cc = cores;
+ cores = null;
+ try {
+ if (metricManager != null) {
+ try {
+ metricManager.unregisterGauges(registryName, metricTag);
+ } catch (NullPointerException e) {
+ // okay
+ } catch (Exception e) {
+ log.warn("Exception closing FileCleaningTracker", e);
+ } finally {
+ metricManager = null;
+ }
+ }
+ } finally {
+ if (cc != null) {
+ httpClient = null;
+ cc.shutdown();
+ }
+ }
+ }
+
+ public void init(ServletContext servletContext) {
+ if (log.isTraceEnabled()) {
+ log.trace("CoreService.init(): {}", this.getClass().getClassLoader());
+ }
+ CoreContainer coresInit = null;
+ try {
+ // "extra" properties must be init'ed first so we know things like "do we have a zkHost"
+ // wrap as defaults (if set) so we can modify w/o polluting the Properties provided by our caller
+ this.extraProperties = SolrXmlConfig.wrapAndSetZkHostFromSysPropIfNeeded
+ ((Properties) servletContext.getAttribute(PROPERTIES_ATTRIBUTE));
+
+ StartupLoggingUtils.checkLogDir();
+ if (log.isInfoEnabled()) {
+ log.info("Using logger factory {}", StartupLoggingUtils.getLoggerImplStr());
+ }
+
+ logWelcomeBanner();
+
+ String muteConsole = System.getProperty(SOLR_LOG_MUTECONSOLE);
+ if (muteConsole != null && !Arrays.asList("false","0","off","no").contains(muteConsole.toLowerCase(Locale.ROOT))) {
+ StartupLoggingUtils.muteConsole();
+ }
+ String logLevel = System.getProperty(SOLR_LOG_LEVEL);
+ if (logLevel != null) {
+ log.info("Log level override, property solr.log.level={}", logLevel);
+ StartupLoggingUtils.changeLogLevel(logLevel);
+ }
+
+ coresInit = createCoreContainer(computeSolrHome(servletContext), extraProperties);
+ this.httpClient = coresInit.getUpdateShardHandler().getDefaultHttpClient();
+ setupJvmMetrics(coresInit);
+
+ SolrZkClient zkClient = null;
+ ZkController zkController = coresInit.getZkController();
+
+ if (zkController != null) {
+ zkClient = zkController.getZkClient();
+ }
+
+ Builder builder = new Builder(zkClient);
+
+ this.rateLimitManager = builder.build();
+
+ if (zkController != null) {
+ zkController.zkStateReader.registerClusterPropertiesListener(this.rateLimitManager);
+ }
+
+ if (log.isDebugEnabled()) {
+ log.debug("user.dir={}", System.getProperty("user.dir"));
+ }
+ } catch( Throwable t ) {
+ // catch this so our filter still works
+ log.error( "Could not start Solr. Check solr/home property and the logs");
+ SolrCore.log( t );
+ if (t instanceof Error) {
+ throw (Error) t;
+ }
+ } finally{
+ log.trace("SolrDispatchFilter.init() done");
+ this.cores = coresInit; // crucially final assignment
+ services.computeIfAbsent(new ContextInitializationKey(servletContext), ServiceHolder::new)
+ .setService(this);
+ init.countDown();
+ }
+ }
+
+
+ private void logWelcomeBanner() {
+ // _Really_ sorry about how clumsy this is as a result of the logging call checker, but this is the only one
+ // that's so ugly so far.
+ if (log.isInfoEnabled()) {
+ log.info(" ___ _ Welcome to Apache Solr™ version {}", solrVersion());
+ }
+ if (log.isInfoEnabled()) {
+ log.info("/ __| ___| |_ _ Starting in {} mode on port {}", isCloudMode() ? "cloud" : "standalone", getSolrPort());
+ }
+ if (log.isInfoEnabled()) {
+ log.info("\\__ \\/ _ \\ | '_| Install dir: {}", System.getProperty(SOLR_INSTALL_DIR_ATTRIBUTE));
+ }
+ if (log.isInfoEnabled()) {
+ log.info("|___/\\___/_|_| Start time: {}", Instant.now());
+ }
+ }
+ private String solrVersion() {
+ String specVer = Version.LATEST.toString();
+ try {
+ String implVer = SolrCore.class.getPackage().getImplementationVersion();
+ return (specVer.equals(implVer.split(" ")[0])) ? specVer : implVer;
+ } catch (Exception e) {
+ return specVer;
+ }
+ }
+
+ private String getSolrPort() {
+ return System.getProperty("jetty.port");
+ }
+
+ /**
+ * We are in cloud mode if Java option zkRun exists OR zkHost exists and is non-empty
+ * @see SolrXmlConfig#wrapAndSetZkHostFromSysPropIfNeeded
+ * @see #extraProperties
+ * @see #init
+ */
+ private boolean isCloudMode() {
+ assert null != extraProperties; // we should never be called w/o this being initialized
+ return (null != extraProperties.getProperty(SolrXmlConfig.ZK_HOST)) || (null != System.getProperty("zkRun"));
+ }
+
+ /**
+ * Returns the effective Solr Home to use for this node, based on looking up the value in this order:
+ * <ol>
+ * <li>attribute in the FilterConfig</li>
+ * <li>JNDI: via java:comp/env/solr/home</li>
+ * <li>The system property solr.solr.home</li>
+ * <li>Look in the current working directory for a solr/ directory</li>
+ * </ol>
+ * <p>
+ *
+ * @return the Solr home, absolute and normalized.
+ */
+ private static Path computeSolrHome(ServletContext servletContext) {
+
+ // start with explicit check of servlet config...
+ String source = "servlet config: " + SOLRHOME_ATTRIBUTE;
+ String home = (String) servletContext.getAttribute(SOLRHOME_ATTRIBUTE);
+
+ if (null == home) {
+ final String lookup = "java:comp/env/solr/home";
+ // Try JNDI
+ source = "JNDI: " + lookup;
+ try {
+ Context c = new InitialContext();
+ home = (String) c.lookup(lookup);
+ } catch (NoInitialContextException e) {
+ log.debug("JNDI not configured for solr (NoInitialContextEx)");
+ } catch (NamingException e) {
+ log.debug("No /solr/home in JNDI");
+ } catch (RuntimeException ex) {
+ log.warn("Odd RuntimeException while testing for JNDI: ", ex);
+ }
+ }
+
+ if (null == home) {
+ // Now try system property
+ final String prop = "solr.solr.home";
+ source = "system property: " + prop;
+ home = System.getProperty(prop);
+ }
+
+ if (null == home) {
+ // if all else fails, assume default dir
+ home = "solr/";
+ source = "defaulted to '" + home + "' ... could not find system property or JNDI";
+ }
+ final Path solrHome = Paths.get(home).toAbsolutePath().normalize();
Review comment:
*PATH_TRAVERSAL_IN:* This API (java/nio/file/Paths.get(Ljava/lang/String;[Ljava/lang/String;)Ljava/nio/file/Path;) reads a file whose location might be specified by user input [(details)](https://find-sec-bugs.github.io/bugs.htm#PATH_TRAVERSAL_IN)
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java
##########
@@ -382,7 +386,8 @@ public void lifeCycleStarted(LifeCycle arg0) {
root.getServletContext().setAttribute(SolrDispatchFilter.PROPERTIES_ATTRIBUTE, nodeProperties);
root.getServletContext().setAttribute(SolrDispatchFilter.SOLRHOME_ATTRIBUTE, solrHome);
-
+ coreService = new CoreService();
+ coreService.init(root.getServletContext());
Review comment:
*THREAD_SAFETY_VIOLATION:* Unprotected write. Non-private method `JettySolrRunner$1.lifeCycleStarted(...)` indirectly writes to field `sql.CalciteSolrDriver.INSTANCE.solrClientCache` outside of synchronization.
Reporting because this access may occur on a background thread.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java
##########
@@ -382,7 +386,8 @@ public void lifeCycleStarted(LifeCycle arg0) {
root.getServletContext().setAttribute(SolrDispatchFilter.PROPERTIES_ATTRIBUTE, nodeProperties);
root.getServletContext().setAttribute(SolrDispatchFilter.SOLRHOME_ATTRIBUTE, solrHome);
-
+ coreService = new CoreService();
Review comment:
*THREAD_SAFETY_VIOLATION:* Unprotected write. Non-private method `JettySolrRunner$1.lifeCycleStarted(...)` writes to field `this.this$0.coreService` outside of synchronization.
Reporting because this access may occur on a background thread.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java
##########
@@ -364,7 +367,9 @@ public void lifeCycleStopping(LifeCycle arg0) {
}
@Override
- public void lifeCycleStopped(LifeCycle arg0) {}
+ public void lifeCycleStopped(LifeCycle arg0) {
+ coreService.close();
Review comment:
*THREAD_SAFETY_VIOLATION:* Unprotected write. Non-private method `JettySolrRunner$1.lifeCycleStopped(...)` indirectly writes to field `noggit.JSONParser.devNull.buf` outside of synchronization.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java
##########
@@ -638,19 +647,15 @@ public void stop() throws Exception {
if (sdf != null) {
customThreadPool = ExecutorUtil.newMDCAwareCachedThreadPool(new SolrNamedThreadFactory("jettyShutDown"));
- sdf.closeOnDestroy(false);
+ sdf.closeOnDestroy();
// customThreadPool.submit(() -> {
// try {
// sdf.close();
// } catch (Throwable t) {
// log.error("Error shutting down Solr", t);
// }
// });
- try {
- sdf.close();
- } catch (Throwable t) {
- log.error("Error shutting down Solr", t);
- }
+
}
QueuedThreadPool qtp = (QueuedThreadPool) server.getThreadPool();
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `JettySolrRunner.stop()` reads without synchronization from `this.server`. Potentially races with write in method `JettySolrRunner.start(...)`.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/servlet/CoreService.java
##########
@@ -0,0 +1,451 @@
+/*
+ * 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.servlet;
+
+import com.codahale.metrics.jvm.ClassLoadingGaugeSet;
+import com.codahale.metrics.jvm.GarbageCollectorMetricSet;
+import com.codahale.metrics.jvm.MemoryUsageGaugeSet;
+import com.codahale.metrics.jvm.ThreadStatesGaugeSet;
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.http.client.HttpClient;
+import org.apache.lucene.util.Version;
+import org.apache.solr.cloud.ZkController;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrInfoBean.Group;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.metrics.AltBufferPoolMetricSet;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.OperatingSystemMetricSet;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricManager.ResolutionStrategy;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.servlet.RateLimitManager.Builder;
+import org.apache.solr.util.StartupLoggingUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
+import javax.naming.NoInitialContextException;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
+import javax.servlet.UnavailableException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.Set;
+import java.util.WeakHashMap;
+import java.util.concurrent.CountDownLatch;
+
+import static org.apache.solr.servlet.SolrCoreUtils.loadNodeConfig;
+import static org.apache.solr.servlet.SolrDispatchFilter.PROPERTIES_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLRHOME_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_INSTALL_DIR_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_LEVEL;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_MUTECONSOLE;
+
+/**
+ * A service that can provide access to solr cores. This allows us to have multiple filters and
+ * servlets that depend on SolrCore and CoreContainer, while still only having one CoreContainer per
+ * instance of solr.
+ */
+public class CoreService implements ServletContextListener {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private final String metricTag = SolrMetricProducer.getUniqueMetricTag(this, null);
+ private CoreContainer cores;
+ private Properties extraProperties;
+ private HttpClient httpClient;
+ private SolrMetricManager metricManager;
+ private RateLimitManager rateLimitManager;
+ private final CountDownLatch init = new CountDownLatch(1);
+ private String registryName;
+
+ // AFAIK the only reason we need this is to support JettySolrRunner for tests. In tests we might have
+ // multiple CoreContainers in the same JVM, but I *think* that doesn't happen in a real server.
+ private static final Map<ContextInitializationKey, ServiceHolder> services =
+ Collections.synchronizedMap(new WeakHashMap<>());
+
+ // todo: dependency injection instead, but CDI is not jetty native so for now this method and the associated
+ // map will have to suffice. Note that this relies on ServletContext.equals() not implementing anything significantly
+ // different thant Object.equals for its .equals method (I've found no implementation that even immplements it).
+ public static ServiceHolder serviceForContext(ServletContext ctx) throws InterruptedException {
+ ContextInitializationKey key = new ContextInitializationKey(ctx);
+ return services.computeIfAbsent(key, ServiceHolder::new);
+ }
+
+ @Override
+ public void contextInitialized(ServletContextEvent sce) {
+ init(sce.getServletContext());
+ }
+
+ @Override
+ public void contextDestroyed(ServletContextEvent sce) {
+ close();
+ }
+
+ CoreContainer getCoreContainer() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return cores;
+ }
+ HttpClient getHttpClient() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return httpClient;
+ }
+
+ private static void waitForCoreContainer(CoreContainerProvider provider, CountDownLatch latch) throws UnavailableException {
+ CoreContainer cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ try {
+ latch.await();
+ } catch (InterruptedException e) { //well, no wait then
+ }
+ cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ final String msg = "Error processing the request. CoreContainer is either not initialized or shutting down.";
+ log.error(msg);
+ throw new UnavailableException(msg);
+ }
+ }
+ }
+ public void close() {
+ CoreContainer cc = cores;
+ cores = null;
+ try {
+ if (metricManager != null) {
+ try {
+ metricManager.unregisterGauges(registryName, metricTag);
+ } catch (NullPointerException e) {
+ // okay
+ } catch (Exception e) {
+ log.warn("Exception closing FileCleaningTracker", e);
+ } finally {
+ metricManager = null;
+ }
+ }
+ } finally {
+ if (cc != null) {
+ httpClient = null;
+ cc.shutdown();
+ }
+ }
+ }
+
+ public void init(ServletContext servletContext) {
+ if (log.isTraceEnabled()) {
+ log.trace("CoreService.init(): {}", this.getClass().getClassLoader());
+ }
+ CoreContainer coresInit = null;
+ try {
+ // "extra" properties must be init'ed first so we know things like "do we have a zkHost"
+ // wrap as defaults (if set) so we can modify w/o polluting the Properties provided by our caller
+ this.extraProperties = SolrXmlConfig.wrapAndSetZkHostFromSysPropIfNeeded
+ ((Properties) servletContext.getAttribute(PROPERTIES_ATTRIBUTE));
+
+ StartupLoggingUtils.checkLogDir();
+ if (log.isInfoEnabled()) {
+ log.info("Using logger factory {}", StartupLoggingUtils.getLoggerImplStr());
+ }
+
+ logWelcomeBanner();
+
+ String muteConsole = System.getProperty(SOLR_LOG_MUTECONSOLE);
+ if (muteConsole != null && !Arrays.asList("false","0","off","no").contains(muteConsole.toLowerCase(Locale.ROOT))) {
+ StartupLoggingUtils.muteConsole();
+ }
+ String logLevel = System.getProperty(SOLR_LOG_LEVEL);
+ if (logLevel != null) {
+ log.info("Log level override, property solr.log.level={}", logLevel);
+ StartupLoggingUtils.changeLogLevel(logLevel);
+ }
+
+ coresInit = createCoreContainer(computeSolrHome(servletContext), extraProperties);
+ this.httpClient = coresInit.getUpdateShardHandler().getDefaultHttpClient();
+ setupJvmMetrics(coresInit);
+
+ SolrZkClient zkClient = null;
+ ZkController zkController = coresInit.getZkController();
+
+ if (zkController != null) {
+ zkClient = zkController.getZkClient();
+ }
+
+ Builder builder = new Builder(zkClient);
+
+ this.rateLimitManager = builder.build();
+
+ if (zkController != null) {
+ zkController.zkStateReader.registerClusterPropertiesListener(this.rateLimitManager);
+ }
+
+ if (log.isDebugEnabled()) {
+ log.debug("user.dir={}", System.getProperty("user.dir"));
+ }
+ } catch( Throwable t ) {
+ // catch this so our filter still works
+ log.error( "Could not start Solr. Check solr/home property and the logs");
+ SolrCore.log( t );
+ if (t instanceof Error) {
+ throw (Error) t;
+ }
+ } finally{
+ log.trace("SolrDispatchFilter.init() done");
+ this.cores = coresInit; // crucially final assignment
+ services.computeIfAbsent(new ContextInitializationKey(servletContext), ServiceHolder::new)
+ .setService(this);
+ init.countDown();
+ }
+ }
+
+
+ private void logWelcomeBanner() {
+ // _Really_ sorry about how clumsy this is as a result of the logging call checker, but this is the only one
+ // that's so ugly so far.
+ if (log.isInfoEnabled()) {
+ log.info(" ___ _ Welcome to Apache Solr™ version {}", solrVersion());
+ }
+ if (log.isInfoEnabled()) {
+ log.info("/ __| ___| |_ _ Starting in {} mode on port {}", isCloudMode() ? "cloud" : "standalone", getSolrPort());
+ }
+ if (log.isInfoEnabled()) {
+ log.info("\\__ \\/ _ \\ | '_| Install dir: {}", System.getProperty(SOLR_INSTALL_DIR_ATTRIBUTE));
+ }
+ if (log.isInfoEnabled()) {
+ log.info("|___/\\___/_|_| Start time: {}", Instant.now());
+ }
+ }
+ private String solrVersion() {
+ String specVer = Version.LATEST.toString();
+ try {
+ String implVer = SolrCore.class.getPackage().getImplementationVersion();
+ return (specVer.equals(implVer.split(" ")[0])) ? specVer : implVer;
+ } catch (Exception e) {
+ return specVer;
+ }
+ }
+
+ private String getSolrPort() {
+ return System.getProperty("jetty.port");
+ }
+
+ /**
+ * We are in cloud mode if Java option zkRun exists OR zkHost exists and is non-empty
+ * @see SolrXmlConfig#wrapAndSetZkHostFromSysPropIfNeeded
+ * @see #extraProperties
+ * @see #init
+ */
+ private boolean isCloudMode() {
+ assert null != extraProperties; // we should never be called w/o this being initialized
+ return (null != extraProperties.getProperty(SolrXmlConfig.ZK_HOST)) || (null != System.getProperty("zkRun"));
+ }
+
+ /**
+ * Returns the effective Solr Home to use for this node, based on looking up the value in this order:
+ * <ol>
+ * <li>attribute in the FilterConfig</li>
+ * <li>JNDI: via java:comp/env/solr/home</li>
+ * <li>The system property solr.solr.home</li>
+ * <li>Look in the current working directory for a solr/ directory</li>
+ * </ol>
+ * <p>
+ *
+ * @return the Solr home, absolute and normalized.
+ */
+ private static Path computeSolrHome(ServletContext servletContext) {
+
+ // start with explicit check of servlet config...
+ String source = "servlet config: " + SOLRHOME_ATTRIBUTE;
+ String home = (String) servletContext.getAttribute(SOLRHOME_ATTRIBUTE);
+
+ if (null == home) {
+ final String lookup = "java:comp/env/solr/home";
+ // Try JNDI
+ source = "JNDI: " + lookup;
+ try {
+ Context c = new InitialContext();
+ home = (String) c.lookup(lookup);
+ } catch (NoInitialContextException e) {
+ log.debug("JNDI not configured for solr (NoInitialContextEx)");
+ } catch (NamingException e) {
+ log.debug("No /solr/home in JNDI");
+ } catch (RuntimeException ex) {
+ log.warn("Odd RuntimeException while testing for JNDI: ", ex);
+ }
+ }
+
+ if (null == home) {
+ // Now try system property
+ final String prop = "solr.solr.home";
+ source = "system property: " + prop;
+ home = System.getProperty(prop);
+ }
+
+ if (null == home) {
+ // if all else fails, assume default dir
+ home = "solr/";
+ source = "defaulted to '" + home + "' ... could not find system property or JNDI";
+ }
+ final Path solrHome = Paths.get(home).toAbsolutePath().normalize();
+ log.info("Solr Home: {} (source: {})", solrHome, source);
+
+ return solrHome;
+ }
+
+ /**
+ * Override this to change CoreContainer initialization
+ * @return a CoreContainer to hold this server's cores
+ */
+ protected CoreContainer createCoreContainer(Path solrHome, Properties nodeProps) {
+ NodeConfig nodeConfig = loadNodeConfig(solrHome, nodeProps);
+ final CoreContainer coreContainer = new CoreContainer(nodeConfig, true);
+ coreContainer.load();
+ return coreContainer;
+ }
+
+
+
+ private void setupJvmMetrics(CoreContainer coresInit) {
+ metricManager = coresInit.getMetricManager();
+ registryName = SolrMetricManager.getRegistryName(Group.jvm);
+ final Set<String> hiddenSysProps = coresInit.getConfig().getMetricsConfig().getHiddenSysProps();
+ try {
+ metricManager.registerAll(registryName, new AltBufferPoolMetricSet(), ResolutionStrategy.IGNORE, "buffers");
+ metricManager.registerAll(registryName, new ClassLoadingGaugeSet(), ResolutionStrategy.IGNORE, "classes");
+ metricManager.registerAll(registryName, new OperatingSystemMetricSet(), ResolutionStrategy.IGNORE, "os");
+ metricManager.registerAll(registryName, new GarbageCollectorMetricSet(), ResolutionStrategy.IGNORE, "gc");
+ metricManager.registerAll(registryName, new MemoryUsageGaugeSet(), ResolutionStrategy.IGNORE, "memory");
+ metricManager.registerAll(registryName, new ThreadStatesGaugeSet(), ResolutionStrategy.IGNORE, "threads"); // todo should we use CachedThreadStatesGaugeSet instead?
+ MetricsMap sysprops = new MetricsMap(map -> System.getProperties().forEach((k, v) -> {
+ //noinspection SuspiciousMethodCalls
+ if (!hiddenSysProps.contains(k)) {
+ map.putNoEx(String.valueOf(k), v);
+ }
+ }));
+ metricManager.registerGauge(null, registryName, sysprops, metricTag, ResolutionStrategy.IGNORE, "properties", "system");
+ MetricsMap sysenv = new MetricsMap(map -> System.getenv().forEach((k, v) -> {
+ if (!hiddenSysProps.contains(k)) {
+ map.putNoEx(String.valueOf(k), v);
+ }
+ }));
+ metricManager.registerGauge(null, registryName, sysenv, metricTag, ResolutionStrategy.IGNORE, "env", "system");
+ } catch (Exception e) {
+ log.warn("Error registering JVM metrics", e);
+ }
+ }
+
+ public RateLimitManager getRateLimitManager() {
+ return rateLimitManager;
+ }
+
+ @VisibleForTesting
+ void setRateLimitManager(RateLimitManager rateLimitManager) {
+ this.rateLimitManager = rateLimitManager;
+ }
+
+ private static class ContextInitializationKey {
+ private ServletContext ctx;
+ volatile CountDownLatch initializing = new CountDownLatch(1);
+
+ private ContextInitializationKey(ServletContext ctx) {
+ if (ctx == null) {
+ throw new IllegalArgumentException("Context must not be null");
+ }
+ // if one of these is reachable both must be to avoid collection from weak hashmap, so
+ // set an attribute holding this object to ensure we never get collected until the ServletContext
+ // is eligible for collection too.
+ ctx.setAttribute(this.getClass().getName(), this);
+ this.ctx = ctx;
+ }
+
+ public synchronized ServletContext getCtx() {
+ return ctx;
+ }
+
+ public synchronized void setCtx(ServletContext ctx) {
+ this.ctx = ctx;
+ }
+
+ synchronized void makeReady() {
+ this.initializing.countDown();
+ }
+
+ // NOT synchronized :)
+ public void waitForReadyService() throws InterruptedException {
+ initializing.await();
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ ContextInitializationKey that = (ContextInitializationKey) o;
+ return ctx.equals(that.ctx);
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `CoreService$ContextInitializationKey.equals(...)` reads without synchronization from `this.ctx`. Potentially races with write in method `CoreService$ContextInitializationKey.setCtx(...)`.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/servlet/CoreService.java
##########
@@ -0,0 +1,451 @@
+/*
+ * 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.servlet;
+
+import com.codahale.metrics.jvm.ClassLoadingGaugeSet;
+import com.codahale.metrics.jvm.GarbageCollectorMetricSet;
+import com.codahale.metrics.jvm.MemoryUsageGaugeSet;
+import com.codahale.metrics.jvm.ThreadStatesGaugeSet;
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.http.client.HttpClient;
+import org.apache.lucene.util.Version;
+import org.apache.solr.cloud.ZkController;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrInfoBean.Group;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.metrics.AltBufferPoolMetricSet;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.OperatingSystemMetricSet;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricManager.ResolutionStrategy;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.servlet.RateLimitManager.Builder;
+import org.apache.solr.util.StartupLoggingUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
+import javax.naming.NoInitialContextException;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
+import javax.servlet.UnavailableException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.Set;
+import java.util.WeakHashMap;
+import java.util.concurrent.CountDownLatch;
+
+import static org.apache.solr.servlet.SolrCoreUtils.loadNodeConfig;
+import static org.apache.solr.servlet.SolrDispatchFilter.PROPERTIES_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLRHOME_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_INSTALL_DIR_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_LEVEL;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_MUTECONSOLE;
+
+/**
+ * A service that can provide access to solr cores. This allows us to have multiple filters and
+ * servlets that depend on SolrCore and CoreContainer, while still only having one CoreContainer per
+ * instance of solr.
+ */
+public class CoreService implements ServletContextListener {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private final String metricTag = SolrMetricProducer.getUniqueMetricTag(this, null);
+ private CoreContainer cores;
+ private Properties extraProperties;
+ private HttpClient httpClient;
+ private SolrMetricManager metricManager;
+ private RateLimitManager rateLimitManager;
+ private final CountDownLatch init = new CountDownLatch(1);
+ private String registryName;
+
+ // AFAIK the only reason we need this is to support JettySolrRunner for tests. In tests we might have
+ // multiple CoreContainers in the same JVM, but I *think* that doesn't happen in a real server.
+ private static final Map<ContextInitializationKey, ServiceHolder> services =
+ Collections.synchronizedMap(new WeakHashMap<>());
+
+ // todo: dependency injection instead, but CDI is not jetty native so for now this method and the associated
+ // map will have to suffice. Note that this relies on ServletContext.equals() not implementing anything significantly
+ // different thant Object.equals for its .equals method (I've found no implementation that even immplements it).
+ public static ServiceHolder serviceForContext(ServletContext ctx) throws InterruptedException {
+ ContextInitializationKey key = new ContextInitializationKey(ctx);
+ return services.computeIfAbsent(key, ServiceHolder::new);
+ }
+
+ @Override
+ public void contextInitialized(ServletContextEvent sce) {
+ init(sce.getServletContext());
+ }
+
+ @Override
+ public void contextDestroyed(ServletContextEvent sce) {
+ close();
+ }
+
+ CoreContainer getCoreContainer() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return cores;
+ }
+ HttpClient getHttpClient() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return httpClient;
+ }
+
+ private static void waitForCoreContainer(CoreContainerProvider provider, CountDownLatch latch) throws UnavailableException {
+ CoreContainer cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ try {
+ latch.await();
+ } catch (InterruptedException e) { //well, no wait then
+ }
+ cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ final String msg = "Error processing the request. CoreContainer is either not initialized or shutting down.";
+ log.error(msg);
+ throw new UnavailableException(msg);
+ }
+ }
+ }
+ public void close() {
+ CoreContainer cc = cores;
+ cores = null;
+ try {
+ if (metricManager != null) {
+ try {
+ metricManager.unregisterGauges(registryName, metricTag);
+ } catch (NullPointerException e) {
+ // okay
+ } catch (Exception e) {
+ log.warn("Exception closing FileCleaningTracker", e);
+ } finally {
+ metricManager = null;
+ }
+ }
+ } finally {
+ if (cc != null) {
+ httpClient = null;
+ cc.shutdown();
+ }
+ }
+ }
+
+ public void init(ServletContext servletContext) {
+ if (log.isTraceEnabled()) {
+ log.trace("CoreService.init(): {}", this.getClass().getClassLoader());
+ }
+ CoreContainer coresInit = null;
+ try {
+ // "extra" properties must be init'ed first so we know things like "do we have a zkHost"
+ // wrap as defaults (if set) so we can modify w/o polluting the Properties provided by our caller
+ this.extraProperties = SolrXmlConfig.wrapAndSetZkHostFromSysPropIfNeeded
+ ((Properties) servletContext.getAttribute(PROPERTIES_ATTRIBUTE));
+
+ StartupLoggingUtils.checkLogDir();
+ if (log.isInfoEnabled()) {
+ log.info("Using logger factory {}", StartupLoggingUtils.getLoggerImplStr());
+ }
+
+ logWelcomeBanner();
+
+ String muteConsole = System.getProperty(SOLR_LOG_MUTECONSOLE);
+ if (muteConsole != null && !Arrays.asList("false","0","off","no").contains(muteConsole.toLowerCase(Locale.ROOT))) {
+ StartupLoggingUtils.muteConsole();
+ }
+ String logLevel = System.getProperty(SOLR_LOG_LEVEL);
+ if (logLevel != null) {
+ log.info("Log level override, property solr.log.level={}", logLevel);
+ StartupLoggingUtils.changeLogLevel(logLevel);
+ }
+
+ coresInit = createCoreContainer(computeSolrHome(servletContext), extraProperties);
+ this.httpClient = coresInit.getUpdateShardHandler().getDefaultHttpClient();
+ setupJvmMetrics(coresInit);
+
+ SolrZkClient zkClient = null;
+ ZkController zkController = coresInit.getZkController();
+
+ if (zkController != null) {
+ zkClient = zkController.getZkClient();
+ }
+
+ Builder builder = new Builder(zkClient);
+
+ this.rateLimitManager = builder.build();
+
+ if (zkController != null) {
+ zkController.zkStateReader.registerClusterPropertiesListener(this.rateLimitManager);
+ }
+
+ if (log.isDebugEnabled()) {
+ log.debug("user.dir={}", System.getProperty("user.dir"));
+ }
+ } catch( Throwable t ) {
+ // catch this so our filter still works
+ log.error( "Could not start Solr. Check solr/home property and the logs");
+ SolrCore.log( t );
+ if (t instanceof Error) {
+ throw (Error) t;
+ }
+ } finally{
+ log.trace("SolrDispatchFilter.init() done");
+ this.cores = coresInit; // crucially final assignment
+ services.computeIfAbsent(new ContextInitializationKey(servletContext), ServiceHolder::new)
+ .setService(this);
+ init.countDown();
+ }
+ }
+
+
+ private void logWelcomeBanner() {
+ // _Really_ sorry about how clumsy this is as a result of the logging call checker, but this is the only one
+ // that's so ugly so far.
+ if (log.isInfoEnabled()) {
+ log.info(" ___ _ Welcome to Apache Solr™ version {}", solrVersion());
+ }
+ if (log.isInfoEnabled()) {
+ log.info("/ __| ___| |_ _ Starting in {} mode on port {}", isCloudMode() ? "cloud" : "standalone", getSolrPort());
+ }
+ if (log.isInfoEnabled()) {
+ log.info("\\__ \\/ _ \\ | '_| Install dir: {}", System.getProperty(SOLR_INSTALL_DIR_ATTRIBUTE));
+ }
+ if (log.isInfoEnabled()) {
+ log.info("|___/\\___/_|_| Start time: {}", Instant.now());
+ }
+ }
+ private String solrVersion() {
+ String specVer = Version.LATEST.toString();
+ try {
+ String implVer = SolrCore.class.getPackage().getImplementationVersion();
+ return (specVer.equals(implVer.split(" ")[0])) ? specVer : implVer;
+ } catch (Exception e) {
+ return specVer;
+ }
+ }
+
+ private String getSolrPort() {
+ return System.getProperty("jetty.port");
+ }
+
+ /**
+ * We are in cloud mode if Java option zkRun exists OR zkHost exists and is non-empty
+ * @see SolrXmlConfig#wrapAndSetZkHostFromSysPropIfNeeded
+ * @see #extraProperties
+ * @see #init
+ */
+ private boolean isCloudMode() {
+ assert null != extraProperties; // we should never be called w/o this being initialized
+ return (null != extraProperties.getProperty(SolrXmlConfig.ZK_HOST)) || (null != System.getProperty("zkRun"));
+ }
+
+ /**
+ * Returns the effective Solr Home to use for this node, based on looking up the value in this order:
+ * <ol>
+ * <li>attribute in the FilterConfig</li>
+ * <li>JNDI: via java:comp/env/solr/home</li>
+ * <li>The system property solr.solr.home</li>
+ * <li>Look in the current working directory for a solr/ directory</li>
+ * </ol>
+ * <p>
+ *
+ * @return the Solr home, absolute and normalized.
+ */
+ private static Path computeSolrHome(ServletContext servletContext) {
+
+ // start with explicit check of servlet config...
+ String source = "servlet config: " + SOLRHOME_ATTRIBUTE;
+ String home = (String) servletContext.getAttribute(SOLRHOME_ATTRIBUTE);
+
+ if (null == home) {
+ final String lookup = "java:comp/env/solr/home";
+ // Try JNDI
+ source = "JNDI: " + lookup;
+ try {
+ Context c = new InitialContext();
+ home = (String) c.lookup(lookup);
+ } catch (NoInitialContextException e) {
+ log.debug("JNDI not configured for solr (NoInitialContextEx)");
+ } catch (NamingException e) {
+ log.debug("No /solr/home in JNDI");
+ } catch (RuntimeException ex) {
+ log.warn("Odd RuntimeException while testing for JNDI: ", ex);
+ }
+ }
+
+ if (null == home) {
+ // Now try system property
+ final String prop = "solr.solr.home";
+ source = "system property: " + prop;
+ home = System.getProperty(prop);
+ }
+
+ if (null == home) {
+ // if all else fails, assume default dir
+ home = "solr/";
+ source = "defaulted to '" + home + "' ... could not find system property or JNDI";
+ }
+ final Path solrHome = Paths.get(home).toAbsolutePath().normalize();
+ log.info("Solr Home: {} (source: {})", solrHome, source);
+
+ return solrHome;
+ }
+
+ /**
+ * Override this to change CoreContainer initialization
+ * @return a CoreContainer to hold this server's cores
+ */
+ protected CoreContainer createCoreContainer(Path solrHome, Properties nodeProps) {
+ NodeConfig nodeConfig = loadNodeConfig(solrHome, nodeProps);
+ final CoreContainer coreContainer = new CoreContainer(nodeConfig, true);
+ coreContainer.load();
+ return coreContainer;
+ }
+
+
+
+ private void setupJvmMetrics(CoreContainer coresInit) {
+ metricManager = coresInit.getMetricManager();
+ registryName = SolrMetricManager.getRegistryName(Group.jvm);
+ final Set<String> hiddenSysProps = coresInit.getConfig().getMetricsConfig().getHiddenSysProps();
+ try {
+ metricManager.registerAll(registryName, new AltBufferPoolMetricSet(), ResolutionStrategy.IGNORE, "buffers");
+ metricManager.registerAll(registryName, new ClassLoadingGaugeSet(), ResolutionStrategy.IGNORE, "classes");
+ metricManager.registerAll(registryName, new OperatingSystemMetricSet(), ResolutionStrategy.IGNORE, "os");
+ metricManager.registerAll(registryName, new GarbageCollectorMetricSet(), ResolutionStrategy.IGNORE, "gc");
+ metricManager.registerAll(registryName, new MemoryUsageGaugeSet(), ResolutionStrategy.IGNORE, "memory");
+ metricManager.registerAll(registryName, new ThreadStatesGaugeSet(), ResolutionStrategy.IGNORE, "threads"); // todo should we use CachedThreadStatesGaugeSet instead?
+ MetricsMap sysprops = new MetricsMap(map -> System.getProperties().forEach((k, v) -> {
+ //noinspection SuspiciousMethodCalls
+ if (!hiddenSysProps.contains(k)) {
+ map.putNoEx(String.valueOf(k), v);
+ }
+ }));
+ metricManager.registerGauge(null, registryName, sysprops, metricTag, ResolutionStrategy.IGNORE, "properties", "system");
+ MetricsMap sysenv = new MetricsMap(map -> System.getenv().forEach((k, v) -> {
+ if (!hiddenSysProps.contains(k)) {
+ map.putNoEx(String.valueOf(k), v);
+ }
+ }));
+ metricManager.registerGauge(null, registryName, sysenv, metricTag, ResolutionStrategy.IGNORE, "env", "system");
+ } catch (Exception e) {
+ log.warn("Error registering JVM metrics", e);
+ }
+ }
+
+ public RateLimitManager getRateLimitManager() {
+ return rateLimitManager;
+ }
+
+ @VisibleForTesting
+ void setRateLimitManager(RateLimitManager rateLimitManager) {
+ this.rateLimitManager = rateLimitManager;
+ }
+
+ private static class ContextInitializationKey {
+ private ServletContext ctx;
+ volatile CountDownLatch initializing = new CountDownLatch(1);
+
+ private ContextInitializationKey(ServletContext ctx) {
+ if (ctx == null) {
+ throw new IllegalArgumentException("Context must not be null");
+ }
+ // if one of these is reachable both must be to avoid collection from weak hashmap, so
+ // set an attribute holding this object to ensure we never get collected until the ServletContext
+ // is eligible for collection too.
+ ctx.setAttribute(this.getClass().getName(), this);
+ this.ctx = ctx;
+ }
+
+ public synchronized ServletContext getCtx() {
+ return ctx;
+ }
+
+ public synchronized void setCtx(ServletContext ctx) {
+ this.ctx = ctx;
+ }
+
+ synchronized void makeReady() {
+ this.initializing.countDown();
+ }
+
+ // NOT synchronized :)
+ public void waitForReadyService() throws InterruptedException {
+ initializing.await();
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ ContextInitializationKey that = (ContextInitializationKey) o;
+ return ctx.equals(that.ctx);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(ctx);
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `CoreService$ContextInitializationKey.hashCode()` reads without synchronization from `this.ctx`. Potentially races with write in method `CoreService$ContextInitializationKey.setCtx(...)`.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] sonatype-lift[bot] commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r744316022
##########
File path: solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java
##########
@@ -382,7 +386,8 @@ public void lifeCycleStarted(LifeCycle arg0) {
root.getServletContext().setAttribute(SolrDispatchFilter.PROPERTIES_ATTRIBUTE, nodeProperties);
root.getServletContext().setAttribute(SolrDispatchFilter.SOLRHOME_ATTRIBUTE, solrHome);
-
+ coreContainerProvider = new CoreContainerProvider();
Review comment:
I've recorded this as ignored for this pull request. If you change your mind, just comment `@sonatype-lift unignore`.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gus-asf commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
gus-asf commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r696742672
##########
File path: solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java
##########
@@ -0,0 +1,23 @@
+/*
+ * 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.servlet;
+
+import org.apache.solr.core.CoreContainer;
+
+public interface CoreContainerProvider {
+ CoreContainer getCores();
Review comment:
+1 yup good point
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gus-asf commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
gus-asf commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r694834616
##########
File path: solr/core/src/java/org/apache/solr/servlet/ExceptionWhileTracing.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.servlet;
+
+public class ExceptionWhileTracing extends RuntimeException {
Review comment:
(this way the calling code can decide what it cares about, which might be different if we wrap different code) ... I should add docs to this class however to explain that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gus-asf commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
gus-asf commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r696756786
##########
File path: solr/core/src/java/org/apache/solr/servlet/PathExcluder.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.servlet;
+
+import java.util.ArrayList;
+import java.util.regex.Pattern;
+
+public interface PathExcluder {
+ void setExcludePatterns(ArrayList<Pattern> excludePatterns);
Review comment:
No reason that I can think of, probably just moved as is, but easy fix I think. +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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
dsmiley commented on pull request #265:
URL: https://github.com/apache/solr/pull/265#issuecomment-906001252
We could convert SolrDispatchFilter to a Servlet. And maybe we should ditch things that seem of suspicious value, like the "excludePatterns" thing. Really if someone wants a servlet of their own then they are welcome to register it at some other path besides /solr. If Solr is at the root then maybe even Jetty can dispatch? I dunno but seems dubious.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gus-asf commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
gus-asf commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r696768554
##########
File path: solr/test-framework/src/java/org/apache/solr/util/BaseTestHarness.java
##########
@@ -77,14 +77,21 @@ public static String validateXPath(String xml, String... tests)
if (tests==null || tests.length == 0) return null;
- Document document = null;
+ Document document;
try {
+// if (xml.startsWith("<html>")) {
Review comment:
I had these in there, because it makes it clear when you are failing due to a 404, (much clearer than "unexpected content type" or whatever... ) commented them because I'm not sure this change has no side-effects or wouldn't bite someone (seems safe but I didn't look into it deeply) wanted the tests to pass unmodified in any case.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r694441386
##########
File path: solr/core/src/java/org/apache/solr/servlet/CoreService.java
##########
@@ -0,0 +1,451 @@
+/*
+ * 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.servlet;
+
+import com.codahale.metrics.jvm.ClassLoadingGaugeSet;
+import com.codahale.metrics.jvm.GarbageCollectorMetricSet;
+import com.codahale.metrics.jvm.MemoryUsageGaugeSet;
+import com.codahale.metrics.jvm.ThreadStatesGaugeSet;
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.http.client.HttpClient;
+import org.apache.lucene.util.Version;
+import org.apache.solr.cloud.ZkController;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrInfoBean.Group;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.metrics.AltBufferPoolMetricSet;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.OperatingSystemMetricSet;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricManager.ResolutionStrategy;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.servlet.RateLimitManager.Builder;
+import org.apache.solr.util.StartupLoggingUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
+import javax.naming.NoInitialContextException;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
+import javax.servlet.UnavailableException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.Set;
+import java.util.WeakHashMap;
+import java.util.concurrent.CountDownLatch;
+
+import static org.apache.solr.servlet.SolrCoreUtils.loadNodeConfig;
+import static org.apache.solr.servlet.SolrDispatchFilter.PROPERTIES_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLRHOME_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_INSTALL_DIR_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_LEVEL;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_MUTECONSOLE;
+
+/**
+ * A service that can provide access to solr cores. This allows us to have multiple filters and
+ * servlets that depend on SolrCore and CoreContainer, while still only having one CoreContainer per
+ * instance of solr.
+ */
+public class CoreService implements ServletContextListener {
Review comment:
I think it's confusing that it's name suggests a tighter relationship to an individual SolrCore. I suggest CoreContainerHolder
##########
File path: solr/core/src/java/org/apache/solr/servlet/AdminServlet.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.servlet;
+
+import javax.servlet.ServletException;
+import javax.servlet.annotation.WebServlet;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.regex.Pattern;
+
+import static org.apache.solr.servlet.ServletUtils.closeShield;
+import static org.apache.solr.servlet.ServletUtils.configExcludes;
+import static org.apache.solr.servlet.ServletUtils.excludedPath;
+
+@WebServlet
+public class AdminServlet extends HttpServlet implements PathExcluder{
+ private ArrayList<Pattern> excludes;
+
+ @Override
+ public void init() throws ServletException {
+ configExcludes(this, getInitParameter("excludePatterns"));
+ }
+
+ @Override
+ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
+ req = closeShield(req, false);
+ resp = closeShield(resp, false);
+ if (excludedPath(excludes,req,resp,null)) {
+ return;
+ }
+ ServletUtils.rateLimitRequest(req,resp,() -> {
Review comment:
should admin requests really be rate limited?
##########
File path: solr/core/src/java/org/apache/solr/servlet/ExceptionWhileTracing.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.servlet;
+
+public class ExceptionWhileTracing extends RuntimeException {
Review comment:
I hope we can avoid having this class.
##########
File path: solr/core/src/java/org/apache/solr/servlet/SolrCoreUtils.java
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.servlet;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrXmlConfig;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.ByteArrayInputStream;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.util.Properties;
+
+public class SolrCoreUtils {
Review comment:
IMO loadNodeConfig definitely doesn't belong on a class named SolrCoreUtils. SCU is a *core* but the method here is for the *node*. Can't it just be a static method on NodeConfig -- a factory method?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gus-asf commented on pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
gus-asf commented on pull request #265:
URL: https://github.com/apache/solr/pull/265#issuecomment-906550074
> We could convert SolrDispatchFilter to a Servlet. And maybe we should ditch things that seem of suspicious value, like the "excludePatterns" thing. Really if someone wants a servlet of their own then they are welcome to register it at some other path besides /solr. If Solr is at the root then maybe even Jetty can dispatch? I dunno but seems dubious.
Yup, other things I intend to enable. The servlet spec REQUIRES the container to match the longest matching path when looking for a context so even if we installed at / this should not cover any other installed servlet. (which is why authentication should be a filter that can be applied (or not) in the config for other servlets if possible)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gus-asf commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
gus-asf commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r696762643
##########
File path: solr/core/src/java/org/apache/solr/servlet/CoreService.java
##########
@@ -0,0 +1,451 @@
+/*
+ * 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.servlet;
+
+import com.codahale.metrics.jvm.ClassLoadingGaugeSet;
+import com.codahale.metrics.jvm.GarbageCollectorMetricSet;
+import com.codahale.metrics.jvm.MemoryUsageGaugeSet;
+import com.codahale.metrics.jvm.ThreadStatesGaugeSet;
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.http.client.HttpClient;
+import org.apache.lucene.util.Version;
+import org.apache.solr.cloud.ZkController;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrInfoBean.Group;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.metrics.AltBufferPoolMetricSet;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.OperatingSystemMetricSet;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricManager.ResolutionStrategy;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.servlet.RateLimitManager.Builder;
+import org.apache.solr.util.StartupLoggingUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
+import javax.naming.NoInitialContextException;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
+import javax.servlet.UnavailableException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.Set;
+import java.util.WeakHashMap;
+import java.util.concurrent.CountDownLatch;
+
+import static org.apache.solr.servlet.SolrCoreUtils.loadNodeConfig;
+import static org.apache.solr.servlet.SolrDispatchFilter.PROPERTIES_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLRHOME_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_INSTALL_DIR_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_LEVEL;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_MUTECONSOLE;
+
+/**
+ * A service that can provide access to solr cores. This allows us to have multiple filters and
+ * servlets that depend on SolrCore and CoreContainer, while still only having one CoreContainer per
+ * instance of solr.
+ */
+public class CoreService implements ServletContextListener {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private final String metricTag = SolrMetricProducer.getUniqueMetricTag(this, null);
+ private CoreContainer cores;
+ private Properties extraProperties;
+ private HttpClient httpClient;
+ private SolrMetricManager metricManager;
+ private RateLimitManager rateLimitManager;
+ private final CountDownLatch init = new CountDownLatch(1);
+ private String registryName;
+
+ // AFAIK the only reason we need this is to support JettySolrRunner for tests. In tests we might have
+ // multiple CoreContainers in the same JVM, but I *think* that doesn't happen in a real server.
+ private static final Map<ContextInitializationKey, ServiceHolder> services =
+ Collections.synchronizedMap(new WeakHashMap<>());
+
+ // todo: dependency injection instead, but CDI is not jetty native so for now this method and the associated
+ // map will have to suffice. Note that this relies on ServletContext.equals() not implementing anything significantly
+ // different thant Object.equals for its .equals method (I've found no implementation that even immplements it).
+ public static ServiceHolder serviceForContext(ServletContext ctx) throws InterruptedException {
+ ContextInitializationKey key = new ContextInitializationKey(ctx);
+ return services.computeIfAbsent(key, ServiceHolder::new);
+ }
+
+ @Override
+ public void contextInitialized(ServletContextEvent sce) {
+ init(sce.getServletContext());
+ }
+
+ @Override
+ public void contextDestroyed(ServletContextEvent sce) {
+ close();
+ }
+
+ CoreContainer getCoreContainer() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return cores;
+ }
+ HttpClient getHttpClient() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return httpClient;
+ }
+
+ private static void waitForCoreContainer(CoreContainerProvider provider, CountDownLatch latch) throws UnavailableException {
+ CoreContainer cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ try {
+ latch.await();
+ } catch (InterruptedException e) { //well, no wait then
+ }
+ cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ final String msg = "Error processing the request. CoreContainer is either not initialized or shutting down.";
+ log.error(msg);
+ throw new UnavailableException(msg);
Review comment:
Not sure that's true in all cases. This is pretty much a dead server so an extra line of logging won't hurt :)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gus-asf commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
gus-asf commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r744316011
##########
File path: solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java
##########
@@ -382,7 +386,8 @@ public void lifeCycleStarted(LifeCycle arg0) {
root.getServletContext().setAttribute(SolrDispatchFilter.PROPERTIES_ATTRIBUTE, nodeProperties);
root.getServletContext().setAttribute(SolrDispatchFilter.SOLRHOME_ATTRIBUTE, solrHome);
-
+ coreContainerProvider = new CoreContainerProvider();
Review comment:
SOLR-15775
##########
File path: solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java
##########
@@ -382,7 +386,8 @@ public void lifeCycleStarted(LifeCycle arg0) {
root.getServletContext().setAttribute(SolrDispatchFilter.PROPERTIES_ATTRIBUTE, nodeProperties);
root.getServletContext().setAttribute(SolrDispatchFilter.SOLRHOME_ATTRIBUTE, solrHome);
-
+ coreContainerProvider = new CoreContainerProvider();
Review comment:
ignore
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
dsmiley commented on pull request #265:
URL: https://github.com/apache/solr/pull/265#issuecomment-905995578
If the idea is to leverage the constructs in the servlet API more to our advantage, then perhaps both Tracing and Rate Limiting should be servlet Filters?
FWIW I don't find SolrDispatchFilter _that_ bad, though there are some warts and my other PR ( #155 ) tries to tackle some. If you read the opening of my PR there, you'll see I dug deeper into some complexities like the cores==null checks that seem unnecessary. Apparently those are there due to Mikhail's realization in SOLR-10615 that init() isn't necessary complete before doFilter can be called which shocked him and me too; I'm kinda in disbelief honestly.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] markrmiller commented on pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
markrmiller commented on pull request #265:
URL: https://github.com/apache/solr/pull/265#issuecomment-907046278
The core provider is good. Using a listener is a much better approach for CoreContainer life-cycle. It could use a bit more as well though - shutdown should actually be two parts - advertising the impending shutdown of the node, most easily by removing the zk live node, which actually needs to happen before the webapp or corecontainer starts shutting down - and then actually shutting down, ideally with a configurable grace timeout. That often means playing with Jetty more than the webapp spec to do right.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gus-asf commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
gus-asf commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r696750678
##########
File path: solr/core/src/java/org/apache/solr/servlet/CoreService.java
##########
@@ -0,0 +1,451 @@
+/*
+ * 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.servlet;
+
+import com.codahale.metrics.jvm.ClassLoadingGaugeSet;
+import com.codahale.metrics.jvm.GarbageCollectorMetricSet;
+import com.codahale.metrics.jvm.MemoryUsageGaugeSet;
+import com.codahale.metrics.jvm.ThreadStatesGaugeSet;
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.http.client.HttpClient;
+import org.apache.lucene.util.Version;
+import org.apache.solr.cloud.ZkController;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrInfoBean.Group;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.metrics.AltBufferPoolMetricSet;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.OperatingSystemMetricSet;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricManager.ResolutionStrategy;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.servlet.RateLimitManager.Builder;
+import org.apache.solr.util.StartupLoggingUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
+import javax.naming.NoInitialContextException;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
+import javax.servlet.UnavailableException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.Set;
+import java.util.WeakHashMap;
+import java.util.concurrent.CountDownLatch;
+
+import static org.apache.solr.servlet.SolrCoreUtils.loadNodeConfig;
+import static org.apache.solr.servlet.SolrDispatchFilter.PROPERTIES_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLRHOME_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_INSTALL_DIR_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_LEVEL;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_MUTECONSOLE;
+
+/**
+ * A service that can provide access to solr cores. This allows us to have multiple filters and
+ * servlets that depend on SolrCore and CoreContainer, while still only having one CoreContainer per
+ * instance of solr.
+ */
+public class CoreService implements ServletContextListener {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private final String metricTag = SolrMetricProducer.getUniqueMetricTag(this, null);
+ private CoreContainer cores;
+ private Properties extraProperties;
+ private HttpClient httpClient;
+ private SolrMetricManager metricManager;
+ private RateLimitManager rateLimitManager;
+ private final CountDownLatch init = new CountDownLatch(1);
+ private String registryName;
+
+ // AFAIK the only reason we need this is to support JettySolrRunner for tests. In tests we might have
+ // multiple CoreContainers in the same JVM, but I *think* that doesn't happen in a real server.
+ private static final Map<ContextInitializationKey, ServiceHolder> services =
+ Collections.synchronizedMap(new WeakHashMap<>());
+
+ // todo: dependency injection instead, but CDI is not jetty native so for now this method and the associated
+ // map will have to suffice. Note that this relies on ServletContext.equals() not implementing anything significantly
+ // different thant Object.equals for its .equals method (I've found no implementation that even immplements it).
+ public static ServiceHolder serviceForContext(ServletContext ctx) throws InterruptedException {
+ ContextInitializationKey key = new ContextInitializationKey(ctx);
+ return services.computeIfAbsent(key, ServiceHolder::new);
+ }
+
+ @Override
+ public void contextInitialized(ServletContextEvent sce) {
+ init(sce.getServletContext());
+ }
+
+ @Override
+ public void contextDestroyed(ServletContextEvent sce) {
+ close();
+ }
+
+ CoreContainer getCoreContainer() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return cores;
+ }
+ HttpClient getHttpClient() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return httpClient;
+ }
+
+ private static void waitForCoreContainer(CoreContainerProvider provider, CountDownLatch latch) throws UnavailableException {
+ CoreContainer cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ try {
+ latch.await();
Review comment:
Perhaps a long one, but I think that if anyone has a bad case where startup is taking a very long time a timeout here creates a duration after which they (and their cluster) are simply hosed and can never start, so I'm a bit leery of it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gus-asf commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
gus-asf commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r696741759
##########
File path: solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java
##########
@@ -527,7 +536,7 @@ public void start(boolean reusePort) throws Exception {
}
synchronized (JettySolrRunner.this) {
int cnt = 0;
- while (!waitOnSolr || !dispatchFilter.isRunning() || getCoreContainer() == null) {
+ while (!waitOnSolr || !dispatchFilter.isRunning() ) {
Review comment:
Actually I suspect that this entire synchronized block is unnecessary. The current form of getCoreContainer() now causes a wait and I was getting lock ups here that the tests didn't ever initialize. I suspect that this startup for JettySolrRunner is actually single threaded and these two checks always pass, and actually waiting on something destined to fail anyway, but I haven't gone back to verify that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gus-asf commented on pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
gus-asf commented on pull request #265:
URL: https://github.com/apache/solr/pull/265#issuecomment-904638223
> I left some comments but overall it seems this is _very_ much in progress, so it's hard for me to judge. If I recall, you were thinking of having some number of Servlets and have Jetty do some dispatch but I don't notice this.
Lost in the wide ranging discussions on the list is the idea that I intend this patch to be a small but hopefully *generally* helpful step in the right direction. The idea in *this* patch is to move some code into re-usable locations and make core-container available outside the SolrDispatchFilter so future authors are not forced to add more stuff to the dispatch filter. Of course nobody is **really** forced to do anything but they would need to front-load 15h or so of yak shaving and a significant dose of ambition if they wanted to avoid expanding the filter. This patch is meant to provide a cleaner, mostly hairless yak for future use by myself and others :)
I'm trying hard to think of useful, discrete steps toward the larger goals discussed, that stand alone and get them merged in chunks that are both reviewable and beneficial on their own, so that my work doesn't get so out of date that I abandon it for pain of merging, and so that others can pitch in if they want, etc. So the question is is this patch (minus AdminServlet, which is an example for review purposes, but not ready for commit at this time. I now wish I'd thought to leave a //nocommit comment to this effect in the class itself rather than just saying so in the issue)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] sonatype-lift[bot] commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r739890201
##########
File path: solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java
##########
@@ -382,7 +386,8 @@ public void lifeCycleStarted(LifeCycle arg0) {
root.getServletContext().setAttribute(SolrDispatchFilter.PROPERTIES_ATTRIBUTE, nodeProperties);
root.getServletContext().setAttribute(SolrDispatchFilter.SOLRHOME_ATTRIBUTE, solrHome);
-
+ coreContainerProvider = new CoreContainerProvider();
Review comment:
*THREAD_SAFETY_VIOLATION:* Unprotected write. Non-private method `JettySolrRunner$1.lifeCycleStarted(...)` writes to field `this.this$0.coreContainerProvider` outside of synchronization.
Reporting because this access may occur on a background thread.
(at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java
##########
@@ -0,0 +1,479 @@
+/*
+ * 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.servlet;
+
+import com.codahale.metrics.jvm.ClassLoadingGaugeSet;
+import com.codahale.metrics.jvm.GarbageCollectorMetricSet;
+import com.codahale.metrics.jvm.MemoryUsageGaugeSet;
+import com.codahale.metrics.jvm.ThreadStatesGaugeSet;
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.http.client.HttpClient;
+import org.apache.lucene.util.Version;
+import org.apache.solr.cloud.ZkController;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrInfoBean.Group;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.metrics.AltBufferPoolMetricSet;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.OperatingSystemMetricSet;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricManager.ResolutionStrategy;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.servlet.RateLimitManager.Builder;
+import org.apache.solr.util.StartupLoggingUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
+import javax.naming.NoInitialContextException;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
+import javax.servlet.UnavailableException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.Set;
+import java.util.WeakHashMap;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Supplier;
+
+import static org.apache.solr.core.NodeConfig.loadNodeConfig;
+import static org.apache.solr.servlet.SolrDispatchFilter.PROPERTIES_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLRHOME_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_INSTALL_DIR_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_LEVEL;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_MUTECONSOLE;
+
+/**
+ * A service that can provide access to solr cores. This allows us to have multiple filters and
+ * servlets that depend on SolrCore and CoreContainer, while still only having one CoreContainer per
+ * instance of solr.
+ */
+public class CoreContainerProvider implements ServletContextListener {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private final String metricTag = SolrMetricProducer.getUniqueMetricTag(this, null);
+ private CoreContainer cores;
+ private Properties extraProperties;
+ private HttpClient httpClient;
+ private SolrMetricManager metricManager;
+ private RateLimitManager rateLimitManager;
+ private final CountDownLatch init = new CountDownLatch(1);
+ private String registryName;
+ private final boolean isV2Enabled = !"true".equals(System.getProperty("disable.v2.api", "false"));
+ // AFAIK the only reason we need this is to support JettySolrRunner for tests. In tests we might have
+ // multiple CoreContainers in the same JVM, but I *think* that doesn't happen in a real server.
+ private static final Map<ContextInitializationKey, ServiceHolder> services =
+ Collections.synchronizedMap(new WeakHashMap<>());
+
+ // todo: dependency injection instead, but CDI is not jetty native so for now this method and the associated
+ // map will have to suffice. Note that this relies on ServletContext.equals() not implementing anything significantly
+ // different thant Object.equals for its .equals method (I've found no implementation that even immplements it).
+ public static ServiceHolder serviceForContext(ServletContext ctx) throws InterruptedException {
+ ContextInitializationKey key = new ContextInitializationKey(ctx);
+ return services.computeIfAbsent(key, ServiceHolder::new);
+ }
+
+ @Override
+ public void contextInitialized(ServletContextEvent sce) {
+ init(sce.getServletContext());
+ }
+
+ @Override
+ public void contextDestroyed(ServletContextEvent sce) {
+ close();
+ }
+
+ CoreContainer getCoreContainer() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return cores;
+ }
+ HttpClient getHttpClient() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return httpClient;
+ }
+
+ private static void waitForCoreContainer(Supplier<CoreContainer> provider, CountDownLatch latch) throws UnavailableException {
+ CoreContainer cores = provider.get();
+ if (cores == null || cores.isShutDown()) {
+ long startWait = System.nanoTime();
+ try {
+ while (!latch.await(10, TimeUnit.SECONDS)) {
+ long now = System.nanoTime();
+ if (log.isInfoEnabled()) {
+ log.info("Still waiting for CoreContainerStartup ({} seconds elapsed)", (now - startWait) / 1_000_000_000);
+ }
+ }
+ } catch (InterruptedException e) { //well, no wait then
+ Thread.currentThread().interrupt();
+ }
+ cores = provider.get();
+ if (cores == null || cores.isShutDown()) {
+ final String msg = "Error processing the request. CoreContainer is either not initialized or shutting down.";
+ log.error(msg);
+ throw new UnavailableException(msg);
+ }
+ }
+ }
+
+ public void close() {
+ CoreContainer cc = cores;
+ if (cc != null) {
+ ZkController zkController = cc.getZkController();
+ if (zkController != null) {
+
+ // Mark Miller suggested that we should be publishing that we are down before anything else which makes
+ // good sense, but the following causes test failures, so that improvement can be the subject of another
+ // PR/issue. Also, jetty might already be refusing requests by this point so that's a potential issue too.
+ // Digging slightly I see that there's a whole mess of code looking up collections and calculating state
+ // changes associated with this call, which smells a lot like we're duplicating node state in collection
+ // stuff, but it will take a lot of code reading to figure out why and if there's room for improvement.
+
+ //zkController.publishNodeAsDown(zkController.getNodeName());
+ }
+ }
+ cores = null;
+ try {
+ if (metricManager != null) {
+ try {
+ metricManager.unregisterGauges(registryName, metricTag);
+ } catch (NullPointerException e) {
+ // okay
+ } catch (Exception e) {
+ log.warn("Exception closing FileCleaningTracker", e);
+ } finally {
+ metricManager = null;
+ }
+ }
+ } finally {
+ if (cc != null) {
+ httpClient = null;
+ cc.shutdown();
+ }
+ }
+ }
+
+ public void init(ServletContext servletContext) {
+ if (log.isTraceEnabled()) {
+ log.trace("CoreService.init(): {}", this.getClass().getClassLoader());
+ }
+ CoreContainer coresInit = null;
+ try {
+ // "extra" properties must be init'ed first so we know things like "do we have a zkHost"
+ // wrap as defaults (if set) so we can modify w/o polluting the Properties provided by our caller
+ this.extraProperties = SolrXmlConfig.wrapAndSetZkHostFromSysPropIfNeeded
+ ((Properties) servletContext.getAttribute(PROPERTIES_ATTRIBUTE));
+
+ StartupLoggingUtils.checkLogDir();
+ if (log.isInfoEnabled()) {
+ log.info("Using logger factory {}", StartupLoggingUtils.getLoggerImplStr());
+ }
+
+ logWelcomeBanner();
+
+ String muteConsole = System.getProperty(SOLR_LOG_MUTECONSOLE);
+ if (muteConsole != null && !Arrays.asList("false","0","off","no").contains(muteConsole.toLowerCase(Locale.ROOT))) {
+ StartupLoggingUtils.muteConsole();
+ }
+ String logLevel = System.getProperty(SOLR_LOG_LEVEL);
+ if (logLevel != null) {
+ log.info("Log level override, property solr.log.level={}", logLevel);
+ StartupLoggingUtils.changeLogLevel(logLevel);
+ }
+
+ coresInit = createCoreContainer(computeSolrHome(servletContext), extraProperties);
+ this.httpClient = coresInit.getUpdateShardHandler().getDefaultHttpClient();
+ setupJvmMetrics(coresInit);
+
+ SolrZkClient zkClient = null;
+ ZkController zkController = coresInit.getZkController();
+
+ if (zkController != null) {
+ zkClient = zkController.getZkClient();
+ }
+
+ Builder builder = new Builder(zkClient);
+
+ this.rateLimitManager = builder.build();
+
+ if (zkController != null) {
+ zkController.zkStateReader.registerClusterPropertiesListener(this.rateLimitManager);
+ }
+
+ if (log.isDebugEnabled()) {
+ log.debug("user.dir={}", System.getProperty("user.dir"));
+ }
+ } catch( Throwable t ) {
+ // catch this so our filter still works
+ log.error( "Could not start Solr. Check solr/home property and the logs");
+ SolrCore.log( t );
+ if (t instanceof Error) {
+ throw (Error) t;
+ }
+ } finally{
+ log.trace("SolrDispatchFilter.init() done");
+ this.cores = coresInit; // crucially final assignment
+ services.computeIfAbsent(new ContextInitializationKey(servletContext), ServiceHolder::new)
+ .setService(this);
+ init.countDown();
+ }
+ }
+
+
+ private void logWelcomeBanner() {
+ // _Really_ sorry about how clumsy this is as a result of the logging call checker, but this is the only one
+ // that's so ugly so far.
+ if (log.isInfoEnabled()) {
+ log.info(" ___ _ Welcome to Apache Solr™ version {}", solrVersion());
+ }
+ if (log.isInfoEnabled()) {
+ log.info("/ __| ___| |_ _ Starting in {} mode on port {}", isCloudMode() ? "cloud" : "standalone", getSolrPort());
+ }
+ if (log.isInfoEnabled()) {
+ log.info("\\__ \\/ _ \\ | '_| Install dir: {}", System.getProperty(SOLR_INSTALL_DIR_ATTRIBUTE));
+ }
+ if (log.isInfoEnabled()) {
+ log.info("|___/\\___/_|_| Start time: {}", Instant.now());
+ }
+ }
+ private String solrVersion() {
+ String specVer = Version.LATEST.toString();
+ try {
+ String implVer = SolrCore.class.getPackage().getImplementationVersion();
+ return (specVer.equals(implVer.split(" ")[0])) ? specVer : implVer;
+ } catch (Exception e) {
+ return specVer;
+ }
+ }
+
+ private String getSolrPort() {
+ return System.getProperty("jetty.port");
+ }
+
+ /**
+ * We are in cloud mode if Java option zkRun exists OR zkHost exists and is non-empty
+ * @see SolrXmlConfig#wrapAndSetZkHostFromSysPropIfNeeded
+ * @see #extraProperties
+ * @see #init
+ */
+ private boolean isCloudMode() {
+ assert null != extraProperties; // we should never be called w/o this being initialized
+ return (null != extraProperties.getProperty(SolrXmlConfig.ZK_HOST)) || (null != System.getProperty("zkRun"));
+ }
+
+ /**
+ * Returns the effective Solr Home to use for this node, based on looking up the value in this order:
+ * <ol>
+ * <li>attribute in the FilterConfig</li>
+ * <li>JNDI: via java:comp/env/solr/home</li>
+ * <li>The system property solr.solr.home</li>
+ * <li>Look in the current working directory for a solr/ directory</li>
+ * </ol>
+ * <p>
+ *
+ * @return the Solr home, absolute and normalized.
+ */
+ private static Path computeSolrHome(ServletContext servletContext) {
+
+ // start with explicit check of servlet config...
+ String source = "servlet config: " + SOLRHOME_ATTRIBUTE;
+ String home = (String) servletContext.getAttribute(SOLRHOME_ATTRIBUTE);
+
+ if (null == home) {
+ final String lookup = "java:comp/env/solr/home";
+ // Try JNDI
+ source = "JNDI: " + lookup;
+ try {
+ Context c = new InitialContext();
+ home = (String) c.lookup(lookup);
+ } catch (NoInitialContextException e) {
+ log.debug("JNDI not configured for solr (NoInitialContextEx)");
+ } catch (NamingException e) {
+ log.debug("No /solr/home in JNDI");
+ } catch (RuntimeException ex) {
+ log.warn("Odd RuntimeException while testing for JNDI: ", ex);
+ }
+ }
+
+ if (null == home) {
+ // Now try system property
+ final String prop = "solr.solr.home";
+ source = "system property: " + prop;
+ home = System.getProperty(prop);
+ }
+
+ if (null == home) {
+ // if all else fails, assume default dir
+ home = "solr/";
+ source = "defaulted to '" + home + "' ... could not find system property or JNDI";
+ }
+ final Path solrHome = Paths.get(home).toAbsolutePath().normalize();
+ log.info("Solr Home: {} (source: {})", solrHome, source);
+
+ return solrHome;
+ }
+
+ /**
+ * CoreContainer initialization
+ * @return a CoreContainer to hold this server's cores
+ */
+ protected CoreContainer createCoreContainer(Path solrHome, Properties nodeProps) {
+ NodeConfig nodeConfig = loadNodeConfig(solrHome, nodeProps);
+ final CoreContainer coreContainer = new CoreContainer(nodeConfig, true);
+ coreContainer.load();
+ return coreContainer;
+ }
+
+
+
+ private void setupJvmMetrics(CoreContainer coresInit) {
+ metricManager = coresInit.getMetricManager();
+ registryName = SolrMetricManager.getRegistryName(Group.jvm);
+ final Set<String> hiddenSysProps = coresInit.getConfig().getMetricsConfig().getHiddenSysProps();
+ try {
+ metricManager.registerAll(registryName, new AltBufferPoolMetricSet(), ResolutionStrategy.IGNORE, "buffers");
+ metricManager.registerAll(registryName, new ClassLoadingGaugeSet(), ResolutionStrategy.IGNORE, "classes");
+ metricManager.registerAll(registryName, new OperatingSystemMetricSet(), ResolutionStrategy.IGNORE, "os");
+ metricManager.registerAll(registryName, new GarbageCollectorMetricSet(), ResolutionStrategy.IGNORE, "gc");
+ metricManager.registerAll(registryName, new MemoryUsageGaugeSet(), ResolutionStrategy.IGNORE, "memory");
+ metricManager.registerAll(registryName, new ThreadStatesGaugeSet(), ResolutionStrategy.IGNORE, "threads"); // todo should we use CachedThreadStatesGaugeSet instead?
+ MetricsMap sysprops = new MetricsMap(map -> System.getProperties().forEach((k, v) -> {
+ //noinspection SuspiciousMethodCalls
+ if (!hiddenSysProps.contains(k)) {
+ map.putNoEx(String.valueOf(k), v);
+ }
+ }));
+ metricManager.registerGauge(null, registryName, sysprops, metricTag, ResolutionStrategy.IGNORE, "properties", "system");
+ MetricsMap sysenv = new MetricsMap(map -> System.getenv().forEach((k, v) -> {
+ if (!hiddenSysProps.contains(k)) {
+ map.putNoEx(String.valueOf(k), v);
+ }
+ }));
+ metricManager.registerGauge(null, registryName, sysenv, metricTag, ResolutionStrategy.IGNORE, "env", "system");
+ } catch (Exception e) {
+ log.warn("Error registering JVM metrics", e);
+ }
+ }
+
+ public RateLimitManager getRateLimitManager() {
+ return rateLimitManager;
+ }
+
+ @VisibleForTesting
+ void setRateLimitManager(RateLimitManager rateLimitManager) {
+ this.rateLimitManager = rateLimitManager;
+ }
+
+ public boolean isV2Enabled() {
+ return isV2Enabled;
+ }
+
+ private static class ContextInitializationKey {
+ private ServletContext ctx;
+ volatile CountDownLatch initializing = new CountDownLatch(1);
+
+ private ContextInitializationKey(ServletContext ctx) {
+ if (ctx == null) {
+ throw new IllegalArgumentException("Context must not be null");
+ }
+ // if one of these is reachable both must be to avoid collection from weak hashmap, so
+ // set an attribute holding this object to ensure we never get collected until the ServletContext
+ // is eligible for collection too.
+ ctx.setAttribute(this.getClass().getName(), this);
+ this.ctx = ctx;
+ }
+
+ public synchronized ServletContext getCtx() {
+ return ctx;
+ }
+
+ public synchronized void setCtx(ServletContext ctx) {
+ this.ctx = ctx;
+ }
+
+ synchronized void makeReady() {
+ this.initializing.countDown();
+ }
+
+ // NOT synchronized :)
+ public void waitForReadyService() throws InterruptedException {
+ initializing.await();
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ ContextInitializationKey that = (ContextInitializationKey) o;
+ return ctx.equals(that.ctx);
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `CoreContainerProvider$ContextInitializationKey.equals(...)` reads without synchronization from `this.ctx`. Potentially races with write in method `CoreContainerProvider$ContextInitializationKey.setCtx(...)`.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java
##########
@@ -0,0 +1,479 @@
+/*
+ * 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.servlet;
+
+import com.codahale.metrics.jvm.ClassLoadingGaugeSet;
+import com.codahale.metrics.jvm.GarbageCollectorMetricSet;
+import com.codahale.metrics.jvm.MemoryUsageGaugeSet;
+import com.codahale.metrics.jvm.ThreadStatesGaugeSet;
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.http.client.HttpClient;
+import org.apache.lucene.util.Version;
+import org.apache.solr.cloud.ZkController;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrInfoBean.Group;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.metrics.AltBufferPoolMetricSet;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.OperatingSystemMetricSet;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricManager.ResolutionStrategy;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.servlet.RateLimitManager.Builder;
+import org.apache.solr.util.StartupLoggingUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
+import javax.naming.NoInitialContextException;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
+import javax.servlet.UnavailableException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.Set;
+import java.util.WeakHashMap;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Supplier;
+
+import static org.apache.solr.core.NodeConfig.loadNodeConfig;
+import static org.apache.solr.servlet.SolrDispatchFilter.PROPERTIES_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLRHOME_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_INSTALL_DIR_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_LEVEL;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_MUTECONSOLE;
+
+/**
+ * A service that can provide access to solr cores. This allows us to have multiple filters and
+ * servlets that depend on SolrCore and CoreContainer, while still only having one CoreContainer per
+ * instance of solr.
+ */
+public class CoreContainerProvider implements ServletContextListener {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private final String metricTag = SolrMetricProducer.getUniqueMetricTag(this, null);
+ private CoreContainer cores;
+ private Properties extraProperties;
+ private HttpClient httpClient;
+ private SolrMetricManager metricManager;
+ private RateLimitManager rateLimitManager;
+ private final CountDownLatch init = new CountDownLatch(1);
+ private String registryName;
+ private final boolean isV2Enabled = !"true".equals(System.getProperty("disable.v2.api", "false"));
+ // AFAIK the only reason we need this is to support JettySolrRunner for tests. In tests we might have
+ // multiple CoreContainers in the same JVM, but I *think* that doesn't happen in a real server.
+ private static final Map<ContextInitializationKey, ServiceHolder> services =
+ Collections.synchronizedMap(new WeakHashMap<>());
+
+ // todo: dependency injection instead, but CDI is not jetty native so for now this method and the associated
+ // map will have to suffice. Note that this relies on ServletContext.equals() not implementing anything significantly
+ // different thant Object.equals for its .equals method (I've found no implementation that even immplements it).
+ public static ServiceHolder serviceForContext(ServletContext ctx) throws InterruptedException {
+ ContextInitializationKey key = new ContextInitializationKey(ctx);
+ return services.computeIfAbsent(key, ServiceHolder::new);
+ }
+
+ @Override
+ public void contextInitialized(ServletContextEvent sce) {
+ init(sce.getServletContext());
+ }
+
+ @Override
+ public void contextDestroyed(ServletContextEvent sce) {
+ close();
+ }
+
+ CoreContainer getCoreContainer() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return cores;
+ }
+ HttpClient getHttpClient() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return httpClient;
+ }
+
+ private static void waitForCoreContainer(Supplier<CoreContainer> provider, CountDownLatch latch) throws UnavailableException {
+ CoreContainer cores = provider.get();
+ if (cores == null || cores.isShutDown()) {
+ long startWait = System.nanoTime();
+ try {
+ while (!latch.await(10, TimeUnit.SECONDS)) {
+ long now = System.nanoTime();
+ if (log.isInfoEnabled()) {
+ log.info("Still waiting for CoreContainerStartup ({} seconds elapsed)", (now - startWait) / 1_000_000_000);
+ }
+ }
+ } catch (InterruptedException e) { //well, no wait then
+ Thread.currentThread().interrupt();
+ }
+ cores = provider.get();
+ if (cores == null || cores.isShutDown()) {
+ final String msg = "Error processing the request. CoreContainer is either not initialized or shutting down.";
+ log.error(msg);
+ throw new UnavailableException(msg);
+ }
+ }
+ }
+
+ public void close() {
+ CoreContainer cc = cores;
+ if (cc != null) {
+ ZkController zkController = cc.getZkController();
+ if (zkController != null) {
+
+ // Mark Miller suggested that we should be publishing that we are down before anything else which makes
+ // good sense, but the following causes test failures, so that improvement can be the subject of another
+ // PR/issue. Also, jetty might already be refusing requests by this point so that's a potential issue too.
+ // Digging slightly I see that there's a whole mess of code looking up collections and calculating state
+ // changes associated with this call, which smells a lot like we're duplicating node state in collection
+ // stuff, but it will take a lot of code reading to figure out why and if there's room for improvement.
+
+ //zkController.publishNodeAsDown(zkController.getNodeName());
+ }
+ }
+ cores = null;
+ try {
+ if (metricManager != null) {
+ try {
+ metricManager.unregisterGauges(registryName, metricTag);
+ } catch (NullPointerException e) {
+ // okay
+ } catch (Exception e) {
+ log.warn("Exception closing FileCleaningTracker", e);
+ } finally {
+ metricManager = null;
+ }
+ }
+ } finally {
+ if (cc != null) {
+ httpClient = null;
+ cc.shutdown();
+ }
+ }
+ }
+
+ public void init(ServletContext servletContext) {
+ if (log.isTraceEnabled()) {
+ log.trace("CoreService.init(): {}", this.getClass().getClassLoader());
+ }
+ CoreContainer coresInit = null;
+ try {
+ // "extra" properties must be init'ed first so we know things like "do we have a zkHost"
+ // wrap as defaults (if set) so we can modify w/o polluting the Properties provided by our caller
+ this.extraProperties = SolrXmlConfig.wrapAndSetZkHostFromSysPropIfNeeded
+ ((Properties) servletContext.getAttribute(PROPERTIES_ATTRIBUTE));
+
+ StartupLoggingUtils.checkLogDir();
+ if (log.isInfoEnabled()) {
+ log.info("Using logger factory {}", StartupLoggingUtils.getLoggerImplStr());
+ }
+
+ logWelcomeBanner();
+
+ String muteConsole = System.getProperty(SOLR_LOG_MUTECONSOLE);
+ if (muteConsole != null && !Arrays.asList("false","0","off","no").contains(muteConsole.toLowerCase(Locale.ROOT))) {
+ StartupLoggingUtils.muteConsole();
+ }
+ String logLevel = System.getProperty(SOLR_LOG_LEVEL);
+ if (logLevel != null) {
+ log.info("Log level override, property solr.log.level={}", logLevel);
+ StartupLoggingUtils.changeLogLevel(logLevel);
+ }
+
+ coresInit = createCoreContainer(computeSolrHome(servletContext), extraProperties);
+ this.httpClient = coresInit.getUpdateShardHandler().getDefaultHttpClient();
+ setupJvmMetrics(coresInit);
+
+ SolrZkClient zkClient = null;
+ ZkController zkController = coresInit.getZkController();
+
+ if (zkController != null) {
+ zkClient = zkController.getZkClient();
+ }
+
+ Builder builder = new Builder(zkClient);
+
+ this.rateLimitManager = builder.build();
+
+ if (zkController != null) {
+ zkController.zkStateReader.registerClusterPropertiesListener(this.rateLimitManager);
+ }
+
+ if (log.isDebugEnabled()) {
+ log.debug("user.dir={}", System.getProperty("user.dir"));
+ }
+ } catch( Throwable t ) {
+ // catch this so our filter still works
+ log.error( "Could not start Solr. Check solr/home property and the logs");
+ SolrCore.log( t );
+ if (t instanceof Error) {
+ throw (Error) t;
+ }
+ } finally{
+ log.trace("SolrDispatchFilter.init() done");
+ this.cores = coresInit; // crucially final assignment
+ services.computeIfAbsent(new ContextInitializationKey(servletContext), ServiceHolder::new)
+ .setService(this);
+ init.countDown();
+ }
+ }
+
+
+ private void logWelcomeBanner() {
+ // _Really_ sorry about how clumsy this is as a result of the logging call checker, but this is the only one
+ // that's so ugly so far.
+ if (log.isInfoEnabled()) {
+ log.info(" ___ _ Welcome to Apache Solr™ version {}", solrVersion());
+ }
+ if (log.isInfoEnabled()) {
+ log.info("/ __| ___| |_ _ Starting in {} mode on port {}", isCloudMode() ? "cloud" : "standalone", getSolrPort());
+ }
+ if (log.isInfoEnabled()) {
+ log.info("\\__ \\/ _ \\ | '_| Install dir: {}", System.getProperty(SOLR_INSTALL_DIR_ATTRIBUTE));
+ }
+ if (log.isInfoEnabled()) {
+ log.info("|___/\\___/_|_| Start time: {}", Instant.now());
+ }
+ }
+ private String solrVersion() {
+ String specVer = Version.LATEST.toString();
+ try {
+ String implVer = SolrCore.class.getPackage().getImplementationVersion();
+ return (specVer.equals(implVer.split(" ")[0])) ? specVer : implVer;
+ } catch (Exception e) {
+ return specVer;
+ }
+ }
+
+ private String getSolrPort() {
+ return System.getProperty("jetty.port");
+ }
+
+ /**
+ * We are in cloud mode if Java option zkRun exists OR zkHost exists and is non-empty
+ * @see SolrXmlConfig#wrapAndSetZkHostFromSysPropIfNeeded
+ * @see #extraProperties
+ * @see #init
+ */
+ private boolean isCloudMode() {
+ assert null != extraProperties; // we should never be called w/o this being initialized
+ return (null != extraProperties.getProperty(SolrXmlConfig.ZK_HOST)) || (null != System.getProperty("zkRun"));
+ }
+
+ /**
+ * Returns the effective Solr Home to use for this node, based on looking up the value in this order:
+ * <ol>
+ * <li>attribute in the FilterConfig</li>
+ * <li>JNDI: via java:comp/env/solr/home</li>
+ * <li>The system property solr.solr.home</li>
+ * <li>Look in the current working directory for a solr/ directory</li>
+ * </ol>
+ * <p>
+ *
+ * @return the Solr home, absolute and normalized.
+ */
+ private static Path computeSolrHome(ServletContext servletContext) {
+
+ // start with explicit check of servlet config...
+ String source = "servlet config: " + SOLRHOME_ATTRIBUTE;
+ String home = (String) servletContext.getAttribute(SOLRHOME_ATTRIBUTE);
+
+ if (null == home) {
+ final String lookup = "java:comp/env/solr/home";
+ // Try JNDI
+ source = "JNDI: " + lookup;
+ try {
+ Context c = new InitialContext();
+ home = (String) c.lookup(lookup);
+ } catch (NoInitialContextException e) {
+ log.debug("JNDI not configured for solr (NoInitialContextEx)");
+ } catch (NamingException e) {
+ log.debug("No /solr/home in JNDI");
+ } catch (RuntimeException ex) {
+ log.warn("Odd RuntimeException while testing for JNDI: ", ex);
+ }
+ }
+
+ if (null == home) {
+ // Now try system property
+ final String prop = "solr.solr.home";
+ source = "system property: " + prop;
+ home = System.getProperty(prop);
+ }
+
+ if (null == home) {
+ // if all else fails, assume default dir
+ home = "solr/";
+ source = "defaulted to '" + home + "' ... could not find system property or JNDI";
+ }
+ final Path solrHome = Paths.get(home).toAbsolutePath().normalize();
+ log.info("Solr Home: {} (source: {})", solrHome, source);
+
+ return solrHome;
+ }
+
+ /**
+ * CoreContainer initialization
+ * @return a CoreContainer to hold this server's cores
+ */
+ protected CoreContainer createCoreContainer(Path solrHome, Properties nodeProps) {
+ NodeConfig nodeConfig = loadNodeConfig(solrHome, nodeProps);
+ final CoreContainer coreContainer = new CoreContainer(nodeConfig, true);
+ coreContainer.load();
+ return coreContainer;
+ }
+
+
+
+ private void setupJvmMetrics(CoreContainer coresInit) {
+ metricManager = coresInit.getMetricManager();
+ registryName = SolrMetricManager.getRegistryName(Group.jvm);
+ final Set<String> hiddenSysProps = coresInit.getConfig().getMetricsConfig().getHiddenSysProps();
+ try {
+ metricManager.registerAll(registryName, new AltBufferPoolMetricSet(), ResolutionStrategy.IGNORE, "buffers");
+ metricManager.registerAll(registryName, new ClassLoadingGaugeSet(), ResolutionStrategy.IGNORE, "classes");
+ metricManager.registerAll(registryName, new OperatingSystemMetricSet(), ResolutionStrategy.IGNORE, "os");
+ metricManager.registerAll(registryName, new GarbageCollectorMetricSet(), ResolutionStrategy.IGNORE, "gc");
+ metricManager.registerAll(registryName, new MemoryUsageGaugeSet(), ResolutionStrategy.IGNORE, "memory");
+ metricManager.registerAll(registryName, new ThreadStatesGaugeSet(), ResolutionStrategy.IGNORE, "threads"); // todo should we use CachedThreadStatesGaugeSet instead?
+ MetricsMap sysprops = new MetricsMap(map -> System.getProperties().forEach((k, v) -> {
+ //noinspection SuspiciousMethodCalls
+ if (!hiddenSysProps.contains(k)) {
+ map.putNoEx(String.valueOf(k), v);
+ }
+ }));
+ metricManager.registerGauge(null, registryName, sysprops, metricTag, ResolutionStrategy.IGNORE, "properties", "system");
+ MetricsMap sysenv = new MetricsMap(map -> System.getenv().forEach((k, v) -> {
+ if (!hiddenSysProps.contains(k)) {
+ map.putNoEx(String.valueOf(k), v);
+ }
+ }));
+ metricManager.registerGauge(null, registryName, sysenv, metricTag, ResolutionStrategy.IGNORE, "env", "system");
+ } catch (Exception e) {
+ log.warn("Error registering JVM metrics", e);
+ }
+ }
+
+ public RateLimitManager getRateLimitManager() {
+ return rateLimitManager;
+ }
+
+ @VisibleForTesting
+ void setRateLimitManager(RateLimitManager rateLimitManager) {
+ this.rateLimitManager = rateLimitManager;
+ }
+
+ public boolean isV2Enabled() {
+ return isV2Enabled;
+ }
+
+ private static class ContextInitializationKey {
+ private ServletContext ctx;
+ volatile CountDownLatch initializing = new CountDownLatch(1);
+
+ private ContextInitializationKey(ServletContext ctx) {
+ if (ctx == null) {
+ throw new IllegalArgumentException("Context must not be null");
+ }
+ // if one of these is reachable both must be to avoid collection from weak hashmap, so
+ // set an attribute holding this object to ensure we never get collected until the ServletContext
+ // is eligible for collection too.
+ ctx.setAttribute(this.getClass().getName(), this);
+ this.ctx = ctx;
+ }
+
+ public synchronized ServletContext getCtx() {
+ return ctx;
+ }
+
+ public synchronized void setCtx(ServletContext ctx) {
+ this.ctx = ctx;
+ }
+
+ synchronized void makeReady() {
+ this.initializing.countDown();
+ }
+
+ // NOT synchronized :)
+ public void waitForReadyService() throws InterruptedException {
+ initializing.await();
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ ContextInitializationKey that = (ContextInitializationKey) o;
+ return ctx.equals(that.ctx);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(ctx);
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `CoreContainerProvider$ContextInitializationKey.hashCode()` reads without synchronization from `this.ctx`. Potentially races with write in method `CoreContainerProvider$ContextInitializationKey.setCtx(...)`.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] markrmiller commented on pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
markrmiller commented on pull request #265:
URL: https://github.com/apache/solr/pull/265#issuecomment-907035777
Also, this is literally littered all over: "The init method must complete successfully before the filter is asked to do any filtering work." The authors may not have said it out load, but its fundamental filter doc reverberating around the internet.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gus-asf commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
gus-asf commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r694823001
##########
File path: solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
##########
@@ -438,147 +170,88 @@ public void doFilter(ServletRequest _request, ServletResponse _response, FilterC
HttpServletRequest request = closeShield((HttpServletRequest)_request, retry);
HttpServletResponse response = closeShield((HttpServletResponse)_response, retry);
- String requestPath = ServletUtils.getPathAfterContext(request);
- // No need to even create the HttpSolrCall object if this path is excluded.
- if (excludePatterns != null) {
- for (Pattern p : excludePatterns) {
- Matcher matcher = p.matcher(requestPath);
- if (matcher.lookingAt()) {
- chain.doFilter(request, response);
- return;
- }
- }
+ if (excludedPath(excludePatterns, request, response, chain)) {
+ return;
}
-
- Tracer tracer = cores == null ? GlobalTracer.get() : cores.getTracer();
- Span span = buildSpan(tracer, request);
- request.setAttribute(Tracer.class.getName(), tracer);
- request.setAttribute(Span.class.getName(), span);
- boolean accepted = false;
- try (var scope = tracer.scopeManager().activate(span)) {
- assert scope != null; // prevent javac warning about scope being unused
-
- if (cores == null || cores.isShutDown()) {
- try {
- init.await();
- } catch (InterruptedException e) { //well, no wait then
- }
- final String msg = "Error processing the request. CoreContainer is either not initialized or shutting down.";
- if (cores == null || cores.isShutDown()) {
- log.error(msg);
- throw new UnavailableException(msg);
- }
- }
-
+ Tracer t = getCores() == null ? GlobalTracer.get() : getCores().getTracer();
+ request.setAttribute(Tracer.class.getName(), t);
+ RateLimitManager rateLimitManager = coreService.getService().getRateLimitManager();
+ request.setAttribute(RateLimitManager.class.getName(), rateLimitManager);
+ ServletUtils.rateLimitRequest(request, response, () -> {
try {
- accepted = rateLimitManager.handleRequest(request);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- throw new SolrException(ErrorCode.SERVER_ERROR, e.getMessage());
+ dispatch(chain, request, response, retry);
+ } catch (IOException | ServletException | SolrAuthenticationException e) {
+ throw new ExceptionWhileTracing( e);
}
+ }, true);
+ }
- if (!accepted) {
- String errorMessage = "Too many requests for this request type." +
- "Please try after some time or increase the quota for this request type";
-
- response.sendError(429, errorMessage);
- }
+ private static Span getSpan(HttpServletRequest req) {
+ return (Span) req.getAttribute(ATTR_TRACING_SPAN);
+ }
- AtomicReference<HttpServletRequest> wrappedRequest = new AtomicReference<>();
- if (!authenticateRequest(request, response, wrappedRequest)) { // the response and status code have already been sent
- return;
- }
+ private void dispatch(FilterChain chain, HttpServletRequest request, HttpServletResponse response, boolean retry) throws IOException, ServletException, SolrAuthenticationException {
- if (wrappedRequest.get() != null) {
- request = wrappedRequest.get();
- }
- if (cores.getAuthenticationPlugin() != null) {
- if (log.isDebugEnabled()) {
- log.debug("User principal: {}", request.getUserPrincipal());
- }
- span.setTag(Tags.DB_USER, String.valueOf(request.getUserPrincipal()));
- }
- HttpSolrCall call = getHttpSolrCall(request, response, retry);
- ExecutorUtil.setServerThreadFlag(Boolean.TRUE);
- try {
- Action result = call.call();
- switch (result) {
- case PASSTHROUGH:
- span.log("SolrDispatchFilter PASSTHROUGH");
- chain.doFilter(request, response);
- break;
- case RETRY:
- span.log("SolrDispatchFilter RETRY");
- doFilter(request, response, chain, true); // RECURSION
- break;
- case FORWARD:
- span.log("SolrDispatchFilter FORWARD");
- request.getRequestDispatcher(call.getPath()).forward(request, response);
- break;
- case ADMIN:
- case PROCESS:
- case REMOTEQUERY:
- case RETURN:
- break;
- }
- } finally {
- call.destroy();
- ExecutorUtil.setServerThreadFlag(null);
- }
- } finally {
- consumeInputFully(request, response);
- SolrRequestInfo.reset();
- SolrRequestParsers.cleanupMultipartFiles(request);
-
- if (accepted) {
- rateLimitManager.decrementActiveRequests(request);
- }
- span.setTag(Tags.HTTP_STATUS, response.getStatus());
- span.finish();
+ AtomicReference<HttpServletRequest> wrappedRequest = new AtomicReference<>();
+ if (!authenticateRequest(request, response, wrappedRequest)) { // the response and status code have already been sent
Review comment:
actually... I should probably just throw from the method and return the request.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
dsmiley commented on pull request #265:
URL: https://github.com/apache/solr/pull/265#issuecomment-906667849
> Seems like it should have generated a ticket with Jetty at least since while the spec doesn't actually state that the call to init() must have been completed, that sure sounds like something the spec authors wouldn't have thought necessary to say...
100% agree.
We agree on a number of things, which is great. Can you try to pursue these step-by-step? The current PR could perhaps be limited to only decoupling CoreContainer initialization from SolrDispatchFilter but not more. Then Tracing & Rate Limiting could be broken off. I'll help with Tracing. SDF can become a Filter, either before or after Tracing & Rate Limiting become Filters. Ditch "excludePatterns" -- find out who added it and talk with them in that JIRA issue.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gus-asf commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
gus-asf commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r696746079
##########
File path: solr/core/src/java/org/apache/solr/servlet/CoreService.java
##########
@@ -0,0 +1,451 @@
+/*
+ * 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.servlet;
+
+import com.codahale.metrics.jvm.ClassLoadingGaugeSet;
+import com.codahale.metrics.jvm.GarbageCollectorMetricSet;
+import com.codahale.metrics.jvm.MemoryUsageGaugeSet;
+import com.codahale.metrics.jvm.ThreadStatesGaugeSet;
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.http.client.HttpClient;
+import org.apache.lucene.util.Version;
+import org.apache.solr.cloud.ZkController;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrInfoBean.Group;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.metrics.AltBufferPoolMetricSet;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.OperatingSystemMetricSet;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricManager.ResolutionStrategy;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.servlet.RateLimitManager.Builder;
+import org.apache.solr.util.StartupLoggingUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
+import javax.naming.NoInitialContextException;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
+import javax.servlet.UnavailableException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.Set;
+import java.util.WeakHashMap;
+import java.util.concurrent.CountDownLatch;
+
+import static org.apache.solr.servlet.SolrCoreUtils.loadNodeConfig;
+import static org.apache.solr.servlet.SolrDispatchFilter.PROPERTIES_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLRHOME_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_INSTALL_DIR_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_LEVEL;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_MUTECONSOLE;
+
+/**
+ * A service that can provide access to solr cores. This allows us to have multiple filters and
+ * servlets that depend on SolrCore and CoreContainer, while still only having one CoreContainer per
+ * instance of solr.
+ */
+public class CoreService implements ServletContextListener {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private final String metricTag = SolrMetricProducer.getUniqueMetricTag(this, null);
+ private CoreContainer cores;
+ private Properties extraProperties;
+ private HttpClient httpClient;
+ private SolrMetricManager metricManager;
+ private RateLimitManager rateLimitManager;
+ private final CountDownLatch init = new CountDownLatch(1);
+ private String registryName;
+
+ // AFAIK the only reason we need this is to support JettySolrRunner for tests. In tests we might have
+ // multiple CoreContainers in the same JVM, but I *think* that doesn't happen in a real server.
+ private static final Map<ContextInitializationKey, ServiceHolder> services =
+ Collections.synchronizedMap(new WeakHashMap<>());
+
+ // todo: dependency injection instead, but CDI is not jetty native so for now this method and the associated
+ // map will have to suffice. Note that this relies on ServletContext.equals() not implementing anything significantly
+ // different thant Object.equals for its .equals method (I've found no implementation that even immplements it).
+ public static ServiceHolder serviceForContext(ServletContext ctx) throws InterruptedException {
+ ContextInitializationKey key = new ContextInitializationKey(ctx);
+ return services.computeIfAbsent(key, ServiceHolder::new);
+ }
+
+ @Override
+ public void contextInitialized(ServletContextEvent sce) {
+ init(sce.getServletContext());
+ }
+
+ @Override
+ public void contextDestroyed(ServletContextEvent sce) {
+ close();
+ }
+
+ CoreContainer getCoreContainer() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return cores;
+ }
+ HttpClient getHttpClient() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return httpClient;
+ }
+
+ private static void waitForCoreContainer(CoreContainerProvider provider, CountDownLatch latch) throws UnavailableException {
+ CoreContainer cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ try {
+ latch.await();
+ } catch (InterruptedException e) { //well, no wait then
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] madrob commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r696759269
##########
File path: solr/core/src/java/org/apache/solr/servlet/CoreService.java
##########
@@ -0,0 +1,451 @@
+/*
+ * 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.servlet;
+
+import com.codahale.metrics.jvm.ClassLoadingGaugeSet;
+import com.codahale.metrics.jvm.GarbageCollectorMetricSet;
+import com.codahale.metrics.jvm.MemoryUsageGaugeSet;
+import com.codahale.metrics.jvm.ThreadStatesGaugeSet;
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.http.client.HttpClient;
+import org.apache.lucene.util.Version;
+import org.apache.solr.cloud.ZkController;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrInfoBean.Group;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.metrics.AltBufferPoolMetricSet;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.OperatingSystemMetricSet;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricManager.ResolutionStrategy;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.servlet.RateLimitManager.Builder;
+import org.apache.solr.util.StartupLoggingUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
+import javax.naming.NoInitialContextException;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
+import javax.servlet.UnavailableException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.Set;
+import java.util.WeakHashMap;
+import java.util.concurrent.CountDownLatch;
+
+import static org.apache.solr.servlet.SolrCoreUtils.loadNodeConfig;
+import static org.apache.solr.servlet.SolrDispatchFilter.PROPERTIES_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLRHOME_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_INSTALL_DIR_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_LEVEL;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_MUTECONSOLE;
+
+/**
+ * A service that can provide access to solr cores. This allows us to have multiple filters and
+ * servlets that depend on SolrCore and CoreContainer, while still only having one CoreContainer per
+ * instance of solr.
+ */
+public class CoreService implements ServletContextListener {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private final String metricTag = SolrMetricProducer.getUniqueMetricTag(this, null);
+ private CoreContainer cores;
+ private Properties extraProperties;
+ private HttpClient httpClient;
+ private SolrMetricManager metricManager;
+ private RateLimitManager rateLimitManager;
+ private final CountDownLatch init = new CountDownLatch(1);
+ private String registryName;
+
+ // AFAIK the only reason we need this is to support JettySolrRunner for tests. In tests we might have
+ // multiple CoreContainers in the same JVM, but I *think* that doesn't happen in a real server.
+ private static final Map<ContextInitializationKey, ServiceHolder> services =
+ Collections.synchronizedMap(new WeakHashMap<>());
+
+ // todo: dependency injection instead, but CDI is not jetty native so for now this method and the associated
+ // map will have to suffice. Note that this relies on ServletContext.equals() not implementing anything significantly
+ // different thant Object.equals for its .equals method (I've found no implementation that even immplements it).
+ public static ServiceHolder serviceForContext(ServletContext ctx) throws InterruptedException {
+ ContextInitializationKey key = new ContextInitializationKey(ctx);
+ return services.computeIfAbsent(key, ServiceHolder::new);
+ }
+
+ @Override
+ public void contextInitialized(ServletContextEvent sce) {
+ init(sce.getServletContext());
+ }
+
+ @Override
+ public void contextDestroyed(ServletContextEvent sce) {
+ close();
+ }
+
+ CoreContainer getCoreContainer() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return cores;
+ }
+ HttpClient getHttpClient() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return httpClient;
+ }
+
+ private static void waitForCoreContainer(CoreContainerProvider provider, CountDownLatch latch) throws UnavailableException {
+ CoreContainer cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ try {
+ latch.await();
Review comment:
I think that’sa good point, it’s not like a single request that we can fall if it’s taking too long. Even so, waking up every 10s to log that we’re still waiting to start up would be nice
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gus-asf commented on pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
gus-asf commented on pull request #265:
URL: https://github.com/apache/solr/pull/265#issuecomment-906544658
> If the idea is to leverage the constructs in the servlet API more to our advantage, then perhaps both Tracing and Rate Limiting should be servlet Filters?
>
YES :) ... very much something this is meant to enable in the future.
> FWIW I don't find SolrDispatchFilter _that_ bad, though there are some warts and my other PR ( #155 ) tries to tackle some. If you read the opening of my PR there, you'll see I dug deeper into some complexities like the cores==null checks that seem unnecessary. Apparently those are there due to Mikhail's realization in SOLR-10615 that init() isn't necessary complete before doFilter can be called which shocked him and me too; I'm kinda in disbelief honestly.
Seems like it should have generated a ticket with Jetty at least since while the spec doesn't actually state that the call to init() must have been completed, that sure sounds like something the spec authors wouldn't have thought necessary to say...
6.2.1 Filter Lifecycle
... snip ...
The container must ensure that it has instantiated a filter of the appropriate class for each filter in the list, and called
its init(FilterConfig config) method. The filter may throw an exception to indicate that it cannot function properly. If the exception is of type UnavailableException, the container may examine the isPermanent attribute of
the exception and may choose to retry the filter at some later time
... snip ...
And this section implies that waiting is the wrong strategy anyway. Requests to a server not fully started should fail temporarily unavailable. (how well that plays with tests might be an issue that caused us to wait though).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] sonatype-lift[bot] commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r690499793
##########
File path: solr/core/src/java/org/apache/solr/servlet/CoreService.java
##########
@@ -0,0 +1,451 @@
+/*
+ * 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.servlet;
+
+import com.codahale.metrics.jvm.ClassLoadingGaugeSet;
+import com.codahale.metrics.jvm.GarbageCollectorMetricSet;
+import com.codahale.metrics.jvm.MemoryUsageGaugeSet;
+import com.codahale.metrics.jvm.ThreadStatesGaugeSet;
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.http.client.HttpClient;
+import org.apache.lucene.util.Version;
+import org.apache.solr.cloud.ZkController;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrInfoBean.Group;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.metrics.AltBufferPoolMetricSet;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.OperatingSystemMetricSet;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricManager.ResolutionStrategy;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.servlet.RateLimitManager.Builder;
+import org.apache.solr.util.StartupLoggingUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
+import javax.naming.NoInitialContextException;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
+import javax.servlet.UnavailableException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.Set;
+import java.util.WeakHashMap;
+import java.util.concurrent.CountDownLatch;
+
+import static org.apache.solr.servlet.SolrCoreUtils.loadNodeConfig;
+import static org.apache.solr.servlet.SolrDispatchFilter.PROPERTIES_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLRHOME_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_INSTALL_DIR_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_LEVEL;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_MUTECONSOLE;
+
+/**
+ * A service that can provide access to solr cores. This allows us to have multiple filters and
+ * servlets that depend on SolrCore and CoreContainer, while still only having one CoreContainer per
+ * instance of solr.
+ */
+public class CoreService implements ServletContextListener {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private final String metricTag = SolrMetricProducer.getUniqueMetricTag(this, null);
+ private CoreContainer cores;
+ private Properties extraProperties;
+ private HttpClient httpClient;
+ private SolrMetricManager metricManager;
+ private RateLimitManager rateLimitManager;
+ private final CountDownLatch init = new CountDownLatch(1);
+ private String registryName;
+
+ // AFAIK the only reason we need this is to support JettySolrRunner for tests. In tests we might have
+ // multiple CoreContainers in the same JVM, but I *think* that doesn't happen in a real server.
+ private static final Map<ContextInitializationKey, ServiceHolder> services =
+ Collections.synchronizedMap(new WeakHashMap<>());
+
+ // todo: dependency injection instead, but CDI is not jetty native so for now this method and the associated
+ // map will have to suffice. Note that this relies on ServletContext.equals() not implementing anything significantly
+ // different thant Object.equals for its .equals method (I've found no implementation that even immplements it).
+ public static ServiceHolder serviceForContext(ServletContext ctx) throws InterruptedException {
+ ContextInitializationKey key = new ContextInitializationKey(ctx);
+ return services.computeIfAbsent(key, ServiceHolder::new);
+ }
+
+ @Override
+ public void contextInitialized(ServletContextEvent sce) {
+ init(sce.getServletContext());
+ }
+
+ @Override
+ public void contextDestroyed(ServletContextEvent sce) {
+ close();
+ }
+
+ CoreContainer getCoreContainer() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return cores;
+ }
+ HttpClient getHttpClient() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return httpClient;
+ }
+
+ private static void waitForCoreContainer(CoreContainerProvider provider, CountDownLatch latch) throws UnavailableException {
+ CoreContainer cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ try {
+ latch.await();
+ } catch (InterruptedException e) { //well, no wait then
+ }
+ cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ final String msg = "Error processing the request. CoreContainer is either not initialized or shutting down.";
+ log.error(msg);
+ throw new UnavailableException(msg);
+ }
+ }
+ }
+ public void close() {
+ CoreContainer cc = cores;
+ cores = null;
+ try {
+ if (metricManager != null) {
+ try {
+ metricManager.unregisterGauges(registryName, metricTag);
+ } catch (NullPointerException e) {
+ // okay
+ } catch (Exception e) {
+ log.warn("Exception closing FileCleaningTracker", e);
+ } finally {
+ metricManager = null;
+ }
+ }
+ } finally {
+ if (cc != null) {
+ httpClient = null;
+ cc.shutdown();
+ }
+ }
+ }
+
+ public void init(ServletContext servletContext) {
+ if (log.isTraceEnabled()) {
+ log.trace("CoreService.init(): {}", this.getClass().getClassLoader());
+ }
+ CoreContainer coresInit = null;
+ try {
+ // "extra" properties must be init'ed first so we know things like "do we have a zkHost"
+ // wrap as defaults (if set) so we can modify w/o polluting the Properties provided by our caller
+ this.extraProperties = SolrXmlConfig.wrapAndSetZkHostFromSysPropIfNeeded
+ ((Properties) servletContext.getAttribute(PROPERTIES_ATTRIBUTE));
+
+ StartupLoggingUtils.checkLogDir();
+ if (log.isInfoEnabled()) {
+ log.info("Using logger factory {}", StartupLoggingUtils.getLoggerImplStr());
+ }
+
+ logWelcomeBanner();
+
+ String muteConsole = System.getProperty(SOLR_LOG_MUTECONSOLE);
+ if (muteConsole != null && !Arrays.asList("false","0","off","no").contains(muteConsole.toLowerCase(Locale.ROOT))) {
+ StartupLoggingUtils.muteConsole();
+ }
+ String logLevel = System.getProperty(SOLR_LOG_LEVEL);
+ if (logLevel != null) {
+ log.info("Log level override, property solr.log.level={}", logLevel);
+ StartupLoggingUtils.changeLogLevel(logLevel);
+ }
+
+ coresInit = createCoreContainer(computeSolrHome(servletContext), extraProperties);
+ this.httpClient = coresInit.getUpdateShardHandler().getDefaultHttpClient();
+ setupJvmMetrics(coresInit);
+
+ SolrZkClient zkClient = null;
+ ZkController zkController = coresInit.getZkController();
+
+ if (zkController != null) {
+ zkClient = zkController.getZkClient();
+ }
+
+ Builder builder = new Builder(zkClient);
+
+ this.rateLimitManager = builder.build();
+
+ if (zkController != null) {
+ zkController.zkStateReader.registerClusterPropertiesListener(this.rateLimitManager);
+ }
+
+ if (log.isDebugEnabled()) {
+ log.debug("user.dir={}", System.getProperty("user.dir"));
+ }
+ } catch( Throwable t ) {
+ // catch this so our filter still works
+ log.error( "Could not start Solr. Check solr/home property and the logs");
+ SolrCore.log( t );
+ if (t instanceof Error) {
+ throw (Error) t;
+ }
+ } finally{
+ log.trace("SolrDispatchFilter.init() done");
+ this.cores = coresInit; // crucially final assignment
+ services.computeIfAbsent(new ContextInitializationKey(servletContext), ServiceHolder::new)
+ .setService(this);
+ init.countDown();
+ }
+ }
+
+
+ private void logWelcomeBanner() {
+ // _Really_ sorry about how clumsy this is as a result of the logging call checker, but this is the only one
+ // that's so ugly so far.
+ if (log.isInfoEnabled()) {
+ log.info(" ___ _ Welcome to Apache Solr™ version {}", solrVersion());
+ }
+ if (log.isInfoEnabled()) {
+ log.info("/ __| ___| |_ _ Starting in {} mode on port {}", isCloudMode() ? "cloud" : "standalone", getSolrPort());
+ }
+ if (log.isInfoEnabled()) {
+ log.info("\\__ \\/ _ \\ | '_| Install dir: {}", System.getProperty(SOLR_INSTALL_DIR_ATTRIBUTE));
+ }
+ if (log.isInfoEnabled()) {
+ log.info("|___/\\___/_|_| Start time: {}", Instant.now());
+ }
+ }
+ private String solrVersion() {
+ String specVer = Version.LATEST.toString();
+ try {
+ String implVer = SolrCore.class.getPackage().getImplementationVersion();
+ return (specVer.equals(implVer.split(" ")[0])) ? specVer : implVer;
+ } catch (Exception e) {
+ return specVer;
+ }
+ }
+
+ private String getSolrPort() {
+ return System.getProperty("jetty.port");
+ }
+
+ /**
+ * We are in cloud mode if Java option zkRun exists OR zkHost exists and is non-empty
+ * @see SolrXmlConfig#wrapAndSetZkHostFromSysPropIfNeeded
+ * @see #extraProperties
+ * @see #init
+ */
+ private boolean isCloudMode() {
+ assert null != extraProperties; // we should never be called w/o this being initialized
+ return (null != extraProperties.getProperty(SolrXmlConfig.ZK_HOST)) || (null != System.getProperty("zkRun"));
+ }
+
+ /**
+ * Returns the effective Solr Home to use for this node, based on looking up the value in this order:
+ * <ol>
+ * <li>attribute in the FilterConfig</li>
+ * <li>JNDI: via java:comp/env/solr/home</li>
+ * <li>The system property solr.solr.home</li>
+ * <li>Look in the current working directory for a solr/ directory</li>
+ * </ol>
+ * <p>
+ *
+ * @return the Solr home, absolute and normalized.
+ */
+ private static Path computeSolrHome(ServletContext servletContext) {
+
+ // start with explicit check of servlet config...
+ String source = "servlet config: " + SOLRHOME_ATTRIBUTE;
+ String home = (String) servletContext.getAttribute(SOLRHOME_ATTRIBUTE);
+
+ if (null == home) {
+ final String lookup = "java:comp/env/solr/home";
+ // Try JNDI
+ source = "JNDI: " + lookup;
+ try {
+ Context c = new InitialContext();
+ home = (String) c.lookup(lookup);
+ } catch (NoInitialContextException e) {
+ log.debug("JNDI not configured for solr (NoInitialContextEx)");
+ } catch (NamingException e) {
+ log.debug("No /solr/home in JNDI");
+ } catch (RuntimeException ex) {
+ log.warn("Odd RuntimeException while testing for JNDI: ", ex);
+ }
+ }
+
+ if (null == home) {
+ // Now try system property
+ final String prop = "solr.solr.home";
+ source = "system property: " + prop;
+ home = System.getProperty(prop);
+ }
+
+ if (null == home) {
+ // if all else fails, assume default dir
+ home = "solr/";
+ source = "defaulted to '" + home + "' ... could not find system property or JNDI";
+ }
+ final Path solrHome = Paths.get(home).toAbsolutePath().normalize();
Review comment:
*PATH_TRAVERSAL_IN:* This API (java/nio/file/Paths.get(Ljava/lang/String;[Ljava/lang/String;)Ljava/nio/file/Path;) reads a file whose location might be specified by user input [(details)](https://find-sec-bugs.github.io/bugs.htm#PATH_TRAVERSAL_IN)
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java
##########
@@ -382,7 +386,8 @@ public void lifeCycleStarted(LifeCycle arg0) {
root.getServletContext().setAttribute(SolrDispatchFilter.PROPERTIES_ATTRIBUTE, nodeProperties);
root.getServletContext().setAttribute(SolrDispatchFilter.SOLRHOME_ATTRIBUTE, solrHome);
-
+ coreService = new CoreService();
+ coreService.init(root.getServletContext());
Review comment:
*THREAD_SAFETY_VIOLATION:* Unprotected write. Non-private method `JettySolrRunner$1.lifeCycleStarted(...)` indirectly writes to field `sql.CalciteSolrDriver.INSTANCE.solrClientCache` outside of synchronization.
Reporting because this access may occur on a background thread.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java
##########
@@ -382,7 +386,8 @@ public void lifeCycleStarted(LifeCycle arg0) {
root.getServletContext().setAttribute(SolrDispatchFilter.PROPERTIES_ATTRIBUTE, nodeProperties);
root.getServletContext().setAttribute(SolrDispatchFilter.SOLRHOME_ATTRIBUTE, solrHome);
-
+ coreService = new CoreService();
Review comment:
*THREAD_SAFETY_VIOLATION:* Unprotected write. Non-private method `JettySolrRunner$1.lifeCycleStarted(...)` writes to field `this.this$0.coreService` outside of synchronization.
Reporting because this access may occur on a background thread.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java
##########
@@ -364,7 +367,9 @@ public void lifeCycleStopping(LifeCycle arg0) {
}
@Override
- public void lifeCycleStopped(LifeCycle arg0) {}
+ public void lifeCycleStopped(LifeCycle arg0) {
+ coreService.close();
Review comment:
*THREAD_SAFETY_VIOLATION:* Unprotected write. Non-private method `JettySolrRunner$1.lifeCycleStopped(...)` indirectly writes to field `noggit.JSONParser.devNull.buf` outside of synchronization.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java
##########
@@ -638,19 +647,15 @@ public void stop() throws Exception {
if (sdf != null) {
customThreadPool = ExecutorUtil.newMDCAwareCachedThreadPool(new SolrNamedThreadFactory("jettyShutDown"));
- sdf.closeOnDestroy(false);
+ sdf.closeOnDestroy();
// customThreadPool.submit(() -> {
// try {
// sdf.close();
// } catch (Throwable t) {
// log.error("Error shutting down Solr", t);
// }
// });
- try {
- sdf.close();
- } catch (Throwable t) {
- log.error("Error shutting down Solr", t);
- }
+
}
QueuedThreadPool qtp = (QueuedThreadPool) server.getThreadPool();
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `JettySolrRunner.stop()` reads without synchronization from `this.server`. Potentially races with write in method `JettySolrRunner.start(...)`.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/servlet/CoreService.java
##########
@@ -0,0 +1,451 @@
+/*
+ * 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.servlet;
+
+import com.codahale.metrics.jvm.ClassLoadingGaugeSet;
+import com.codahale.metrics.jvm.GarbageCollectorMetricSet;
+import com.codahale.metrics.jvm.MemoryUsageGaugeSet;
+import com.codahale.metrics.jvm.ThreadStatesGaugeSet;
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.http.client.HttpClient;
+import org.apache.lucene.util.Version;
+import org.apache.solr.cloud.ZkController;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrInfoBean.Group;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.metrics.AltBufferPoolMetricSet;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.OperatingSystemMetricSet;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricManager.ResolutionStrategy;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.servlet.RateLimitManager.Builder;
+import org.apache.solr.util.StartupLoggingUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
+import javax.naming.NoInitialContextException;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
+import javax.servlet.UnavailableException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.Set;
+import java.util.WeakHashMap;
+import java.util.concurrent.CountDownLatch;
+
+import static org.apache.solr.servlet.SolrCoreUtils.loadNodeConfig;
+import static org.apache.solr.servlet.SolrDispatchFilter.PROPERTIES_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLRHOME_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_INSTALL_DIR_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_LEVEL;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_MUTECONSOLE;
+
+/**
+ * A service that can provide access to solr cores. This allows us to have multiple filters and
+ * servlets that depend on SolrCore and CoreContainer, while still only having one CoreContainer per
+ * instance of solr.
+ */
+public class CoreService implements ServletContextListener {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private final String metricTag = SolrMetricProducer.getUniqueMetricTag(this, null);
+ private CoreContainer cores;
+ private Properties extraProperties;
+ private HttpClient httpClient;
+ private SolrMetricManager metricManager;
+ private RateLimitManager rateLimitManager;
+ private final CountDownLatch init = new CountDownLatch(1);
+ private String registryName;
+
+ // AFAIK the only reason we need this is to support JettySolrRunner for tests. In tests we might have
+ // multiple CoreContainers in the same JVM, but I *think* that doesn't happen in a real server.
+ private static final Map<ContextInitializationKey, ServiceHolder> services =
+ Collections.synchronizedMap(new WeakHashMap<>());
+
+ // todo: dependency injection instead, but CDI is not jetty native so for now this method and the associated
+ // map will have to suffice. Note that this relies on ServletContext.equals() not implementing anything significantly
+ // different thant Object.equals for its .equals method (I've found no implementation that even immplements it).
+ public static ServiceHolder serviceForContext(ServletContext ctx) throws InterruptedException {
+ ContextInitializationKey key = new ContextInitializationKey(ctx);
+ return services.computeIfAbsent(key, ServiceHolder::new);
+ }
+
+ @Override
+ public void contextInitialized(ServletContextEvent sce) {
+ init(sce.getServletContext());
+ }
+
+ @Override
+ public void contextDestroyed(ServletContextEvent sce) {
+ close();
+ }
+
+ CoreContainer getCoreContainer() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return cores;
+ }
+ HttpClient getHttpClient() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return httpClient;
+ }
+
+ private static void waitForCoreContainer(CoreContainerProvider provider, CountDownLatch latch) throws UnavailableException {
+ CoreContainer cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ try {
+ latch.await();
+ } catch (InterruptedException e) { //well, no wait then
+ }
+ cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ final String msg = "Error processing the request. CoreContainer is either not initialized or shutting down.";
+ log.error(msg);
+ throw new UnavailableException(msg);
+ }
+ }
+ }
+ public void close() {
+ CoreContainer cc = cores;
+ cores = null;
+ try {
+ if (metricManager != null) {
+ try {
+ metricManager.unregisterGauges(registryName, metricTag);
+ } catch (NullPointerException e) {
+ // okay
+ } catch (Exception e) {
+ log.warn("Exception closing FileCleaningTracker", e);
+ } finally {
+ metricManager = null;
+ }
+ }
+ } finally {
+ if (cc != null) {
+ httpClient = null;
+ cc.shutdown();
+ }
+ }
+ }
+
+ public void init(ServletContext servletContext) {
+ if (log.isTraceEnabled()) {
+ log.trace("CoreService.init(): {}", this.getClass().getClassLoader());
+ }
+ CoreContainer coresInit = null;
+ try {
+ // "extra" properties must be init'ed first so we know things like "do we have a zkHost"
+ // wrap as defaults (if set) so we can modify w/o polluting the Properties provided by our caller
+ this.extraProperties = SolrXmlConfig.wrapAndSetZkHostFromSysPropIfNeeded
+ ((Properties) servletContext.getAttribute(PROPERTIES_ATTRIBUTE));
+
+ StartupLoggingUtils.checkLogDir();
+ if (log.isInfoEnabled()) {
+ log.info("Using logger factory {}", StartupLoggingUtils.getLoggerImplStr());
+ }
+
+ logWelcomeBanner();
+
+ String muteConsole = System.getProperty(SOLR_LOG_MUTECONSOLE);
+ if (muteConsole != null && !Arrays.asList("false","0","off","no").contains(muteConsole.toLowerCase(Locale.ROOT))) {
+ StartupLoggingUtils.muteConsole();
+ }
+ String logLevel = System.getProperty(SOLR_LOG_LEVEL);
+ if (logLevel != null) {
+ log.info("Log level override, property solr.log.level={}", logLevel);
+ StartupLoggingUtils.changeLogLevel(logLevel);
+ }
+
+ coresInit = createCoreContainer(computeSolrHome(servletContext), extraProperties);
+ this.httpClient = coresInit.getUpdateShardHandler().getDefaultHttpClient();
+ setupJvmMetrics(coresInit);
+
+ SolrZkClient zkClient = null;
+ ZkController zkController = coresInit.getZkController();
+
+ if (zkController != null) {
+ zkClient = zkController.getZkClient();
+ }
+
+ Builder builder = new Builder(zkClient);
+
+ this.rateLimitManager = builder.build();
+
+ if (zkController != null) {
+ zkController.zkStateReader.registerClusterPropertiesListener(this.rateLimitManager);
+ }
+
+ if (log.isDebugEnabled()) {
+ log.debug("user.dir={}", System.getProperty("user.dir"));
+ }
+ } catch( Throwable t ) {
+ // catch this so our filter still works
+ log.error( "Could not start Solr. Check solr/home property and the logs");
+ SolrCore.log( t );
+ if (t instanceof Error) {
+ throw (Error) t;
+ }
+ } finally{
+ log.trace("SolrDispatchFilter.init() done");
+ this.cores = coresInit; // crucially final assignment
+ services.computeIfAbsent(new ContextInitializationKey(servletContext), ServiceHolder::new)
+ .setService(this);
+ init.countDown();
+ }
+ }
+
+
+ private void logWelcomeBanner() {
+ // _Really_ sorry about how clumsy this is as a result of the logging call checker, but this is the only one
+ // that's so ugly so far.
+ if (log.isInfoEnabled()) {
+ log.info(" ___ _ Welcome to Apache Solr™ version {}", solrVersion());
+ }
+ if (log.isInfoEnabled()) {
+ log.info("/ __| ___| |_ _ Starting in {} mode on port {}", isCloudMode() ? "cloud" : "standalone", getSolrPort());
+ }
+ if (log.isInfoEnabled()) {
+ log.info("\\__ \\/ _ \\ | '_| Install dir: {}", System.getProperty(SOLR_INSTALL_DIR_ATTRIBUTE));
+ }
+ if (log.isInfoEnabled()) {
+ log.info("|___/\\___/_|_| Start time: {}", Instant.now());
+ }
+ }
+ private String solrVersion() {
+ String specVer = Version.LATEST.toString();
+ try {
+ String implVer = SolrCore.class.getPackage().getImplementationVersion();
+ return (specVer.equals(implVer.split(" ")[0])) ? specVer : implVer;
+ } catch (Exception e) {
+ return specVer;
+ }
+ }
+
+ private String getSolrPort() {
+ return System.getProperty("jetty.port");
+ }
+
+ /**
+ * We are in cloud mode if Java option zkRun exists OR zkHost exists and is non-empty
+ * @see SolrXmlConfig#wrapAndSetZkHostFromSysPropIfNeeded
+ * @see #extraProperties
+ * @see #init
+ */
+ private boolean isCloudMode() {
+ assert null != extraProperties; // we should never be called w/o this being initialized
+ return (null != extraProperties.getProperty(SolrXmlConfig.ZK_HOST)) || (null != System.getProperty("zkRun"));
+ }
+
+ /**
+ * Returns the effective Solr Home to use for this node, based on looking up the value in this order:
+ * <ol>
+ * <li>attribute in the FilterConfig</li>
+ * <li>JNDI: via java:comp/env/solr/home</li>
+ * <li>The system property solr.solr.home</li>
+ * <li>Look in the current working directory for a solr/ directory</li>
+ * </ol>
+ * <p>
+ *
+ * @return the Solr home, absolute and normalized.
+ */
+ private static Path computeSolrHome(ServletContext servletContext) {
+
+ // start with explicit check of servlet config...
+ String source = "servlet config: " + SOLRHOME_ATTRIBUTE;
+ String home = (String) servletContext.getAttribute(SOLRHOME_ATTRIBUTE);
+
+ if (null == home) {
+ final String lookup = "java:comp/env/solr/home";
+ // Try JNDI
+ source = "JNDI: " + lookup;
+ try {
+ Context c = new InitialContext();
+ home = (String) c.lookup(lookup);
+ } catch (NoInitialContextException e) {
+ log.debug("JNDI not configured for solr (NoInitialContextEx)");
+ } catch (NamingException e) {
+ log.debug("No /solr/home in JNDI");
+ } catch (RuntimeException ex) {
+ log.warn("Odd RuntimeException while testing for JNDI: ", ex);
+ }
+ }
+
+ if (null == home) {
+ // Now try system property
+ final String prop = "solr.solr.home";
+ source = "system property: " + prop;
+ home = System.getProperty(prop);
+ }
+
+ if (null == home) {
+ // if all else fails, assume default dir
+ home = "solr/";
+ source = "defaulted to '" + home + "' ... could not find system property or JNDI";
+ }
+ final Path solrHome = Paths.get(home).toAbsolutePath().normalize();
+ log.info("Solr Home: {} (source: {})", solrHome, source);
+
+ return solrHome;
+ }
+
+ /**
+ * Override this to change CoreContainer initialization
+ * @return a CoreContainer to hold this server's cores
+ */
+ protected CoreContainer createCoreContainer(Path solrHome, Properties nodeProps) {
+ NodeConfig nodeConfig = loadNodeConfig(solrHome, nodeProps);
+ final CoreContainer coreContainer = new CoreContainer(nodeConfig, true);
+ coreContainer.load();
+ return coreContainer;
+ }
+
+
+
+ private void setupJvmMetrics(CoreContainer coresInit) {
+ metricManager = coresInit.getMetricManager();
+ registryName = SolrMetricManager.getRegistryName(Group.jvm);
+ final Set<String> hiddenSysProps = coresInit.getConfig().getMetricsConfig().getHiddenSysProps();
+ try {
+ metricManager.registerAll(registryName, new AltBufferPoolMetricSet(), ResolutionStrategy.IGNORE, "buffers");
+ metricManager.registerAll(registryName, new ClassLoadingGaugeSet(), ResolutionStrategy.IGNORE, "classes");
+ metricManager.registerAll(registryName, new OperatingSystemMetricSet(), ResolutionStrategy.IGNORE, "os");
+ metricManager.registerAll(registryName, new GarbageCollectorMetricSet(), ResolutionStrategy.IGNORE, "gc");
+ metricManager.registerAll(registryName, new MemoryUsageGaugeSet(), ResolutionStrategy.IGNORE, "memory");
+ metricManager.registerAll(registryName, new ThreadStatesGaugeSet(), ResolutionStrategy.IGNORE, "threads"); // todo should we use CachedThreadStatesGaugeSet instead?
+ MetricsMap sysprops = new MetricsMap(map -> System.getProperties().forEach((k, v) -> {
+ //noinspection SuspiciousMethodCalls
+ if (!hiddenSysProps.contains(k)) {
+ map.putNoEx(String.valueOf(k), v);
+ }
+ }));
+ metricManager.registerGauge(null, registryName, sysprops, metricTag, ResolutionStrategy.IGNORE, "properties", "system");
+ MetricsMap sysenv = new MetricsMap(map -> System.getenv().forEach((k, v) -> {
+ if (!hiddenSysProps.contains(k)) {
+ map.putNoEx(String.valueOf(k), v);
+ }
+ }));
+ metricManager.registerGauge(null, registryName, sysenv, metricTag, ResolutionStrategy.IGNORE, "env", "system");
+ } catch (Exception e) {
+ log.warn("Error registering JVM metrics", e);
+ }
+ }
+
+ public RateLimitManager getRateLimitManager() {
+ return rateLimitManager;
+ }
+
+ @VisibleForTesting
+ void setRateLimitManager(RateLimitManager rateLimitManager) {
+ this.rateLimitManager = rateLimitManager;
+ }
+
+ private static class ContextInitializationKey {
+ private ServletContext ctx;
+ volatile CountDownLatch initializing = new CountDownLatch(1);
+
+ private ContextInitializationKey(ServletContext ctx) {
+ if (ctx == null) {
+ throw new IllegalArgumentException("Context must not be null");
+ }
+ // if one of these is reachable both must be to avoid collection from weak hashmap, so
+ // set an attribute holding this object to ensure we never get collected until the ServletContext
+ // is eligible for collection too.
+ ctx.setAttribute(this.getClass().getName(), this);
+ this.ctx = ctx;
+ }
+
+ public synchronized ServletContext getCtx() {
+ return ctx;
+ }
+
+ public synchronized void setCtx(ServletContext ctx) {
+ this.ctx = ctx;
+ }
+
+ synchronized void makeReady() {
+ this.initializing.countDown();
+ }
+
+ // NOT synchronized :)
+ public void waitForReadyService() throws InterruptedException {
+ initializing.await();
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ ContextInitializationKey that = (ContextInitializationKey) o;
+ return ctx.equals(that.ctx);
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `CoreService$ContextInitializationKey.equals(...)` reads without synchronization from `this.ctx`. Potentially races with write in method `CoreService$ContextInitializationKey.setCtx(...)`.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/servlet/CoreService.java
##########
@@ -0,0 +1,451 @@
+/*
+ * 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.servlet;
+
+import com.codahale.metrics.jvm.ClassLoadingGaugeSet;
+import com.codahale.metrics.jvm.GarbageCollectorMetricSet;
+import com.codahale.metrics.jvm.MemoryUsageGaugeSet;
+import com.codahale.metrics.jvm.ThreadStatesGaugeSet;
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.http.client.HttpClient;
+import org.apache.lucene.util.Version;
+import org.apache.solr.cloud.ZkController;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrInfoBean.Group;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.metrics.AltBufferPoolMetricSet;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.OperatingSystemMetricSet;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricManager.ResolutionStrategy;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.servlet.RateLimitManager.Builder;
+import org.apache.solr.util.StartupLoggingUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
+import javax.naming.NoInitialContextException;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
+import javax.servlet.UnavailableException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.Set;
+import java.util.WeakHashMap;
+import java.util.concurrent.CountDownLatch;
+
+import static org.apache.solr.servlet.SolrCoreUtils.loadNodeConfig;
+import static org.apache.solr.servlet.SolrDispatchFilter.PROPERTIES_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLRHOME_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_INSTALL_DIR_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_LEVEL;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_MUTECONSOLE;
+
+/**
+ * A service that can provide access to solr cores. This allows us to have multiple filters and
+ * servlets that depend on SolrCore and CoreContainer, while still only having one CoreContainer per
+ * instance of solr.
+ */
+public class CoreService implements ServletContextListener {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private final String metricTag = SolrMetricProducer.getUniqueMetricTag(this, null);
+ private CoreContainer cores;
+ private Properties extraProperties;
+ private HttpClient httpClient;
+ private SolrMetricManager metricManager;
+ private RateLimitManager rateLimitManager;
+ private final CountDownLatch init = new CountDownLatch(1);
+ private String registryName;
+
+ // AFAIK the only reason we need this is to support JettySolrRunner for tests. In tests we might have
+ // multiple CoreContainers in the same JVM, but I *think* that doesn't happen in a real server.
+ private static final Map<ContextInitializationKey, ServiceHolder> services =
+ Collections.synchronizedMap(new WeakHashMap<>());
+
+ // todo: dependency injection instead, but CDI is not jetty native so for now this method and the associated
+ // map will have to suffice. Note that this relies on ServletContext.equals() not implementing anything significantly
+ // different thant Object.equals for its .equals method (I've found no implementation that even immplements it).
+ public static ServiceHolder serviceForContext(ServletContext ctx) throws InterruptedException {
+ ContextInitializationKey key = new ContextInitializationKey(ctx);
+ return services.computeIfAbsent(key, ServiceHolder::new);
+ }
+
+ @Override
+ public void contextInitialized(ServletContextEvent sce) {
+ init(sce.getServletContext());
+ }
+
+ @Override
+ public void contextDestroyed(ServletContextEvent sce) {
+ close();
+ }
+
+ CoreContainer getCoreContainer() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return cores;
+ }
+ HttpClient getHttpClient() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return httpClient;
+ }
+
+ private static void waitForCoreContainer(CoreContainerProvider provider, CountDownLatch latch) throws UnavailableException {
+ CoreContainer cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ try {
+ latch.await();
+ } catch (InterruptedException e) { //well, no wait then
+ }
+ cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ final String msg = "Error processing the request. CoreContainer is either not initialized or shutting down.";
+ log.error(msg);
+ throw new UnavailableException(msg);
+ }
+ }
+ }
+ public void close() {
+ CoreContainer cc = cores;
+ cores = null;
+ try {
+ if (metricManager != null) {
+ try {
+ metricManager.unregisterGauges(registryName, metricTag);
+ } catch (NullPointerException e) {
+ // okay
+ } catch (Exception e) {
+ log.warn("Exception closing FileCleaningTracker", e);
+ } finally {
+ metricManager = null;
+ }
+ }
+ } finally {
+ if (cc != null) {
+ httpClient = null;
+ cc.shutdown();
+ }
+ }
+ }
+
+ public void init(ServletContext servletContext) {
+ if (log.isTraceEnabled()) {
+ log.trace("CoreService.init(): {}", this.getClass().getClassLoader());
+ }
+ CoreContainer coresInit = null;
+ try {
+ // "extra" properties must be init'ed first so we know things like "do we have a zkHost"
+ // wrap as defaults (if set) so we can modify w/o polluting the Properties provided by our caller
+ this.extraProperties = SolrXmlConfig.wrapAndSetZkHostFromSysPropIfNeeded
+ ((Properties) servletContext.getAttribute(PROPERTIES_ATTRIBUTE));
+
+ StartupLoggingUtils.checkLogDir();
+ if (log.isInfoEnabled()) {
+ log.info("Using logger factory {}", StartupLoggingUtils.getLoggerImplStr());
+ }
+
+ logWelcomeBanner();
+
+ String muteConsole = System.getProperty(SOLR_LOG_MUTECONSOLE);
+ if (muteConsole != null && !Arrays.asList("false","0","off","no").contains(muteConsole.toLowerCase(Locale.ROOT))) {
+ StartupLoggingUtils.muteConsole();
+ }
+ String logLevel = System.getProperty(SOLR_LOG_LEVEL);
+ if (logLevel != null) {
+ log.info("Log level override, property solr.log.level={}", logLevel);
+ StartupLoggingUtils.changeLogLevel(logLevel);
+ }
+
+ coresInit = createCoreContainer(computeSolrHome(servletContext), extraProperties);
+ this.httpClient = coresInit.getUpdateShardHandler().getDefaultHttpClient();
+ setupJvmMetrics(coresInit);
+
+ SolrZkClient zkClient = null;
+ ZkController zkController = coresInit.getZkController();
+
+ if (zkController != null) {
+ zkClient = zkController.getZkClient();
+ }
+
+ Builder builder = new Builder(zkClient);
+
+ this.rateLimitManager = builder.build();
+
+ if (zkController != null) {
+ zkController.zkStateReader.registerClusterPropertiesListener(this.rateLimitManager);
+ }
+
+ if (log.isDebugEnabled()) {
+ log.debug("user.dir={}", System.getProperty("user.dir"));
+ }
+ } catch( Throwable t ) {
+ // catch this so our filter still works
+ log.error( "Could not start Solr. Check solr/home property and the logs");
+ SolrCore.log( t );
+ if (t instanceof Error) {
+ throw (Error) t;
+ }
+ } finally{
+ log.trace("SolrDispatchFilter.init() done");
+ this.cores = coresInit; // crucially final assignment
+ services.computeIfAbsent(new ContextInitializationKey(servletContext), ServiceHolder::new)
+ .setService(this);
+ init.countDown();
+ }
+ }
+
+
+ private void logWelcomeBanner() {
+ // _Really_ sorry about how clumsy this is as a result of the logging call checker, but this is the only one
+ // that's so ugly so far.
+ if (log.isInfoEnabled()) {
+ log.info(" ___ _ Welcome to Apache Solr™ version {}", solrVersion());
+ }
+ if (log.isInfoEnabled()) {
+ log.info("/ __| ___| |_ _ Starting in {} mode on port {}", isCloudMode() ? "cloud" : "standalone", getSolrPort());
+ }
+ if (log.isInfoEnabled()) {
+ log.info("\\__ \\/ _ \\ | '_| Install dir: {}", System.getProperty(SOLR_INSTALL_DIR_ATTRIBUTE));
+ }
+ if (log.isInfoEnabled()) {
+ log.info("|___/\\___/_|_| Start time: {}", Instant.now());
+ }
+ }
+ private String solrVersion() {
+ String specVer = Version.LATEST.toString();
+ try {
+ String implVer = SolrCore.class.getPackage().getImplementationVersion();
+ return (specVer.equals(implVer.split(" ")[0])) ? specVer : implVer;
+ } catch (Exception e) {
+ return specVer;
+ }
+ }
+
+ private String getSolrPort() {
+ return System.getProperty("jetty.port");
+ }
+
+ /**
+ * We are in cloud mode if Java option zkRun exists OR zkHost exists and is non-empty
+ * @see SolrXmlConfig#wrapAndSetZkHostFromSysPropIfNeeded
+ * @see #extraProperties
+ * @see #init
+ */
+ private boolean isCloudMode() {
+ assert null != extraProperties; // we should never be called w/o this being initialized
+ return (null != extraProperties.getProperty(SolrXmlConfig.ZK_HOST)) || (null != System.getProperty("zkRun"));
+ }
+
+ /**
+ * Returns the effective Solr Home to use for this node, based on looking up the value in this order:
+ * <ol>
+ * <li>attribute in the FilterConfig</li>
+ * <li>JNDI: via java:comp/env/solr/home</li>
+ * <li>The system property solr.solr.home</li>
+ * <li>Look in the current working directory for a solr/ directory</li>
+ * </ol>
+ * <p>
+ *
+ * @return the Solr home, absolute and normalized.
+ */
+ private static Path computeSolrHome(ServletContext servletContext) {
+
+ // start with explicit check of servlet config...
+ String source = "servlet config: " + SOLRHOME_ATTRIBUTE;
+ String home = (String) servletContext.getAttribute(SOLRHOME_ATTRIBUTE);
+
+ if (null == home) {
+ final String lookup = "java:comp/env/solr/home";
+ // Try JNDI
+ source = "JNDI: " + lookup;
+ try {
+ Context c = new InitialContext();
+ home = (String) c.lookup(lookup);
+ } catch (NoInitialContextException e) {
+ log.debug("JNDI not configured for solr (NoInitialContextEx)");
+ } catch (NamingException e) {
+ log.debug("No /solr/home in JNDI");
+ } catch (RuntimeException ex) {
+ log.warn("Odd RuntimeException while testing for JNDI: ", ex);
+ }
+ }
+
+ if (null == home) {
+ // Now try system property
+ final String prop = "solr.solr.home";
+ source = "system property: " + prop;
+ home = System.getProperty(prop);
+ }
+
+ if (null == home) {
+ // if all else fails, assume default dir
+ home = "solr/";
+ source = "defaulted to '" + home + "' ... could not find system property or JNDI";
+ }
+ final Path solrHome = Paths.get(home).toAbsolutePath().normalize();
+ log.info("Solr Home: {} (source: {})", solrHome, source);
+
+ return solrHome;
+ }
+
+ /**
+ * Override this to change CoreContainer initialization
+ * @return a CoreContainer to hold this server's cores
+ */
+ protected CoreContainer createCoreContainer(Path solrHome, Properties nodeProps) {
+ NodeConfig nodeConfig = loadNodeConfig(solrHome, nodeProps);
+ final CoreContainer coreContainer = new CoreContainer(nodeConfig, true);
+ coreContainer.load();
+ return coreContainer;
+ }
+
+
+
+ private void setupJvmMetrics(CoreContainer coresInit) {
+ metricManager = coresInit.getMetricManager();
+ registryName = SolrMetricManager.getRegistryName(Group.jvm);
+ final Set<String> hiddenSysProps = coresInit.getConfig().getMetricsConfig().getHiddenSysProps();
+ try {
+ metricManager.registerAll(registryName, new AltBufferPoolMetricSet(), ResolutionStrategy.IGNORE, "buffers");
+ metricManager.registerAll(registryName, new ClassLoadingGaugeSet(), ResolutionStrategy.IGNORE, "classes");
+ metricManager.registerAll(registryName, new OperatingSystemMetricSet(), ResolutionStrategy.IGNORE, "os");
+ metricManager.registerAll(registryName, new GarbageCollectorMetricSet(), ResolutionStrategy.IGNORE, "gc");
+ metricManager.registerAll(registryName, new MemoryUsageGaugeSet(), ResolutionStrategy.IGNORE, "memory");
+ metricManager.registerAll(registryName, new ThreadStatesGaugeSet(), ResolutionStrategy.IGNORE, "threads"); // todo should we use CachedThreadStatesGaugeSet instead?
+ MetricsMap sysprops = new MetricsMap(map -> System.getProperties().forEach((k, v) -> {
+ //noinspection SuspiciousMethodCalls
+ if (!hiddenSysProps.contains(k)) {
+ map.putNoEx(String.valueOf(k), v);
+ }
+ }));
+ metricManager.registerGauge(null, registryName, sysprops, metricTag, ResolutionStrategy.IGNORE, "properties", "system");
+ MetricsMap sysenv = new MetricsMap(map -> System.getenv().forEach((k, v) -> {
+ if (!hiddenSysProps.contains(k)) {
+ map.putNoEx(String.valueOf(k), v);
+ }
+ }));
+ metricManager.registerGauge(null, registryName, sysenv, metricTag, ResolutionStrategy.IGNORE, "env", "system");
+ } catch (Exception e) {
+ log.warn("Error registering JVM metrics", e);
+ }
+ }
+
+ public RateLimitManager getRateLimitManager() {
+ return rateLimitManager;
+ }
+
+ @VisibleForTesting
+ void setRateLimitManager(RateLimitManager rateLimitManager) {
+ this.rateLimitManager = rateLimitManager;
+ }
+
+ private static class ContextInitializationKey {
+ private ServletContext ctx;
+ volatile CountDownLatch initializing = new CountDownLatch(1);
+
+ private ContextInitializationKey(ServletContext ctx) {
+ if (ctx == null) {
+ throw new IllegalArgumentException("Context must not be null");
+ }
+ // if one of these is reachable both must be to avoid collection from weak hashmap, so
+ // set an attribute holding this object to ensure we never get collected until the ServletContext
+ // is eligible for collection too.
+ ctx.setAttribute(this.getClass().getName(), this);
+ this.ctx = ctx;
+ }
+
+ public synchronized ServletContext getCtx() {
+ return ctx;
+ }
+
+ public synchronized void setCtx(ServletContext ctx) {
+ this.ctx = ctx;
+ }
+
+ synchronized void makeReady() {
+ this.initializing.countDown();
+ }
+
+ // NOT synchronized :)
+ public void waitForReadyService() throws InterruptedException {
+ initializing.await();
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ ContextInitializationKey that = (ContextInitializationKey) o;
+ return ctx.equals(that.ctx);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(ctx);
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `CoreService$ContextInitializationKey.hashCode()` reads without synchronization from `this.ctx`. Potentially races with write in method `CoreService$ContextInitializationKey.setCtx(...)`.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gus-asf commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
gus-asf commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r694832411
##########
File path: solr/core/src/java/org/apache/solr/servlet/ExceptionWhileTracing.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.servlet;
+
+public class ExceptionWhileTracing extends RuntimeException {
Review comment:
It's a choice between a custom exception to carry exceptions out of the runnable passed into traceHttpRequestExecution2(), or a custom interface to replace the runnable that throws a whole bunch of exceptions we want to distinguish...
(yes that method name should loose its 2 which is a left over from a refactor ).
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] madrob commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r695204142
##########
File path: solr/core/src/java/org/apache/solr/servlet/CoreService.java
##########
@@ -0,0 +1,451 @@
+/*
+ * 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.servlet;
+
+import com.codahale.metrics.jvm.ClassLoadingGaugeSet;
+import com.codahale.metrics.jvm.GarbageCollectorMetricSet;
+import com.codahale.metrics.jvm.MemoryUsageGaugeSet;
+import com.codahale.metrics.jvm.ThreadStatesGaugeSet;
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.http.client.HttpClient;
+import org.apache.lucene.util.Version;
+import org.apache.solr.cloud.ZkController;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrInfoBean.Group;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.metrics.AltBufferPoolMetricSet;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.OperatingSystemMetricSet;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricManager.ResolutionStrategy;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.servlet.RateLimitManager.Builder;
+import org.apache.solr.util.StartupLoggingUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
+import javax.naming.NoInitialContextException;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
+import javax.servlet.UnavailableException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.Set;
+import java.util.WeakHashMap;
+import java.util.concurrent.CountDownLatch;
+
+import static org.apache.solr.servlet.SolrCoreUtils.loadNodeConfig;
+import static org.apache.solr.servlet.SolrDispatchFilter.PROPERTIES_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLRHOME_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_INSTALL_DIR_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_LEVEL;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_MUTECONSOLE;
+
+/**
+ * A service that can provide access to solr cores. This allows us to have multiple filters and
+ * servlets that depend on SolrCore and CoreContainer, while still only having one CoreContainer per
+ * instance of solr.
+ */
+public class CoreService implements ServletContextListener {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private final String metricTag = SolrMetricProducer.getUniqueMetricTag(this, null);
+ private CoreContainer cores;
+ private Properties extraProperties;
+ private HttpClient httpClient;
+ private SolrMetricManager metricManager;
+ private RateLimitManager rateLimitManager;
+ private final CountDownLatch init = new CountDownLatch(1);
+ private String registryName;
+
+ // AFAIK the only reason we need this is to support JettySolrRunner for tests. In tests we might have
+ // multiple CoreContainers in the same JVM, but I *think* that doesn't happen in a real server.
+ private static final Map<ContextInitializationKey, ServiceHolder> services =
+ Collections.synchronizedMap(new WeakHashMap<>());
+
+ // todo: dependency injection instead, but CDI is not jetty native so for now this method and the associated
+ // map will have to suffice. Note that this relies on ServletContext.equals() not implementing anything significantly
+ // different thant Object.equals for its .equals method (I've found no implementation that even immplements it).
+ public static ServiceHolder serviceForContext(ServletContext ctx) throws InterruptedException {
+ ContextInitializationKey key = new ContextInitializationKey(ctx);
+ return services.computeIfAbsent(key, ServiceHolder::new);
+ }
+
+ @Override
+ public void contextInitialized(ServletContextEvent sce) {
+ init(sce.getServletContext());
+ }
+
+ @Override
+ public void contextDestroyed(ServletContextEvent sce) {
+ close();
+ }
+
+ CoreContainer getCoreContainer() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return cores;
+ }
+ HttpClient getHttpClient() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return httpClient;
+ }
+
+ private static void waitForCoreContainer(CoreContainerProvider provider, CountDownLatch latch) throws UnavailableException {
+ CoreContainer cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ try {
+ latch.await();
+ } catch (InterruptedException e) { //well, no wait then
Review comment:
reset interrupted status?
##########
File path: solr/core/src/java/org/apache/solr/servlet/CoreService.java
##########
@@ -0,0 +1,451 @@
+/*
+ * 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.servlet;
+
+import com.codahale.metrics.jvm.ClassLoadingGaugeSet;
+import com.codahale.metrics.jvm.GarbageCollectorMetricSet;
+import com.codahale.metrics.jvm.MemoryUsageGaugeSet;
+import com.codahale.metrics.jvm.ThreadStatesGaugeSet;
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.http.client.HttpClient;
+import org.apache.lucene.util.Version;
+import org.apache.solr.cloud.ZkController;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrInfoBean.Group;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.metrics.AltBufferPoolMetricSet;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.OperatingSystemMetricSet;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricManager.ResolutionStrategy;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.servlet.RateLimitManager.Builder;
+import org.apache.solr.util.StartupLoggingUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
+import javax.naming.NoInitialContextException;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
+import javax.servlet.UnavailableException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.Set;
+import java.util.WeakHashMap;
+import java.util.concurrent.CountDownLatch;
+
+import static org.apache.solr.servlet.SolrCoreUtils.loadNodeConfig;
+import static org.apache.solr.servlet.SolrDispatchFilter.PROPERTIES_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLRHOME_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_INSTALL_DIR_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_LEVEL;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_MUTECONSOLE;
+
+/**
+ * A service that can provide access to solr cores. This allows us to have multiple filters and
+ * servlets that depend on SolrCore and CoreContainer, while still only having one CoreContainer per
+ * instance of solr.
+ */
+public class CoreService implements ServletContextListener {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private final String metricTag = SolrMetricProducer.getUniqueMetricTag(this, null);
+ private CoreContainer cores;
+ private Properties extraProperties;
+ private HttpClient httpClient;
+ private SolrMetricManager metricManager;
+ private RateLimitManager rateLimitManager;
+ private final CountDownLatch init = new CountDownLatch(1);
+ private String registryName;
+
+ // AFAIK the only reason we need this is to support JettySolrRunner for tests. In tests we might have
+ // multiple CoreContainers in the same JVM, but I *think* that doesn't happen in a real server.
+ private static final Map<ContextInitializationKey, ServiceHolder> services =
+ Collections.synchronizedMap(new WeakHashMap<>());
+
+ // todo: dependency injection instead, but CDI is not jetty native so for now this method and the associated
+ // map will have to suffice. Note that this relies on ServletContext.equals() not implementing anything significantly
+ // different thant Object.equals for its .equals method (I've found no implementation that even immplements it).
+ public static ServiceHolder serviceForContext(ServletContext ctx) throws InterruptedException {
+ ContextInitializationKey key = new ContextInitializationKey(ctx);
+ return services.computeIfAbsent(key, ServiceHolder::new);
+ }
+
+ @Override
+ public void contextInitialized(ServletContextEvent sce) {
+ init(sce.getServletContext());
+ }
+
+ @Override
+ public void contextDestroyed(ServletContextEvent sce) {
+ close();
+ }
+
+ CoreContainer getCoreContainer() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return cores;
+ }
+ HttpClient getHttpClient() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return httpClient;
+ }
+
+ private static void waitForCoreContainer(CoreContainerProvider provider, CountDownLatch latch) throws UnavailableException {
+ CoreContainer cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ try {
+ latch.await();
Review comment:
with timeout?
##########
File path: solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java
##########
@@ -0,0 +1,23 @@
+/*
+ * 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.servlet;
+
+import org.apache.solr.core.CoreContainer;
+
+public interface CoreContainerProvider {
+ CoreContainer getCores();
Review comment:
Javadoc please!
##########
File path: solr/core/src/java/org/apache/solr/servlet/PathExcluder.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.servlet;
+
+import java.util.ArrayList;
+import java.util.regex.Pattern;
+
+public interface PathExcluder {
+ void setExcludePatterns(ArrayList<Pattern> excludePatterns);
Review comment:
Why ArrayList not List?
##########
File path: solr/core/src/java/org/apache/solr/servlet/CoreService.java
##########
@@ -0,0 +1,451 @@
+/*
+ * 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.servlet;
+
+import com.codahale.metrics.jvm.ClassLoadingGaugeSet;
+import com.codahale.metrics.jvm.GarbageCollectorMetricSet;
+import com.codahale.metrics.jvm.MemoryUsageGaugeSet;
+import com.codahale.metrics.jvm.ThreadStatesGaugeSet;
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.http.client.HttpClient;
+import org.apache.lucene.util.Version;
+import org.apache.solr.cloud.ZkController;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrInfoBean.Group;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.metrics.AltBufferPoolMetricSet;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.OperatingSystemMetricSet;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricManager.ResolutionStrategy;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.servlet.RateLimitManager.Builder;
+import org.apache.solr.util.StartupLoggingUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
+import javax.naming.NoInitialContextException;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
+import javax.servlet.UnavailableException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.Set;
+import java.util.WeakHashMap;
+import java.util.concurrent.CountDownLatch;
+
+import static org.apache.solr.servlet.SolrCoreUtils.loadNodeConfig;
+import static org.apache.solr.servlet.SolrDispatchFilter.PROPERTIES_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLRHOME_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_INSTALL_DIR_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_LEVEL;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_MUTECONSOLE;
+
+/**
+ * A service that can provide access to solr cores. This allows us to have multiple filters and
+ * servlets that depend on SolrCore and CoreContainer, while still only having one CoreContainer per
+ * instance of solr.
+ */
+public class CoreService implements ServletContextListener {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private final String metricTag = SolrMetricProducer.getUniqueMetricTag(this, null);
+ private CoreContainer cores;
+ private Properties extraProperties;
+ private HttpClient httpClient;
+ private SolrMetricManager metricManager;
+ private RateLimitManager rateLimitManager;
+ private final CountDownLatch init = new CountDownLatch(1);
+ private String registryName;
+
+ // AFAIK the only reason we need this is to support JettySolrRunner for tests. In tests we might have
+ // multiple CoreContainers in the same JVM, but I *think* that doesn't happen in a real server.
+ private static final Map<ContextInitializationKey, ServiceHolder> services =
+ Collections.synchronizedMap(new WeakHashMap<>());
+
+ // todo: dependency injection instead, but CDI is not jetty native so for now this method and the associated
+ // map will have to suffice. Note that this relies on ServletContext.equals() not implementing anything significantly
+ // different thant Object.equals for its .equals method (I've found no implementation that even immplements it).
+ public static ServiceHolder serviceForContext(ServletContext ctx) throws InterruptedException {
+ ContextInitializationKey key = new ContextInitializationKey(ctx);
+ return services.computeIfAbsent(key, ServiceHolder::new);
+ }
+
+ @Override
+ public void contextInitialized(ServletContextEvent sce) {
+ init(sce.getServletContext());
+ }
+
+ @Override
+ public void contextDestroyed(ServletContextEvent sce) {
+ close();
+ }
+
+ CoreContainer getCoreContainer() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return cores;
+ }
+ HttpClient getHttpClient() throws UnavailableException {
+ waitForCoreContainer(() -> cores,init);
+ return httpClient;
+ }
+
+ private static void waitForCoreContainer(CoreContainerProvider provider, CountDownLatch latch) throws UnavailableException {
+ CoreContainer cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ try {
+ latch.await();
+ } catch (InterruptedException e) { //well, no wait then
+ }
+ cores = provider.getCores();
+ if (cores == null || cores.isShutDown()) {
+ final String msg = "Error processing the request. CoreContainer is either not initialized or shutting down.";
+ log.error(msg);
+ throw new UnavailableException(msg);
Review comment:
log-and-throw is not great, since we will typically log wherever we catch this bubbling up
##########
File path: solr/test-framework/src/java/org/apache/solr/util/BaseTestHarness.java
##########
@@ -77,14 +77,21 @@ public static String validateXPath(String xml, String... tests)
if (tests==null || tests.length == 0) return null;
- Document document = null;
+ Document document;
try {
+// if (xml.startsWith("<html>")) {
Review comment:
I'm not sure what's going on here, or if we need a replacement?
##########
File path: solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java
##########
@@ -527,7 +536,7 @@ public void start(boolean reusePort) throws Exception {
}
synchronized (JettySolrRunner.this) {
int cnt = 0;
- while (!waitOnSolr || !dispatchFilter.isRunning() || getCoreContainer() == null) {
+ while (!waitOnSolr || !dispatchFilter.isRunning() ) {
Review comment:
why do we no longer need the null check? this is because it would always throw otherwise, right?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gus-asf commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
gus-asf commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r694798127
##########
File path: solr/core/src/java/org/apache/solr/servlet/CoreService.java
##########
@@ -0,0 +1,451 @@
+/*
+ * 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.servlet;
+
+import com.codahale.metrics.jvm.ClassLoadingGaugeSet;
+import com.codahale.metrics.jvm.GarbageCollectorMetricSet;
+import com.codahale.metrics.jvm.MemoryUsageGaugeSet;
+import com.codahale.metrics.jvm.ThreadStatesGaugeSet;
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.http.client.HttpClient;
+import org.apache.lucene.util.Version;
+import org.apache.solr.cloud.ZkController;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrInfoBean.Group;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.metrics.AltBufferPoolMetricSet;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.OperatingSystemMetricSet;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricManager.ResolutionStrategy;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.servlet.RateLimitManager.Builder;
+import org.apache.solr.util.StartupLoggingUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
+import javax.naming.NoInitialContextException;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
+import javax.servlet.UnavailableException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.Set;
+import java.util.WeakHashMap;
+import java.util.concurrent.CountDownLatch;
+
+import static org.apache.solr.servlet.SolrCoreUtils.loadNodeConfig;
+import static org.apache.solr.servlet.SolrDispatchFilter.PROPERTIES_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLRHOME_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_INSTALL_DIR_ATTRIBUTE;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_LEVEL;
+import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_MUTECONSOLE;
+
+/**
+ * A service that can provide access to solr cores. This allows us to have multiple filters and
+ * servlets that depend on SolrCore and CoreContainer, while still only having one CoreContainer per
+ * instance of solr.
+ */
+public class CoreService implements ServletContextListener {
Review comment:
I almost named it that but waffled for a slightly shorter name. :) no problem will change.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gus-asf commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
gus-asf commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r694794497
##########
File path: solr/core/src/java/org/apache/solr/servlet/SolrCoreUtils.java
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.servlet;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrXmlConfig;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.ByteArrayInputStream;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.util.Properties;
+
+public class SolrCoreUtils {
Review comment:
Good idea.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gus-asf commented on a change in pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
gus-asf commented on a change in pull request #265:
URL: https://github.com/apache/solr/pull/265#discussion_r694797222
##########
File path: solr/core/src/java/org/apache/solr/servlet/AdminServlet.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.servlet;
+
+import javax.servlet.ServletException;
+import javax.servlet.annotation.WebServlet;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.regex.Pattern;
+
+import static org.apache.solr.servlet.ServletUtils.closeShield;
+import static org.apache.solr.servlet.ServletUtils.configExcludes;
+import static org.apache.solr.servlet.ServletUtils.excludedPath;
+
+@WebServlet
+public class AdminServlet extends HttpServlet implements PathExcluder{
+ private ArrayList<Pattern> excludes;
+
+ @Override
+ public void init() throws ServletException {
+ configExcludes(this, getInitParameter("excludePatterns"));
+ }
+
+ @Override
+ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
+ req = closeShield(req, false);
+ resp = closeShield(resp, false);
+ if (excludedPath(excludes,req,resp,null)) {
+ return;
+ }
+ ServletUtils.rateLimitRequest(req,resp,() -> {
Review comment:
Probably not. This class as noted in the linked issue, is just a foil for development at the moment, and need not be included at all in the final version of *this* commit at all. As I was working the specific goal was exact parity, so that I could demonstrate solutions to problems caused by breaking things out. While rate limit probably isn't needed here we would want it if update and query each had their own paths some day, so this just demonstrates that I have a way to apply it to *some* servlet.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gus-asf commented on pull request #265: SOLR-15590 - Startup and CoreContainer managed by Core Service
Posted by GitBox <gi...@apache.org>.
gus-asf commented on pull request #265:
URL: https://github.com/apache/solr/pull/265#issuecomment-962658431
continued with https://github.com/apache/solr/pull/397
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org