From 40e54dd2101ca47a5e91e4d378b209e745d2e364 Mon Sep 17 00:00:00 2001 From: Gus Heck Date: Tue, 10 Aug 2021 21:34:16 -0400 Subject: [PATCH 1/9] Make an admin servlet that does nothing, migrate close shielding to utils --- .../org/apache/solr/servlet/AdminServlet.java | 25 +++ .../solr/servlet/LoadAdminUiServlet.java | 4 +- .../org/apache/solr/servlet/ServletUtils.java | 135 ++++++++++++ .../solr/servlet/SolrDispatchFilter.java | 193 +++--------------- 4 files changed, 193 insertions(+), 164 deletions(-) create mode 100644 solr/core/src/java/org/apache/solr/servlet/AdminServlet.java diff --git a/solr/core/src/java/org/apache/solr/servlet/AdminServlet.java b/solr/core/src/java/org/apache/solr/servlet/AdminServlet.java new file mode 100644 index 00000000000..01bbc0c34fc --- /dev/null +++ b/solr/core/src/java/org/apache/solr/servlet/AdminServlet.java @@ -0,0 +1,25 @@ +package org.apache.solr.servlet; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; + +import static org.apache.solr.servlet.ServletUtils.closeShield; + +public class AdminServlet extends HttpServlet { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + req = closeShield(req, false); + resp = closeShield(resp, false); + super.doGet(req, resp); + } + + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + req = closeShield(req, false); + resp = closeShield(resp, false); + super.doPost(req, resp); + } +} diff --git a/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java b/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java index 54d592435d6..42103f2f4b1 100644 --- a/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java +++ b/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java @@ -48,8 +48,8 @@ public void doGet(HttpServletRequest _request, HttpServletResponse _response) th "ENABLED in bin/solr.in.sh or solr.in.cmd."); return; } - HttpServletRequest request = SolrDispatchFilter.closeShield(_request, false); - HttpServletResponse response = SolrDispatchFilter.closeShield(_response, false); + HttpServletRequest request = ServletUtils.closeShield(_request, false); + HttpServletResponse response = ServletUtils.closeShield(_response, false); response.addHeader("X-Frame-Options", "DENY"); // security: SOLR-7966 - avoid clickjacking for admin interface diff --git a/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java b/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java index e61831ac4e7..bf90197337c 100644 --- a/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java +++ b/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java @@ -17,12 +17,28 @@ package org.apache.solr.servlet; +import javax.servlet.ReadListener; +import javax.servlet.ServletInputStream; +import javax.servlet.ServletOutputStream; +import javax.servlet.WriteListener; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletRequestWrapper; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpServletResponseWrapper; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; /** * Various Util methods for interaction on servlet level, i.e. HttpServletRequest */ public abstract class ServletUtils { + static String CLOSE_STREAM_MSG = "Attempted close of http request or response stream - in general you should not do this, " + + "you may spoil connection reuse and possibly disrupt a client. If you must close without actually needing to close, " + + "use a CloseShield*Stream. Closing or flushing the response stream commits the response and prevents us from modifying it. " + + "Closing the request stream prevents us from guaranteeing ourselves that streams are fully read for proper connection reuse." + + "Let the container manage the lifecycle of these streams when possible."; + private ServletUtils() { /* only static methods in this class */ } /** @@ -34,4 +50,123 @@ private ServletUtils() { /* only static methods in this class */ } public static String getPathAfterContext(HttpServletRequest request) { return request.getServletPath() + (request.getPathInfo() != null ? request.getPathInfo() : ""); } + + /** + * Wrap the request's input stream with a close shield. If this is a + * retry, we will assume that the stream has already been wrapped and do nothing. + * + * Only the container should ever actually close the servlet output stream. + * + * @param request The request to wrap. + * @param retry If this is an original request or a retry. + * @return A request object with an {@link InputStream} that will ignore calls to close. + */ + public static HttpServletRequest closeShield(HttpServletRequest request, boolean retry) { + if (!retry) { + return new HttpServletRequestWrapper(request) { + + @Override + public ServletInputStream getInputStream() throws IOException { + + return new ServletInputStreamWrapper(super.getInputStream()) { + @Override + public void close() { + // even though we skip closes, we let local tests know not to close so that a full understanding can take + // place + assert Thread.currentThread().getStackTrace()[2].getClassName().matches( + "org\\.apache\\.(?:solr|lucene).*") ? false : true : CLOSE_STREAM_MSG; + this.stream = ClosedServletInputStream.CLOSED_SERVLET_INPUT_STREAM; + } + }; + + } + }; + } else { + return request; + } + } + + /** + * Wrap the response's output stream with a close shield. If this is a + * retry, we will assume that the stream has already been wrapped and do nothing. + * + * Only the container should ever actually close the servlet request stream. + * + * @param response The response to wrap. + * @param retry If this response corresponds to an original request or a retry. + * @return A response object with an {@link OutputStream} that will ignore calls to close. + */ + public static HttpServletResponse closeShield(HttpServletResponse response, boolean retry) { + if (!retry) { + return new HttpServletResponseWrapper(response) { + + @Override + public ServletOutputStream getOutputStream() throws IOException { + + return new ServletOutputStreamWrapper(super.getOutputStream()) { + @Override + public void close() { + // even though we skip closes, we let local tests know not to close so that a full understanding can take + // place + assert Thread.currentThread().getStackTrace()[2].getClassName().matches( + "org\\.apache\\.(?:solr|lucene).*") ? false + : true : CLOSE_STREAM_MSG; + stream = ClosedServletOutputStream.CLOSED_SERVLET_OUTPUT_STREAM; + } + }; + } + + }; + } else { + return response; + } + } + + public static class ClosedServletInputStream extends ServletInputStream { + + public static final ClosedServletInputStream CLOSED_SERVLET_INPUT_STREAM = new ClosedServletInputStream(); + + @Override + public int read() { + return -1; + } + + @Override + public boolean isFinished() { + return false; + } + + @Override + public boolean isReady() { + return false; + } + + @Override + public void setReadListener(ReadListener arg0) {} + } + + public static class ClosedServletOutputStream extends ServletOutputStream { + + public static final ClosedServletOutputStream CLOSED_SERVLET_OUTPUT_STREAM = new ClosedServletOutputStream(); + + @Override + public void write(final int b) throws IOException { + throw new IOException("write(" + b + ") failed: stream is closed"); + } + + @Override + public void flush() throws IOException { + throw new IOException("flush() failed: stream is closed"); + } + + @Override + public boolean isReady() { + return false; + } + + @Override + public void setWriteListener(WriteListener arg0) { + throw new RuntimeException("setWriteListener() failed: stream is closed"); + } + } } diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java index 28167270099..da34b389b9f 100644 --- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java +++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java @@ -16,43 +16,6 @@ */ package org.apache.solr.servlet; -import javax.naming.Context; -import javax.naming.InitialContext; -import javax.naming.NamingException; -import javax.naming.NoInitialContextException; -import javax.servlet.FilterChain; -import javax.servlet.FilterConfig; -import javax.servlet.ReadListener; -import javax.servlet.ServletException; -import javax.servlet.ServletInputStream; -import javax.servlet.ServletOutputStream; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import javax.servlet.UnavailableException; -import javax.servlet.WriteListener; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletRequestWrapper; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpServletResponseWrapper; -import java.io.ByteArrayInputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.lang.invoke.MethodHandles; -import java.nio.file.Path; -import java.nio.file.Paths; -import java.time.Instant; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Locale; -import java.util.Properties; -import java.util.Set; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicReference; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - import com.codahale.metrics.jvm.ClassLoadingGaugeSet; import com.codahale.metrics.jvm.GarbageCollectorMetricSet; import com.codahale.metrics.jvm.MemoryUsageGaugeSet; @@ -96,7 +59,38 @@ 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.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletInputStream; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.UnavailableException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.lang.invoke.MethodHandles; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.time.Instant; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Locale; +import java.util.Properties; +import java.util.Set; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + import static org.apache.solr.security.AuditEvent.EventType; +import static org.apache.solr.servlet.ServletUtils.closeShield; /** * This filter looks at the incoming URL maps them to handlers defined in solrconfig.xml @@ -642,61 +636,7 @@ private boolean authenticateRequest(HttpServletRequest request, HttpServletRespo } return true; } - - public static class ClosedServletInputStream extends ServletInputStream { - - public static final ClosedServletInputStream CLOSED_SERVLET_INPUT_STREAM = new ClosedServletInputStream(); - - @Override - public int read() { - return -1; - } - - @Override - public boolean isFinished() { - return false; - } - - @Override - public boolean isReady() { - return false; - } - - @Override - public void setReadListener(ReadListener arg0) {} - } - - public static class ClosedServletOutputStream extends ServletOutputStream { - - public static final ClosedServletOutputStream CLOSED_SERVLET_OUTPUT_STREAM = new ClosedServletOutputStream(); - - @Override - public void write(final int b) throws IOException { - throw new IOException("write(" + b + ") failed: stream is closed"); - } - - @Override - public void flush() throws IOException { - throw new IOException("flush() failed: stream is closed"); - } - - @Override - public boolean isReady() { - return false; - } - - @Override - public void setWriteListener(WriteListener arg0) { - throw new RuntimeException("setWriteListener() failed: stream is closed"); - } - } - private static String CLOSE_STREAM_MSG = "Attempted close of http request or response stream - in general you should not do this, " - + "you may spoil connection reuse and possibly disrupt a client. If you must close without actually needing to close, " - + "use a CloseShield*Stream. Closing or flushing the response stream commits the response and prevents us from modifying it. " - + "Closing the request stream prevents us from gauranteeing ourselves that streams are fully read for proper connection reuse." - + "Let the container manage the lifecycle of these streams when possible."; - /** * Check if audit logging is enabled and should happen for given event type @@ -705,77 +645,6 @@ public void setWriteListener(WriteListener arg0) { private boolean shouldAudit(AuditEvent.EventType eventType) { return cores.getAuditLoggerPlugin() != null && cores.getAuditLoggerPlugin().shouldLog(eventType); } - - /** - * Wrap the request's input stream with a close shield. If this is a - * retry, we will assume that the stream has already been wrapped and do nothing. - * - * Only the container should ever actually close the servlet output stream. - * - * @param request The request to wrap. - * @param retry If this is an original request or a retry. - * @return A request object with an {@link InputStream} that will ignore calls to close. - */ - public static HttpServletRequest closeShield(HttpServletRequest request, boolean retry) { - if (!retry) { - return new HttpServletRequestWrapper(request) { - - @Override - public ServletInputStream getInputStream() throws IOException { - - return new ServletInputStreamWrapper(super.getInputStream()) { - @Override - public void close() { - // even though we skip closes, we let local tests know not to close so that a full understanding can take - // place - assert Thread.currentThread().getStackTrace()[2].getClassName().matches( - "org\\.apache\\.(?:solr|lucene).*") ? false : true : CLOSE_STREAM_MSG; - this.stream = ClosedServletInputStream.CLOSED_SERVLET_INPUT_STREAM; - } - }; - - } - }; - } else { - return request; - } - } - - /** - * Wrap the response's output stream with a close shield. If this is a - * retry, we will assume that the stream has already been wrapped and do nothing. - * - * Only the container should ever actually close the servlet request stream. - * - * @param response The response to wrap. - * @param retry If this response corresponds to an original request or a retry. - * @return A response object with an {@link OutputStream} that will ignore calls to close. - */ - public static HttpServletResponse closeShield(HttpServletResponse response, boolean retry) { - if (!retry) { - return new HttpServletResponseWrapper(response) { - - @Override - public ServletOutputStream getOutputStream() throws IOException { - - return new ServletOutputStreamWrapper(super.getOutputStream()) { - @Override - public void close() { - // even though we skip closes, we let local tests know not to close so that a full understanding can take - // place - assert Thread.currentThread().getStackTrace()[2].getClassName().matches( - "org\\.apache\\.(?:solr|lucene).*") ? false - : true : CLOSE_STREAM_MSG; - stream = ClosedServletOutputStream.CLOSED_SERVLET_OUTPUT_STREAM; - } - }; - } - - }; - } else { - return response; - } - } public void closeOnDestroy(boolean closeOnDestroy) { this.closeOnDestroy = closeOnDestroy; From ba2b7abd9de9f3af781b891f195d97b007c98c01 Mon Sep 17 00:00:00 2001 From: Gus Heck Date: Tue, 10 Aug 2021 21:57:52 -0400 Subject: [PATCH 2/9] reuse path exclusion --- .../org/apache/solr/servlet/AdminServlet.java | 26 ++++++++++- .../org/apache/solr/servlet/PathExcluder.java | 8 ++++ .../org/apache/solr/servlet/ServletUtils.java | 46 +++++++++++++++++-- .../solr/servlet/SolrDispatchFilter.java | 34 ++++++-------- 4 files changed, 87 insertions(+), 27 deletions(-) create mode 100644 solr/core/src/java/org/apache/solr/servlet/PathExcluder.java diff --git a/solr/core/src/java/org/apache/solr/servlet/AdminServlet.java b/solr/core/src/java/org/apache/solr/servlet/AdminServlet.java index 01bbc0c34fc..b56ac9744ff 100644 --- a/solr/core/src/java/org/apache/solr/servlet/AdminServlet.java +++ b/solr/core/src/java/org/apache/solr/servlet/AdminServlet.java @@ -1,18 +1,34 @@ 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 excludes; + + @Override + public void init() throws ServletException { + configExcludes(this, getInitParameter("excludePatterns")); + } -public class AdminServlet extends HttpServlet { @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; + } super.doGet(req, resp); } @@ -20,6 +36,14 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { req = closeShield(req, false); resp = closeShield(resp, false); + if (excludedPath(excludes,req,resp,null)) { + return; + } super.doPost(req, resp); } + + @Override + public void setExcludePatterns(ArrayList excludePatterns) { + this.excludes = excludePatterns; + } } diff --git a/solr/core/src/java/org/apache/solr/servlet/PathExcluder.java b/solr/core/src/java/org/apache/solr/servlet/PathExcluder.java new file mode 100644 index 00000000000..9ff21211b7f --- /dev/null +++ b/solr/core/src/java/org/apache/solr/servlet/PathExcluder.java @@ -0,0 +1,8 @@ +package org.apache.solr.servlet; + +import java.util.ArrayList; +import java.util.regex.Pattern; + +public interface PathExcluder { + void setExcludePatterns(ArrayList excludePatterns); +} diff --git a/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java b/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java index bf90197337c..7bf6b51dc55 100644 --- a/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java +++ b/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java @@ -17,7 +17,9 @@ package org.apache.solr.servlet; +import javax.servlet.FilterChain; import javax.servlet.ReadListener; +import javax.servlet.ServletException; import javax.servlet.ServletInputStream; import javax.servlet.ServletOutputStream; import javax.servlet.WriteListener; @@ -28,6 +30,9 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.ArrayList; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /** * Various Util methods for interaction on servlet level, i.e. HttpServletRequest @@ -73,8 +78,8 @@ public ServletInputStream getInputStream() throws IOException { public void close() { // even though we skip closes, we let local tests know not to close so that a full understanding can take // place - assert Thread.currentThread().getStackTrace()[2].getClassName().matches( - "org\\.apache\\.(?:solr|lucene).*") ? false : true : CLOSE_STREAM_MSG; + assert !Thread.currentThread().getStackTrace()[2].getClassName().matches( + "org\\.apache\\.(?:solr|lucene).*") : CLOSE_STREAM_MSG; this.stream = ClosedServletInputStream.CLOSED_SERVLET_INPUT_STREAM; } }; @@ -108,9 +113,8 @@ public ServletOutputStream getOutputStream() throws IOException { public void close() { // even though we skip closes, we let local tests know not to close so that a full understanding can take // place - assert Thread.currentThread().getStackTrace()[2].getClassName().matches( - "org\\.apache\\.(?:solr|lucene).*") ? false - : true : CLOSE_STREAM_MSG; + assert !Thread.currentThread().getStackTrace()[2].getClassName().matches( + "org\\.apache\\.(?:solr|lucene).*") : CLOSE_STREAM_MSG; stream = ClosedServletOutputStream.CLOSED_SERVLET_OUTPUT_STREAM; } }; @@ -122,6 +126,38 @@ public void close() { } } + static boolean excludedPath(ArrayList excludePatterns, HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { + String requestPath = 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()) { + if (chain != null) { + chain.doFilter(request, response); + } + return true; + } + } + } + return false; + } + + static boolean excludedPath(ArrayList excludePatterns, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { + return excludedPath(excludePatterns,request,response, null); + } + + static void configExcludes(PathExcluder excluder, String patternConfig) { + if(patternConfig != null) { + String[] excludeArray = patternConfig.split(","); + ArrayList patterns = new ArrayList<>(); + excluder.setExcludePatterns(patterns); + for (String element : excludeArray) { + patterns.add(Pattern.compile(element)); + } + } + } + public static class ClosedServletInputStream extends ServletInputStream { public static final ClosedServletInputStream CLOSED_SERVLET_INPUT_STREAM = new ClosedServletInputStream(); diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java index da34b389b9f..b5c8d9df1d0 100644 --- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java +++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java @@ -86,18 +86,19 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; -import java.util.regex.Matcher; import java.util.regex.Pattern; import static org.apache.solr.security.AuditEvent.EventType; import static org.apache.solr.servlet.ServletUtils.closeShield; +import static org.apache.solr.servlet.ServletUtils.configExcludes; +import static org.apache.solr.servlet.ServletUtils.excludedPath; /** * This filter looks at the incoming URL maps them to handlers defined in solrconfig.xml * * @since solr 1.2 */ -public class SolrDispatchFilter extends BaseSolrFilter { +public class SolrDispatchFilter extends BaseSolrFilter implements PathExcluder { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); protected volatile CoreContainer cores; @@ -106,6 +107,12 @@ public class SolrDispatchFilter extends BaseSolrFilter { protected String abortErrorMessage = null; //TODO using Http2Client protected HttpClient httpClient; + + @Override + public void setExcludePatterns(ArrayList excludePatterns) { + this.excludePatterns = excludePatterns; + } + private ArrayList excludePatterns; private boolean isV2Enabled = !"true".equals(System.getProperty("disable.v2.api", "false")); @@ -174,15 +181,8 @@ public void init(FilterConfig config) throws ServletException { log.info("Log level override, property solr.log.level={}", logLevel); StartupLoggingUtils.changeLogLevel(logLevel); } - - String exclude = config.getInitParameter("excludePatterns"); - if(exclude != null) { - String[] excludeArray = exclude.split(","); - excludePatterns = new ArrayList<>(); - for (String element : excludeArray) { - excludePatterns.add(Pattern.compile(element)); - } - } + + configExcludes(this, config.getInitParameter("excludePatterns")); coresInit = createCoreContainer(computeSolrHome(config), extraProperties); this.httpClient = coresInit.getUpdateShardHandler().getDefaultHttpClient(); @@ -432,16 +432,8 @@ 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(); From a4e8235227bd11ee5a6169e4ca7616c8ff9a2a3c Mon Sep 17 00:00:00 2001 From: Gus Heck Date: Sat, 14 Aug 2021 18:19:17 -0400 Subject: [PATCH 3/9] pull out tracing and rate limiting for later re-use (requires access to CoreContainer) --- .../org/apache/solr/servlet/AdminServlet.java | 26 ++- .../solr/servlet/ExceptionWhileTracing.java | 25 +++ .../org/apache/solr/servlet/PathExcluder.java | 16 ++ .../org/apache/solr/servlet/ServletUtils.java | 135 +++++++++++++ .../servlet/SolrAuthenticationException.java | 20 ++ .../solr/servlet/SolrDispatchFilter.java | 181 +++++++----------- solr/webapp/web/WEB-INF/web.xml | 10 + 7 files changed, 299 insertions(+), 114 deletions(-) create mode 100644 solr/core/src/java/org/apache/solr/servlet/ExceptionWhileTracing.java create mode 100644 solr/core/src/java/org/apache/solr/servlet/SolrAuthenticationException.java diff --git a/solr/core/src/java/org/apache/solr/servlet/AdminServlet.java b/solr/core/src/java/org/apache/solr/servlet/AdminServlet.java index b56ac9744ff..a0cefee3767 100644 --- a/solr/core/src/java/org/apache/solr/servlet/AdminServlet.java +++ b/solr/core/src/java/org/apache/solr/servlet/AdminServlet.java @@ -1,3 +1,19 @@ +/* + * 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; @@ -29,7 +45,10 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se if (excludedPath(excludes,req,resp,null)) { return; } - super.doGet(req, resp); + ServletUtils.rateLimitRequest(req,resp,() -> { + // stuff from HttpSolrCallHere + },false); + // todo: enable tracing } @Override @@ -39,7 +58,10 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S if (excludedPath(excludes,req,resp,null)) { return; } - super.doPost(req, resp); + ServletUtils.rateLimitRequest(req,resp,() -> { + // stuff from HttpSolrCallHere + },false); + //todo: enable tracing } @Override diff --git a/solr/core/src/java/org/apache/solr/servlet/ExceptionWhileTracing.java b/solr/core/src/java/org/apache/solr/servlet/ExceptionWhileTracing.java new file mode 100644 index 00000000000..2a7aa427845 --- /dev/null +++ b/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 { + public Exception e; + + public ExceptionWhileTracing(Exception e) { + this.e = e; + } +} diff --git a/solr/core/src/java/org/apache/solr/servlet/PathExcluder.java b/solr/core/src/java/org/apache/solr/servlet/PathExcluder.java index 9ff21211b7f..26653dc9084 100644 --- a/solr/core/src/java/org/apache/solr/servlet/PathExcluder.java +++ b/solr/core/src/java/org/apache/solr/servlet/PathExcluder.java @@ -1,3 +1,19 @@ +/* + * 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; diff --git a/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java b/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java index 7bf6b51dc55..5a8f1478d6d 100644 --- a/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java +++ b/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java @@ -17,6 +17,20 @@ package org.apache.solr.servlet; +import io.opentracing.Span; +import io.opentracing.Tracer; +import io.opentracing.noop.NoopSpan; +import io.opentracing.noop.NoopTracer; +import io.opentracing.propagation.Format; +import io.opentracing.tag.Tags; +import org.apache.http.HttpHeaders; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.SolrException.ErrorCode; +import org.apache.solr.request.SolrRequestInfo; +import org.apache.solr.util.tracing.HttpServletCarrier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import javax.servlet.FilterChain; import javax.servlet.ReadListener; import javax.servlet.ServletException; @@ -30,6 +44,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.lang.invoke.MethodHandles; import java.util.ArrayList; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -38,6 +53,8 @@ * Various Util methods for interaction on servlet level, i.e. HttpServletRequest */ public abstract class ServletUtils { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + static String CLOSE_STREAM_MSG = "Attempted close of http request or response stream - in general you should not do this, " + "you may spoil connection reuse and possibly disrupt a client. If you must close without actually needing to close, " + "use a CloseShield*Stream. Closing or flushing the response stream commits the response and prevents us from modifying it. " @@ -158,6 +175,124 @@ static void configExcludes(PathExcluder excluder, String patternConfig) { } } + static void rateLimitRequest(HttpServletRequest request, HttpServletResponse response, Runnable limitedExecution, boolean trace) throws ServletException, IOException { + boolean accepted = false; + RateLimitManager rateLimitManager = getRateLimitManager(request); + try { + try { + accepted = rateLimitManager.handleRequest(request); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new SolrException(ErrorCode.SERVER_ERROR, e.getMessage()); + } + + 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); + } + // todo: this shouldn't be required, tracing and rate limiting should be independently composable + traceHttpRequestExecution2(request, response, limitedExecution, trace); + } finally { + if (accepted) { + rateLimitManager.decrementActiveRequests(request); + } + } + } + + /** + * Sets up tracing for an HTTP request. + * + * @param tracedExecution the executed code + */ + private static void traceHttpRequestExecution2(HttpServletRequest request, HttpServletResponse response, Runnable tracedExecution, boolean required) throws ServletException, IOException { + Tracer tracer = getTracer(request); + if (tracer != null) { + Span span = buildSpan(tracer, request); + + request.setAttribute(Span.class.getName(), span); + try (var scope = tracer.scopeManager().activate(span)) { + + assert scope != null; // prevent javac warning about scope being unused + try { + tracedExecution.run(); + } catch (ExceptionWhileTracing e) { + if (e.e instanceof SolrAuthenticationException) { + throw (SolrAuthenticationException) e.e; + } + if (e.e instanceof ServletException) { + throw (ServletException) e.e; + } + if (e.e instanceof IOException) { + throw (IOException) e.e; + } + if (e.e instanceof RuntimeException) { + throw (RuntimeException) e.e; + } else { + throw new RuntimeException(e.e); + } + } + } catch (SolrAuthenticationException e) { + // done, the response and status code have already been sent + } finally { + consumeInputFully(request, response); + SolrRequestInfo.reset(); + SolrRequestParsers.cleanupMultipartFiles(request); + + + span.setTag(Tags.HTTP_STATUS, response.getStatus()); + span.finish(); + } + } else { + if (required) { + throw new IllegalStateException("Tracing required, but could not find Tracer in request attribute:" + SolrDispatchFilter.ATTR_TRACING_TRACER); + } else { + tracedExecution.run(); + } + } + } + + private static Tracer getTracer(HttpServletRequest req) { + return (Tracer) req.getAttribute(SolrDispatchFilter.ATTR_TRACING_TRACER); + } + + private static RateLimitManager getRateLimitManager(HttpServletRequest req) { + return (RateLimitManager) req.getAttribute(SolrDispatchFilter.ATTR_RATELIMIT_MANAGER); + } + + protected static Span buildSpan(Tracer tracer, HttpServletRequest request) { + if (tracer instanceof NoopTracer) { + return NoopSpan.INSTANCE; + } + Tracer.SpanBuilder spanBuilder = tracer.buildSpan("http.request") // will be changed later + .asChildOf(tracer.extract(Format.Builtin.HTTP_HEADERS, new HttpServletCarrier(request))) + .withTag(Tags.SPAN_KIND, Tags.SPAN_KIND_SERVER) + .withTag(Tags.HTTP_METHOD, request.getMethod()) + .withTag(Tags.HTTP_URL, request.getRequestURL().toString()); + if (request.getQueryString() != null) { + spanBuilder.withTag("http.params", request.getQueryString()); + } + spanBuilder.withTag(Tags.DB_TYPE, "solr"); + return spanBuilder.start(); + } + + // we make sure we read the full client request so that the client does + // not hit a connection reset and we can reuse the + // connection - see SOLR-8453 and SOLR-8683 + private static void consumeInputFully(HttpServletRequest req, HttpServletResponse response) { + try { + ServletInputStream is = req.getInputStream(); + while (!is.isFinished() && is.read() != -1) {} + } catch (IOException e) { + if (req.getHeader(HttpHeaders.EXPECT) != null && response.isCommitted()) { + log.debug("No input stream to consume from client"); + } else { + log.info("Could not consume full client request", e); + } + } + } + public static class ClosedServletInputStream extends ServletInputStream { public static final ClosedServletInputStream CLOSED_SERVLET_INPUT_STREAM = new ClosedServletInputStream(); diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrAuthenticationException.java b/solr/core/src/java/org/apache/solr/servlet/SolrAuthenticationException.java new file mode 100644 index 00000000000..d59f84b43e3 --- /dev/null +++ b/solr/core/src/java/org/apache/solr/servlet/SolrAuthenticationException.java @@ -0,0 +1,20 @@ +/* + * 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 SolrAuthenticationException extends Exception{ +} diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java index b5c8d9df1d0..c2bf5122b29 100644 --- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java +++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java @@ -23,17 +23,14 @@ import com.google.common.annotations.VisibleForTesting; import io.opentracing.Span; import io.opentracing.Tracer; -import io.opentracing.noop.NoopSpan; -import io.opentracing.noop.NoopTracer; -import io.opentracing.propagation.Format; import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; import org.apache.commons.lang3.StringUtils; -import org.apache.http.HttpHeaders; import org.apache.http.client.HttpClient; import org.apache.lucene.util.Version; import org.apache.solr.api.V2HttpCall; 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; @@ -48,14 +45,12 @@ import org.apache.solr.metrics.OperatingSystemMetricSet; import org.apache.solr.metrics.SolrMetricManager; import org.apache.solr.metrics.SolrMetricProducer; -import org.apache.solr.request.SolrRequestInfo; import org.apache.solr.security.AuditEvent; import org.apache.solr.security.AuthenticationPlugin; import org.apache.solr.security.PKIAuthenticationPlugin; import org.apache.solr.security.PublicKeyHandler; import org.apache.solr.util.StartupLoggingUtils; import org.apache.solr.util.configuration.SSLConfigurationsFactory; -import org.apache.solr.util.tracing.HttpServletCarrier; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -66,7 +61,6 @@ import javax.servlet.FilterChain; import javax.servlet.FilterConfig; import javax.servlet.ServletException; -import javax.servlet.ServletInputStream; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.UnavailableException; @@ -98,8 +92,14 @@ * * @since solr 1.2 */ +// todo: get rid of this class entirely! Request dispatch is the container's responsibility. Much of what we have here +// should be several separate but composable servlet Filters, and a ServletContextListener for startup/shutdown +// that sets up a service from which things like CoreContainer can be requested. (or better yet injected) public class SolrDispatchFilter extends BaseSolrFilter implements PathExcluder { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + public static final String ATTR_TRACING_SPAN = Span.class.getName(); + public static final String ATTR_TRACING_TRACER = Tracer.class.getName(); + public static final String ATTR_RATELIMIT_MANAGER = RateLimitManager.class.getName(); protected volatile CoreContainer cores; protected final CountDownLatch init = new CountDownLatch(1); @@ -435,129 +435,86 @@ public void doFilter(ServletRequest _request, ServletResponse _response, FilterC 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); - } - } - + waitForCoreContainer(cores, init); + request.setAttribute(Tracer.class.getName(), cores == null ? GlobalTracer.get() : cores.getTracer()); + 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 void waitForCoreContainer(CoreContainer cores, CountDownLatch latch) throws UnavailableException { + if (cores == null || cores.isShutDown()) { + try { + latch.await(); + } catch (InterruptedException e) { //well, no wait then } - - AtomicReference wrappedRequest = new AtomicReference<>(); - if (!authenticateRequest(request, response, wrappedRequest)) { // the response and status code have already been sent - return; + 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); } + } + } - if (wrappedRequest.get() != null) { - request = wrappedRequest.get(); - } + private static Span getSpan(HttpServletRequest req) { + return (Span) req.getAttribute(ATTR_TRACING_SPAN); + } - if (cores.getAuthenticationPlugin() != null) { - if (log.isDebugEnabled()) { - log.debug("User principal: {}", request.getUserPrincipal()); - } - span.setTag(Tags.DB_USER, String.valueOf(request.getUserPrincipal())); - } + private void dispatch(FilterChain chain, HttpServletRequest request, HttpServletResponse response, boolean retry) throws IOException, ServletException, SolrAuthenticationException { - 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 wrappedRequest = new AtomicReference<>(); + if (!authenticateRequest(request, response, wrappedRequest)) { // the response and status code have already been sent + throw new SolrAuthenticationException(); } - } - protected Span buildSpan(Tracer tracer, HttpServletRequest request) { - if (tracer instanceof NoopTracer) { - return NoopSpan.INSTANCE; + if (wrappedRequest.get() != null) { + request = wrappedRequest.get(); } - Tracer.SpanBuilder spanBuilder = tracer.buildSpan("http.request") // will be changed later - .asChildOf(tracer.extract(Format.Builtin.HTTP_HEADERS, new HttpServletCarrier(request))) - .withTag(Tags.SPAN_KIND, Tags.SPAN_KIND_SERVER) - .withTag(Tags.HTTP_METHOD, request.getMethod()) - .withTag(Tags.HTTP_URL, request.getRequestURL().toString()); - if (request.getQueryString() != null) { - spanBuilder.withTag("http.params", request.getQueryString()); + + if (cores.getAuthenticationPlugin() != null) { + if (log.isDebugEnabled()) { + log.debug("User principal: {}", request.getUserPrincipal()); + } + getSpan(request).setTag(Tags.DB_USER, String.valueOf(request.getUserPrincipal())); } - spanBuilder.withTag(Tags.DB_TYPE, "solr"); - return spanBuilder.start(); - } - // we make sure we read the full client request so that the client does - // not hit a connection reset and we can reuse the - // connection - see SOLR-8453 and SOLR-8683 - private void consumeInputFully(HttpServletRequest req, HttpServletResponse response) { + HttpSolrCall call = getHttpSolrCall(request, response, retry); + ExecutorUtil.setServerThreadFlag(Boolean.TRUE); try { - ServletInputStream is = req.getInputStream(); - while (!is.isFinished() && is.read() != -1) {} - } catch (IOException e) { - if (req.getHeader(HttpHeaders.EXPECT) != null && response.isCommitted()) { - log.debug("No input stream to consume from client"); - } else { - log.info("Could not consume full client request", e); + Action result = call.call(); + switch (result) { + case PASSTHROUGH: + getSpan(request).log("SolrDispatchFilter PASSTHROUGH"); + chain.doFilter(request, response); + break; + case RETRY: + getSpan(request).log("SolrDispatchFilter RETRY"); + doFilter(request, response, chain, true); // RECURSION + break; + case FORWARD: + getSpan(request).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); } } - + /** * Allow a subclass to modify the HttpSolrCall. In particular, subclasses may * want to add attributes to the request and send errors differently @@ -573,7 +530,7 @@ protected HttpSolrCall getHttpSolrCall(HttpServletRequest request, HttpServletRe } private boolean authenticateRequest(HttpServletRequest request, HttpServletResponse response, final AtomicReference wrappedRequest) throws IOException { - boolean requestContinues = false; + boolean requestContinues; final AtomicBoolean isAuthenticated = new AtomicBoolean(false); AuthenticationPlugin authenticationPlugin = cores.getAuthenticationPlugin(); if (authenticationPlugin == null) { diff --git a/solr/webapp/web/WEB-INF/web.xml b/solr/webapp/web/WEB-INF/web.xml index 2599420ddb1..694060d174e 100644 --- a/solr/webapp/web/WEB-INF/web.xml +++ b/solr/webapp/web/WEB-INF/web.xml @@ -42,6 +42,16 @@ /* + + AdminServlet + org.apache.solr.servlet.AdminServlet + + + + AdminServlet + /admin2 + + LoadAdminUI org.apache.solr.servlet.LoadAdminUiServlet From 24b0a4613a0fa61ce2fd914215a6fd06165d76df Mon Sep 17 00:00:00 2001 From: Gus Heck Date: Sun, 15 Aug 2021 23:04:08 -0400 Subject: [PATCH 4/9] CoreService to grant access to the CoreContainer which is started as a ServletContextListener --- .../solrj/embedded/JettySolrRunner.java | 48 +- .../solr/servlet/CoreContainerProvider.java | 23 + .../org/apache/solr/servlet/CoreService.java | 451 ++++++++++++++++++ .../org/apache/solr/servlet/HttpSolrCall.java | 4 +- .../apache/solr/servlet/SolrCoreUtils.java | 63 +++ .../solr/servlet/SolrDispatchFilter.java | 361 ++------------ .../apache/solr/cloud/SolrXmlInZkTest.java | 4 +- .../org/apache/solr/core/TestLazyCores.java | 4 +- .../solr/servlet/HttpSolrCallGetCoreTest.java | 3 +- .../org/apache/solr/util/BaseTestHarness.java | 11 +- solr/webapp/web/WEB-INF/web.xml | 4 +- 11 files changed, 636 insertions(+), 340 deletions(-) create mode 100644 solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java create mode 100644 solr/core/src/java/org/apache/solr/servlet/CoreService.java create mode 100644 solr/core/src/java/org/apache/solr/servlet/SolrCoreUtils.java diff --git a/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java b/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java index 5f481c48e92..fab0709f230 100644 --- a/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java +++ b/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java @@ -23,6 +23,7 @@ import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; +import javax.servlet.UnavailableException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -69,6 +70,7 @@ import org.apache.solr.handler.admin.CoreAdminOperation; import org.apache.solr.handler.admin.LukeRequestHandler; import org.apache.solr.metrics.SolrMetricManager; +import org.apache.solr.servlet.CoreService; import org.apache.solr.servlet.SolrDispatchFilter; import org.apache.solr.util.TimeOut; import org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory; @@ -142,6 +144,7 @@ public class JettySolrRunner { private String host; private volatile boolean started = false; + private CoreService coreService; public static class DebugFilter implements Filter { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); @@ -364,7 +367,9 @@ public void lifeCycleStopping(LifeCycle arg0) { } @Override - public void lifeCycleStopped(LifeCycle arg0) {} + public void lifeCycleStopped(LifeCycle arg0) { + coreService.close(); + } @Override public void lifeCycleStarting(LifeCycle arg0) { @@ -373,7 +378,6 @@ public void lifeCycleStarting(LifeCycle arg0) { @Override public void lifeCycleStarted(LifeCycle arg0) { - jettyPort = getFirstConnectorPort(); int port = jettyPort; if (proxyPort != -1) port = proxyPort; @@ -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()); log.info("Jetty properties: {}", nodeProperties); debugFilter = root.addFilter(DebugFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST) ); @@ -455,10 +460,14 @@ protected HandlerWrapper injectJettyHandlers(HandlerWrapper chain) { * @return the {@link CoreContainer} for this node */ public CoreContainer getCoreContainer() { - if (getSolrDispatchFilter() == null || getSolrDispatchFilter().getCores() == null) { - return null; + try { + if (getSolrDispatchFilter() == null || getSolrDispatchFilter().getCores() == null) { + return null; + } + return getSolrDispatchFilter().getCores(); + } catch (UnavailableException e) { + throw new RuntimeException(e); } - return getSolrDispatchFilter().getCores(); } public String getNodeName() { @@ -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() ) { this.wait(100); if (cnt++ == 15) { throw new RuntimeException("Jetty/Solr unresponsive"); @@ -561,7 +570,7 @@ public void start(boolean reusePort) throws Exception { private void setProtocolAndHost() { - String protocol = null; + String protocol; Connector[] conns = server.getConnectors(); if (0 == conns.length) { @@ -575,7 +584,7 @@ private void setProtocolAndHost() { this.host = c.getHost(); } - private void retryOnPortBindFailure(int portRetryTime, int port) throws Exception, InterruptedException { + private void retryOnPortBindFailure(int portRetryTime, int port) throws Exception { TimeOut timeout = new TimeOut(portRetryTime, TimeUnit.SECONDS, TimeSource.NANO_TIME); int tryCnt = 1; while (true) { @@ -638,7 +647,7 @@ public void stop() throws Exception { if (sdf != null) { customThreadPool = ExecutorUtil.newMDCAwareCachedThreadPool(new SolrNamedThreadFactory("jettyShutDown")); - sdf.closeOnDestroy(false); + sdf.closeOnDestroy(); // customThreadPool.submit(() -> { // try { // sdf.close(); @@ -646,11 +655,7 @@ public void stop() throws Exception { // 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(); @@ -687,8 +692,7 @@ public void stop() throws Exception { rte.stop(); TimeOut timeout = new TimeOut(30, TimeUnit.SECONDS, TimeSource.NANO_TIME); - timeout.waitFor("Timeout waiting for reserved executor to stop.", () - -> rte.isStopped()); + timeout.waitFor("Timeout waiting for reserved executor to stop.", rte::isStopped); } if (customThreadPool != null) { @@ -750,12 +754,11 @@ public void dumpCoresInfo(PrintStream pw) throws IOException { NamedList coreStatus = CoreAdminOperation.getCoreStatus(getCoreContainer(), core.getName(), false); core.withSearcher(solrIndexSearcher -> { SimpleOrderedMap lukeIndexInfo = LukeRequestHandler.getIndexInfo(solrIndexSearcher.getIndexReader()); - @SuppressWarnings({"unchecked", "rawtypes"}) Map indexInfoMap = coreStatus.toMap(new LinkedHashMap<>()); indexInfoMap.putAll(lukeIndexInfo.toMap(new LinkedHashMap<>())); pw.println(JSONUtil.toJSON(indexInfoMap, 2)); - pw.println(""); + pw.println(); return null; }); } @@ -897,7 +900,12 @@ public Properties getNodeProperties() { private void waitForLoadingCoresToFinish(long timeoutMs) { if (dispatchFilter != null) { SolrDispatchFilter solrFilter = (SolrDispatchFilter) dispatchFilter.getFilter(); - CoreContainer cores = solrFilter.getCores(); + CoreContainer cores; + try { + cores = solrFilter.getCores(); + } catch (UnavailableException e) { + throw new IllegalStateException("The CoreContainer is unavailable!"); + } if (cores != null) { cores.waitForLoadingCoresToFinish(timeoutMs); } else { diff --git a/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java b/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java new file mode 100644 index 00000000000..8c3d8ef5718 --- /dev/null +++ b/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(); +} diff --git a/solr/core/src/java/org/apache/solr/servlet/CoreService.java b/solr/core/src/java/org/apache/solr/servlet/CoreService.java new file mode 100644 index 00000000000..11e948f5c29 --- /dev/null +++ b/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 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: + *
    + *
  1. attribute in the FilterConfig
    2. + *
  2. JNDI: via java:comp/env/solr/home
    3. + *
  3. The system property solr.solr.home
    4. + *
  4. Look in the current working directory for a solr/ directory
    5. + *
+ *

+ * + * @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 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); + } + } + + + static class ServiceHolder { + private volatile CoreService service; + private volatile ContextInitializationKey key; + + private ServiceHolder(ContextInitializationKey key) { + if (key == null) { + throw new IllegalArgumentException("Key for accessing this service holder must be supplied"); + } + this.key = key; + } + + public void setService(CoreService service) { + this.service = service; + key.makeReady(); + key = null; // be sure not to hold a reference to the context via the key + } + + public CoreService getService() { + try { + if (key != null) { + try { + key.waitForReadyService(); + } catch (NullPointerException e) { + // ignore, means we raced with set service and lost, but that's fine since null implies we are ready. + } + } + } catch (InterruptedException e) { + throw new SolrException(ErrorCode.SERVER_ERROR,"Interrupted while obtaining reference to CoreService"); + } + return service; + } + } +} diff --git a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java index 460e8d6b4d7..b247c4200e7 100644 --- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java +++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java @@ -460,7 +460,7 @@ Action authorize() throws IOException { if (statusCode == AuthorizationResponse.PROMPT.statusCode) { @SuppressWarnings({"unchecked"}) - Map headers = (Map) getReq().getAttribute(AuthenticationPlugin.class.getName()); + Map headers = (Map) getReq().getAttribute(AuthenticationPlugin.class.getName()); if (headers != null) { for (Map.Entry e : headers.entrySet()) response.setHeader(e.getKey(), e.getValue()); } @@ -730,7 +730,7 @@ private void remoteQuery(String coreUrl, HttpServletResponse resp) throws IOExce } final HttpResponse response - = solrDispatchFilter.httpClient.execute(method, HttpClientUtil.createNewHttpClientRequestContext()); + = solrDispatchFilter.getHttpClient().execute(method, HttpClientUtil.createNewHttpClientRequestContext()); int httpStatus = response.getStatusLine().getStatusCode(); httpEntity = response.getEntity(); diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrCoreUtils.java b/solr/core/src/java/org/apache/solr/servlet/SolrCoreUtils.java new file mode 100644 index 00000000000..3d7a7a59c27 --- /dev/null +++ b/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 { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + /** + * Get the NodeConfig whether stored on disk, in ZooKeeper, etc. + * This may also be used by custom filters to load relevant configuration. + * @return the NodeConfig + */ + public static NodeConfig loadNodeConfig(Path solrHome, Properties nodeProperties) { + if (!StringUtils.isEmpty(System.getProperty("solr.solrxml.location"))) { + log.warn("Solr property solr.solrxml.location is no longer supported. Will automatically load solr.xml from ZooKeeper if it exists"); + } + nodeProperties = SolrXmlConfig.wrapAndSetZkHostFromSysPropIfNeeded(nodeProperties); + String zkHost = nodeProperties.getProperty(SolrXmlConfig.ZK_HOST); + if (!StringUtils.isEmpty(zkHost)) { + int startUpZkTimeOut = Integer.getInteger("waitForZk", 30); + startUpZkTimeOut *= 1000; + try (SolrZkClient zkClient = new SolrZkClient(zkHost, startUpZkTimeOut, startUpZkTimeOut)) { + if (zkClient.exists("/solr.xml", true)) { + log.info("solr.xml found in ZooKeeper. Loading..."); + byte[] data = zkClient.getData("/solr.xml", null, null, true); + return SolrXmlConfig.fromInputStream(solrHome, new ByteArrayInputStream(data), nodeProperties, true); + } + } catch (Exception e) { + throw new SolrException(ErrorCode.SERVER_ERROR, "Error occurred while loading solr.xml from zookeeper", e); + } + log.info("Loading solr.xml from SolrHome (not found in ZooKeeper)"); + } + + return SolrXmlConfig.fromSolrHome(solrHome, nodeProperties); + } +} diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java index c2bf5122b29..e7a028369c1 100644 --- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java +++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java @@ -16,48 +16,27 @@ */ 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 io.opentracing.Span; import io.opentracing.Tracer; import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; -import org.apache.commons.lang3.StringUtils; import org.apache.http.client.HttpClient; -import org.apache.lucene.util.Version; import org.apache.solr.api.V2HttpCall; -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.common.util.ExecutorUtil; import org.apache.solr.core.CoreContainer; -import org.apache.solr.core.NodeConfig; import org.apache.solr.core.SolrCore; -import org.apache.solr.core.SolrInfoBean; -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.SolrMetricProducer; import org.apache.solr.security.AuditEvent; import org.apache.solr.security.AuthenticationPlugin; import org.apache.solr.security.PKIAuthenticationPlugin; import org.apache.solr.security.PublicKeyHandler; -import org.apache.solr.util.StartupLoggingUtils; +import org.apache.solr.servlet.CoreService.ServiceHolder; import org.apache.solr.util.configuration.SSLConfigurationsFactory; 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.FilterChain; import javax.servlet.FilterConfig; import javax.servlet.ServletException; @@ -66,17 +45,9 @@ import javax.servlet.UnavailableException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import java.io.ByteArrayInputStream; import java.io.IOException; import java.lang.invoke.MethodHandles; -import java.nio.file.Path; -import java.nio.file.Paths; -import java.time.Instant; import java.util.ArrayList; -import java.util.Arrays; -import java.util.Locale; -import java.util.Properties; -import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; @@ -101,12 +72,12 @@ public class SolrDispatchFilter extends BaseSolrFilter implements PathExcluder { public static final String ATTR_TRACING_TRACER = Tracer.class.getName(); public static final String ATTR_RATELIMIT_MANAGER = RateLimitManager.class.getName(); - protected volatile CoreContainer cores; + // TODO: see if we can get rid of the holder here (Servlet spec actually guarantees ContextListeners run before filter init) + private ServiceHolder coreService; + protected final CountDownLatch init = new CountDownLatch(1); protected String abortErrorMessage = null; - //TODO using Http2Client - protected HttpClient httpClient; @Override public void setExcludePatterns(ArrayList excludePatterns) { @@ -115,16 +86,18 @@ public void setExcludePatterns(ArrayList excludePatterns) { private ArrayList excludePatterns; - private boolean isV2Enabled = !"true".equals(System.getProperty("disable.v2.api", "false")); - - private final String metricTag = SolrMetricProducer.getUniqueMetricTag(this, null); - private SolrMetricManager metricManager; - private String registryName; - private volatile boolean closeOnDestroy = true; - private Properties extraProperties; + private final boolean isV2Enabled = !"true".equals(System.getProperty("disable.v2.api", "false")); private RateLimitManager rateLimitManager; + public HttpClient getHttpClient() { + try { + return coreService.getService().getHttpClient(); + } catch (UnavailableException e) { + throw new SolrException(ErrorCode.SERVER_ERROR, "Internal Http Client Unavailable, startup may have failed"); + } + } + /** * Enum to define action that needs to be processed. * PASSTHROUGH: Pass through to another filter via webapp. @@ -154,274 +127,39 @@ public SolrDispatchFilter() { @Override public void init(FilterConfig config) throws ServletException { - SSLConfigurationsFactory.current().init(); - if (log.isTraceEnabled()) { - log.trace("SolrDispatchFilter.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) config.getServletContext().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); + coreService = CoreService.serviceForContext(config.getServletContext()); + SSLConfigurationsFactory.current().init(); + if (log.isTraceEnabled()) { + log.trace("SolrDispatchFilter.init(): {}", this.getClass().getClassLoader()); } configExcludes(this, config.getInitParameter("excludePatterns")); + } catch (InterruptedException e) { + throw new ServletException("Interrupted while fetching core service"); - coresInit = createCoreContainer(computeSolrHome(config), extraProperties); - this.httpClient = coresInit.getUpdateShardHandler().getDefaultHttpClient(); - setupJvmMetrics(coresInit); - - SolrZkClient zkClient = null; - ZkController zkController = coresInit.getZkController(); - - if (zkController != null) { - zkClient = zkController.getZkClient(); - } - - RateLimitManager.Builder builder = new RateLimitManager.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 (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 ); + log.error("Could not start Dispatch Filter."); + SolrCore.log(t); if (t instanceof Error) { throw (Error) t; } - - } finally{ + } finally { log.trace("SolrDispatchFilter.init() done"); - this.cores = coresInit; // crucially final assignment init.countDown(); } } - private void setupJvmMetrics(CoreContainer coresInit) { - metricManager = coresInit.getMetricManager(); - registryName = SolrMetricManager.getRegistryName(SolrInfoBean.Group.jvm); - final Set hiddenSysProps = coresInit.getConfig().getMetricsConfig().getHiddenSysProps(); - try { - metricManager.registerAll(registryName, new AltBufferPoolMetricSet(), SolrMetricManager.ResolutionStrategy.IGNORE, "buffers"); - metricManager.registerAll(registryName, new ClassLoadingGaugeSet(), SolrMetricManager.ResolutionStrategy.IGNORE, "classes"); - metricManager.registerAll(registryName, new OperatingSystemMetricSet(), SolrMetricManager.ResolutionStrategy.IGNORE, "os"); - metricManager.registerAll(registryName, new GarbageCollectorMetricSet(), SolrMetricManager.ResolutionStrategy.IGNORE, "gc"); - metricManager.registerAll(registryName, new MemoryUsageGaugeSet(), SolrMetricManager.ResolutionStrategy.IGNORE, "memory"); - metricManager.registerAll(registryName, new ThreadStatesGaugeSet(), SolrMetricManager.ResolutionStrategy.IGNORE, "threads"); // todo should we use CachedThreadStatesGaugeSet instead? - MetricsMap sysprops = new MetricsMap(map -> { - System.getProperties().forEach((k, v) -> { - if (!hiddenSysProps.contains(k)) { - map.putNoEx(String.valueOf(k), v); - } - }); - }); - metricManager.registerGauge(null, registryName, sysprops, metricTag, SolrMetricManager.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, SolrMetricManager.ResolutionStrategy.IGNORE, "env", "system"); - } catch (Exception e) { - log.warn("Error registering JVM metrics", e); - } - } - - 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")); + public CoreContainer getCores() throws UnavailableException { + return coreService.getService().getCoreContainer(); } - /** - * Returns the effective Solr Home to use for this node, based on looking up the value in this order: - *

    - *
  1. attribute in the FilterConfig
    2. - *
  2. JNDI: via java:comp/env/solr/home
    3. - *
  3. The system property solr.solr.home
    4. - *
  4. Look in the current working directory for a solr/ directory
    5. - *
- *

- * - * @return the Solr home, absolute and normalized. - * @see #SOLRHOME_ATTRIBUTE - */ - private static Path computeSolrHome(FilterConfig config) { - - // start with explicit check of servlet config... - String source = "servlet config: " + SOLRHOME_ATTRIBUTE; - String home = (String) config.getServletContext().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; - } - - /** - * Get the NodeConfig whether stored on disk, in ZooKeeper, etc. - * This may also be used by custom filters to load relevant configuration. - * @return the NodeConfig - */ - public static NodeConfig loadNodeConfig(Path solrHome, Properties nodeProperties) { - if (!StringUtils.isEmpty(System.getProperty("solr.solrxml.location"))) { - log.warn("Solr property solr.solrxml.location is no longer supported. Will automatically load solr.xml from ZooKeeper if it exists"); - } - nodeProperties = SolrXmlConfig.wrapAndSetZkHostFromSysPropIfNeeded(nodeProperties); - String zkHost = nodeProperties.getProperty(SolrXmlConfig.ZK_HOST); - if (!StringUtils.isEmpty(zkHost)) { - int startUpZkTimeOut = Integer.getInteger("waitForZk", 30); - startUpZkTimeOut *= 1000; - try (SolrZkClient zkClient = new SolrZkClient(zkHost, startUpZkTimeOut, startUpZkTimeOut)) { - if (zkClient.exists("/solr.xml", true)) { - log.info("solr.xml found in ZooKeeper. Loading..."); - byte[] data = zkClient.getData("/solr.xml", null, null, true); - return SolrXmlConfig.fromInputStream(solrHome, new ByteArrayInputStream(data), nodeProperties, true); - } - } catch (Exception e) { - throw new SolrException(ErrorCode.SERVER_ERROR, "Error occurred while loading solr.xml from zookeeper", e); - } - log.info("Loading solr.xml from SolrHome (not found in ZooKeeper)"); - } - - return SolrXmlConfig.fromSolrHome(solrHome, nodeProperties); - } - - public CoreContainer getCores() { - return cores; - } - @Override public void destroy() { - if (closeOnDestroy) { - close(); - } + // CoreService shuts itself down as a ContextListener. The filter does not own anything with a lifecycle anymore! Yay! } - - 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(); - } - } - } - + @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { doFilter(request, response, chain, false); @@ -435,8 +173,9 @@ public void doFilter(ServletRequest _request, ServletResponse _response, FilterC if (excludedPath(excludePatterns, request, response, chain)) { return; } - waitForCoreContainer(cores, init); - request.setAttribute(Tracer.class.getName(), cores == null ? GlobalTracer.get() : cores.getTracer()); + 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 { @@ -447,21 +186,6 @@ public void doFilter(ServletRequest _request, ServletResponse _response, FilterC }, true); } - - private static void waitForCoreContainer(CoreContainer cores, CountDownLatch latch) throws UnavailableException { - if (cores == null || cores.isShutDown()) { - try { - latch.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); - } - } - } - private static Span getSpan(HttpServletRequest req) { return (Span) req.getAttribute(ATTR_TRACING_SPAN); } @@ -479,7 +203,7 @@ private void dispatch(FilterChain chain, HttpServletRequest request, HttpServlet request = wrappedRequest.get(); } - if (cores.getAuthenticationPlugin() != null) { + if (getCores().getAuthenticationPlugin() != null) { if (log.isDebugEnabled()) { log.debug("User principal: {}", request.getUserPrincipal()); } @@ -522,6 +246,12 @@ private void dispatch(FilterChain chain, HttpServletRequest request, HttpServlet protected HttpSolrCall getHttpSolrCall(HttpServletRequest request, HttpServletResponse response, boolean retry) { String path = ServletUtils.getPathAfterContext(request); + CoreContainer cores; + try { + cores = getCores(); + } catch (UnavailableException e) { + throw new SolrException(ErrorCode.SERVER_ERROR, "Core Container Unavailable"); + } if (isV2Enabled && (path.startsWith("/____v2/") || path.equals("/____v2"))) { return new V2HttpCall(this, cores, request, response, false); } else { @@ -532,6 +262,12 @@ protected HttpSolrCall getHttpSolrCall(HttpServletRequest request, HttpServletRe private boolean authenticateRequest(HttpServletRequest request, HttpServletResponse response, final AtomicReference wrappedRequest) throws IOException { boolean requestContinues; final AtomicBoolean isAuthenticated = new AtomicBoolean(false); + CoreContainer cores; + try { + cores = getCores(); + } catch (UnavailableException e) { + throw new SolrException(ErrorCode.SERVER_ERROR, "Core Container Unavailable"); + } AuthenticationPlugin authenticationPlugin = cores.getAuthenticationPlugin(); if (authenticationPlugin == null) { if (shouldAudit(EventType.ANONYMOUS)) { @@ -592,15 +328,20 @@ private boolean authenticateRequest(HttpServletRequest request, HttpServletRespo * @param eventType the audit event */ private boolean shouldAudit(AuditEvent.EventType eventType) { + CoreContainer cores; + try { + cores = getCores(); + } catch (UnavailableException e) { + throw new SolrException(ErrorCode.SERVER_ERROR, "Core Container Unavailable"); + } return cores.getAuditLoggerPlugin() != null && cores.getAuditLoggerPlugin().shouldLog(eventType); } - public void closeOnDestroy(boolean closeOnDestroy) { - this.closeOnDestroy = closeOnDestroy; + public void closeOnDestroy() { } @VisibleForTesting void replaceRateLimitManager(RateLimitManager rateLimitManager) { - this.rateLimitManager = rateLimitManager; + coreService.getService().setRateLimitManager(rateLimitManager); } } diff --git a/solr/core/src/test/org/apache/solr/cloud/SolrXmlInZkTest.java b/solr/core/src/test/org/apache/solr/cloud/SolrXmlInZkTest.java index 8e1358c9fb8..e3437f0ff5f 100644 --- a/solr/core/src/test/org/apache/solr/cloud/SolrXmlInZkTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/SolrXmlInZkTest.java @@ -29,7 +29,7 @@ import org.apache.solr.common.cloud.SolrZkClient; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.core.NodeConfig; -import org.apache.solr.servlet.SolrDispatchFilter; +import org.apache.solr.servlet.SolrCoreUtils; import org.junit.Rule; import org.junit.Test; import org.junit.rules.RuleChain; @@ -90,7 +90,7 @@ private void setUpZkAndDiskXml(boolean toZk, boolean leaveOnLocal) throws Except props.setProperty("solr.test.sys.prop1", "propone"); props.setProperty("solr.test.sys.prop2", "proptwo"); - cfg = SolrDispatchFilter.loadNodeConfig(solrHome, props); + cfg = SolrCoreUtils.loadNodeConfig(solrHome, props); if (log.isInfoEnabled()) { log.info("####SETUP_END {}", getTestName()); } diff --git a/solr/core/src/test/org/apache/solr/core/TestLazyCores.java b/solr/core/src/test/org/apache/solr/core/TestLazyCores.java index 11aa24694bc..e790a825026 100644 --- a/solr/core/src/test/org/apache/solr/core/TestLazyCores.java +++ b/solr/core/src/test/org/apache/solr/core/TestLazyCores.java @@ -40,7 +40,7 @@ import org.apache.solr.request.LocalSolrQueryRequest; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; -import org.apache.solr.servlet.SolrDispatchFilter; +import org.apache.solr.servlet.SolrCoreUtils; import org.apache.solr.update.AddUpdateCommand; import org.apache.solr.update.CommitUpdateCommand; import org.apache.solr.update.UpdateHandler; @@ -94,7 +94,7 @@ private CoreContainer init() throws Exception { copyMinConf(new File(solrHomeDirectory, "collection" + idx)); } - NodeConfig cfg = SolrDispatchFilter.loadNodeConfig(solrHomeDirectory.toPath(), null); + NodeConfig cfg = SolrCoreUtils.loadNodeConfig(solrHomeDirectory.toPath(), null); return createCoreContainer(cfg, testCores); } diff --git a/solr/core/src/test/org/apache/solr/servlet/HttpSolrCallGetCoreTest.java b/solr/core/src/test/org/apache/solr/servlet/HttpSolrCallGetCoreTest.java index 34eb34471fc..7c06028e6ed 100644 --- a/solr/core/src/test/org/apache/solr/servlet/HttpSolrCallGetCoreTest.java +++ b/solr/core/src/test/org/apache/solr/servlet/HttpSolrCallGetCoreTest.java @@ -20,6 +20,7 @@ import javax.servlet.ReadListener; import javax.servlet.ServletInputStream; import javax.servlet.ServletOutputStream; +import javax.servlet.UnavailableException; import javax.servlet.WriteListener; import java.io.IOException; import java.util.HashSet; @@ -60,7 +61,7 @@ public void test() throws Exception { assertCoreChosen(NUM_SHARD * REPLICA_FACTOR, new TestRequest("/collection1/select")); } - private void assertCoreChosen(int numCores, TestRequest testRequest) { + private void assertCoreChosen(int numCores, TestRequest testRequest) throws UnavailableException { JettySolrRunner jettySolrRunner = cluster.getJettySolrRunner(0); Set coreNames = new HashSet<>(); SolrDispatchFilter dispatchFilter = jettySolrRunner.getSolrDispatchFilter(); diff --git a/solr/test-framework/src/java/org/apache/solr/util/BaseTestHarness.java b/solr/test-framework/src/java/org/apache/solr/util/BaseTestHarness.java index ad1d38e4ddc..33f439db7fd 100644 --- a/solr/test-framework/src/java/org/apache/solr/util/BaseTestHarness.java +++ b/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("")) { +// // probably got an error page +// System.err.println(xml); +// } document = getXmlDocumentBuilder().parse(new ByteArrayInputStream (xml.getBytes(StandardCharsets.UTF_8))); } catch (UnsupportedEncodingException e1) { throw new RuntimeException("Totally weird UTF-8 exception", e1); } catch (IOException e2) { throw new RuntimeException("Totally weird io exception", e2); +// } catch (SAXParseException spe) { +// System.err.println(xml); +// throw spe; } for (String xp : tests) { @@ -102,7 +109,7 @@ public static Object evaluateXPath(String xml, String xpath, QName returnType) throws XPathExpressionException, SAXException { if (null == xpath) return null; - Document document = null; + Document document; try { document = getXmlDocumentBuilder().parse(new ByteArrayInputStream (xml.getBytes(StandardCharsets.UTF_8))); diff --git a/solr/webapp/web/WEB-INF/web.xml b/solr/webapp/web/WEB-INF/web.xml index 694060d174e..f4b9544a4b0 100644 --- a/solr/webapp/web/WEB-INF/web.xml +++ b/solr/webapp/web/WEB-INF/web.xml @@ -21,7 +21,9 @@ version="2.5" metadata-complete="true" > - + + org.apache.solr.servlet.CoreService + SolrRequestFilter From a37a814b6b97eeeca0d18d294841b6ed1711e086 Mon Sep 17 00:00:00 2001 From: Gus Heck Date: Sun, 31 Oct 2021 17:42:45 -0400 Subject: [PATCH 5/9] Review feedback, but PR is too stale so will need to re-do on new branch --- .../solrj/embedded/JettySolrRunner.java | 10 +- .../java/org/apache/solr/core/NodeConfig.java | 39 ++ .../org/apache/solr/servlet/AdminServlet.java | 18 +- .../solr/servlet/CoreContainerProvider.java | 460 +++++++++++++++++- .../org/apache/solr/servlet/CoreService.java | 451 ----------------- .../solr/servlet/ExceptionWhileTracing.java | 3 + .../org/apache/solr/servlet/PathExcluder.java | 8 +- .../org/apache/solr/servlet/ServletUtils.java | 3 +- .../apache/solr/servlet/SolrCoreUtils.java | 63 --- .../solr/servlet/SolrDispatchFilter.java | 45 +- .../apache/solr/cloud/SolrXmlInZkTest.java | 3 +- .../org/apache/solr/core/TestLazyCores.java | 3 +- solr/webapp/web/WEB-INF/web.xml | 2 +- 13 files changed, 552 insertions(+), 556 deletions(-) delete mode 100644 solr/core/src/java/org/apache/solr/servlet/CoreService.java delete mode 100644 solr/core/src/java/org/apache/solr/servlet/SolrCoreUtils.java diff --git a/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java b/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java index fab0709f230..03a4beb064b 100644 --- a/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java +++ b/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java @@ -70,7 +70,7 @@ import org.apache.solr.handler.admin.CoreAdminOperation; import org.apache.solr.handler.admin.LukeRequestHandler; import org.apache.solr.metrics.SolrMetricManager; -import org.apache.solr.servlet.CoreService; +import org.apache.solr.servlet.CoreContainerProvider; import org.apache.solr.servlet.SolrDispatchFilter; import org.apache.solr.util.TimeOut; import org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory; @@ -144,7 +144,7 @@ public class JettySolrRunner { private String host; private volatile boolean started = false; - private CoreService coreService; + private CoreContainerProvider coreContainerProvider; public static class DebugFilter implements Filter { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); @@ -368,7 +368,7 @@ public void lifeCycleStopping(LifeCycle arg0) { @Override public void lifeCycleStopped(LifeCycle arg0) { - coreService.close(); + coreContainerProvider.close(); } @Override @@ -386,8 +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()); + coreContainerProvider = new CoreContainerProvider(); + coreContainerProvider.init(root.getServletContext()); log.info("Jetty properties: {}", nodeProperties); debugFilter = root.addFilter(DebugFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST) ); diff --git a/solr/core/src/java/org/apache/solr/core/NodeConfig.java b/solr/core/src/java/org/apache/solr/core/NodeConfig.java index cd8bca76740..03195c73392 100644 --- a/solr/core/src/java/org/apache/solr/core/NodeConfig.java +++ b/solr/core/src/java/org/apache/solr/core/NodeConfig.java @@ -16,11 +16,19 @@ */ package org.apache.solr.core; +import org.apache.commons.lang3.StringUtils; import org.apache.lucene.search.IndexSearcher; import org.apache.solr.common.SolrException; +import org.apache.solr.common.SolrException.ErrorCode; +import org.apache.solr.common.cloud.SolrZkClient; import org.apache.solr.logging.LogWatcherConfig; + import org.apache.solr.update.UpdateShardHandlerConfig; +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.Arrays; import java.util.Collections; @@ -31,6 +39,8 @@ public class NodeConfig { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + // all Path fields here are absolute and normalized. private final String nodeName; @@ -149,6 +159,35 @@ private NodeConfig(String nodeName, Path coreRootDirectory, Path solrDataHome, I if (null == this.loader) throw new NullPointerException("loader"); } + /** + * Get the NodeConfig whether stored on disk, in ZooKeeper, etc. + * This may also be used by custom filters to load relevant configuration. + * @return the NodeConfig + */ + public static NodeConfig loadNodeConfig(Path solrHome, Properties nodeProperties) { + if (!StringUtils.isEmpty(System.getProperty("solr.solrxml.location"))) { + log.warn("Solr property solr.solrxml.location is no longer supported. Will automatically load solr.xml from ZooKeeper if it exists"); + } + nodeProperties = SolrXmlConfig.wrapAndSetZkHostFromSysPropIfNeeded(nodeProperties); + String zkHost = nodeProperties.getProperty(SolrXmlConfig.ZK_HOST); + if (!StringUtils.isEmpty(zkHost)) { + int startUpZkTimeOut = Integer.getInteger("waitForZk", 30); + startUpZkTimeOut *= 1000; + try (SolrZkClient zkClient = new SolrZkClient(zkHost, startUpZkTimeOut, startUpZkTimeOut)) { + if (zkClient.exists("/solr.xml", true)) { + log.info("solr.xml found in ZooKeeper. Loading..."); + byte[] data = zkClient.getData("/solr.xml", null, null, true); + return SolrXmlConfig.fromInputStream(solrHome, new ByteArrayInputStream(data), nodeProperties, true); + } + } catch (Exception e) { + throw new SolrException(ErrorCode.SERVER_ERROR, "Error occurred while loading solr.xml from zookeeper", e); + } + log.info("Loading solr.xml from SolrHome (not found in ZooKeeper)"); + } + + return SolrXmlConfig.fromSolrHome(solrHome, nodeProperties); + } + public String getConfigSetServiceClass() { return this.configSetServiceClass; } diff --git a/solr/core/src/java/org/apache/solr/servlet/AdminServlet.java b/solr/core/src/java/org/apache/solr/servlet/AdminServlet.java index a0cefee3767..034578429a6 100644 --- a/solr/core/src/java/org/apache/solr/servlet/AdminServlet.java +++ b/solr/core/src/java/org/apache/solr/servlet/AdminServlet.java @@ -17,21 +17,27 @@ 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.List; 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 excludes; +/** + * WARNING. NOT A REAL CLASS (yet) + * + * This class is NOT yet functional it's serving as a foil for other development anything and everything + * might change here and it presently completes without doing anything useful at all. It might also + * be abandoned/deleted in the future too. No guarantees + */ +//@WebServlet //<=== uncomment for testing purposes only +public class AdminServlet extends HttpServlet implements PathExcluder { + private List excludes; @Override public void init() throws ServletException { @@ -65,7 +71,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S } @Override - public void setExcludePatterns(ArrayList excludePatterns) { + public void setExcludePatterns(List excludePatterns) { this.excludes = excludePatterns; } } diff --git a/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java b/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java index 8c3d8ef5718..c7eee2004ab 100644 --- a/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java +++ b/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java @@ -16,8 +16,464 @@ */ 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; -public interface CoreContainerProvider { - CoreContainer getCores(); +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 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 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: + *

    + *
  1. attribute in the FilterConfig
    2. + *
  2. JNDI: via java:comp/env/solr/home
    3. + *
  3. The system property solr.solr.home
    4. + *
  4. Look in the current working directory for a solr/ directory
    5. + *
+ *

+ * + * @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 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); + } + } + + + static class ServiceHolder { + private volatile CoreContainerProvider service; + private volatile ContextInitializationKey key; + + private ServiceHolder(ContextInitializationKey key) { + if (key == null) { + throw new IllegalArgumentException("Key for accessing this service holder must be supplied"); + } + this.key = key; + } + + public void setService(CoreContainerProvider service) { + this.service = service; + key.makeReady(); + key = null; // be sure not to hold a reference to the context via the key + } + + public CoreContainerProvider getService() { + try { + if (key != null) { + try { + key.waitForReadyService(); + } catch (NullPointerException e) { + // ignore, means we raced with set service and lost, but that's fine since null implies we are ready. + } + } + } catch (InterruptedException e) { + throw new SolrException(ErrorCode.SERVER_ERROR,"Interrupted while obtaining reference to CoreService"); + } + return service; + } + } } diff --git a/solr/core/src/java/org/apache/solr/servlet/CoreService.java b/solr/core/src/java/org/apache/solr/servlet/CoreService.java deleted file mode 100644 index 11e948f5c29..00000000000 --- a/solr/core/src/java/org/apache/solr/servlet/CoreService.java +++ /dev/null @@ -1,451 +0,0 @@ -/* - * 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 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: - *

    - *
  1. attribute in the FilterConfig
    2. - *
  2. JNDI: via java:comp/env/solr/home
    3. - *
  3. The system property solr.solr.home
    4. - *
  4. Look in the current working directory for a solr/ directory
    5. - *
- *

- * - * @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 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); - } - } - - - static class ServiceHolder { - private volatile CoreService service; - private volatile ContextInitializationKey key; - - private ServiceHolder(ContextInitializationKey key) { - if (key == null) { - throw new IllegalArgumentException("Key for accessing this service holder must be supplied"); - } - this.key = key; - } - - public void setService(CoreService service) { - this.service = service; - key.makeReady(); - key = null; // be sure not to hold a reference to the context via the key - } - - public CoreService getService() { - try { - if (key != null) { - try { - key.waitForReadyService(); - } catch (NullPointerException e) { - // ignore, means we raced with set service and lost, but that's fine since null implies we are ready. - } - } - } catch (InterruptedException e) { - throw new SolrException(ErrorCode.SERVER_ERROR,"Interrupted while obtaining reference to CoreService"); - } - return service; - } - } -} diff --git a/solr/core/src/java/org/apache/solr/servlet/ExceptionWhileTracing.java b/solr/core/src/java/org/apache/solr/servlet/ExceptionWhileTracing.java index 2a7aa427845..c01d3d0c3f8 100644 --- a/solr/core/src/java/org/apache/solr/servlet/ExceptionWhileTracing.java +++ b/solr/core/src/java/org/apache/solr/servlet/ExceptionWhileTracing.java @@ -16,6 +16,9 @@ */ package org.apache.solr.servlet; +/* + * This is not pretty, hope to remove it when tracing becomes a filter. + */ public class ExceptionWhileTracing extends RuntimeException { public Exception e; diff --git a/solr/core/src/java/org/apache/solr/servlet/PathExcluder.java b/solr/core/src/java/org/apache/solr/servlet/PathExcluder.java index 26653dc9084..fef696da490 100644 --- a/solr/core/src/java/org/apache/solr/servlet/PathExcluder.java +++ b/solr/core/src/java/org/apache/solr/servlet/PathExcluder.java @@ -16,9 +16,13 @@ */ package org.apache.solr.servlet; -import java.util.ArrayList; +import java.util.List; import java.util.regex.Pattern; +/** + * Denotes an object, usually a servlet that denies access to some paths based on the supplied patterns. + * Typically, this would be implemented via compiled regular expressions. + */ public interface PathExcluder { - void setExcludePatterns(ArrayList excludePatterns); + void setExcludePatterns(List excludePatterns); } diff --git a/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java b/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java index 5a8f1478d6d..373f72a7197 100644 --- a/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java +++ b/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java @@ -46,6 +46,7 @@ import java.io.OutputStream; import java.lang.invoke.MethodHandles; import java.util.ArrayList; +import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -143,7 +144,7 @@ public void close() { } } - static boolean excludedPath(ArrayList excludePatterns, HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { + static boolean excludedPath(List excludePatterns, HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { String requestPath = getPathAfterContext(request); // No need to even create the HttpSolrCall object if this path is excluded. if (excludePatterns != null) { diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrCoreUtils.java b/solr/core/src/java/org/apache/solr/servlet/SolrCoreUtils.java deleted file mode 100644 index 3d7a7a59c27..00000000000 --- a/solr/core/src/java/org/apache/solr/servlet/SolrCoreUtils.java +++ /dev/null @@ -1,63 +0,0 @@ -/* - * 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 { - private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - /** - * Get the NodeConfig whether stored on disk, in ZooKeeper, etc. - * This may also be used by custom filters to load relevant configuration. - * @return the NodeConfig - */ - public static NodeConfig loadNodeConfig(Path solrHome, Properties nodeProperties) { - if (!StringUtils.isEmpty(System.getProperty("solr.solrxml.location"))) { - log.warn("Solr property solr.solrxml.location is no longer supported. Will automatically load solr.xml from ZooKeeper if it exists"); - } - nodeProperties = SolrXmlConfig.wrapAndSetZkHostFromSysPropIfNeeded(nodeProperties); - String zkHost = nodeProperties.getProperty(SolrXmlConfig.ZK_HOST); - if (!StringUtils.isEmpty(zkHost)) { - int startUpZkTimeOut = Integer.getInteger("waitForZk", 30); - startUpZkTimeOut *= 1000; - try (SolrZkClient zkClient = new SolrZkClient(zkHost, startUpZkTimeOut, startUpZkTimeOut)) { - if (zkClient.exists("/solr.xml", true)) { - log.info("solr.xml found in ZooKeeper. Loading..."); - byte[] data = zkClient.getData("/solr.xml", null, null, true); - return SolrXmlConfig.fromInputStream(solrHome, new ByteArrayInputStream(data), nodeProperties, true); - } - } catch (Exception e) { - throw new SolrException(ErrorCode.SERVER_ERROR, "Error occurred while loading solr.xml from zookeeper", e); - } - log.info("Loading solr.xml from SolrHome (not found in ZooKeeper)"); - } - - return SolrXmlConfig.fromSolrHome(solrHome, nodeProperties); - } -} diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java index e7a028369c1..a68281fa166 100644 --- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java +++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java @@ -32,7 +32,7 @@ import org.apache.solr.security.AuthenticationPlugin; import org.apache.solr.security.PKIAuthenticationPlugin; import org.apache.solr.security.PublicKeyHandler; -import org.apache.solr.servlet.CoreService.ServiceHolder; +import org.apache.solr.servlet.CoreContainerProvider.ServiceHolder; import org.apache.solr.util.configuration.SSLConfigurationsFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -47,7 +47,7 @@ import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.lang.invoke.MethodHandles; -import java.util.ArrayList; +import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; @@ -80,15 +80,13 @@ public class SolrDispatchFilter extends BaseSolrFilter implements PathExcluder { protected String abortErrorMessage = null; @Override - public void setExcludePatterns(ArrayList excludePatterns) { + public void setExcludePatterns(List excludePatterns) { this.excludePatterns = excludePatterns; } - private ArrayList excludePatterns; - - private final boolean isV2Enabled = !"true".equals(System.getProperty("disable.v2.api", "false")); + private List excludePatterns; - private RateLimitManager rateLimitManager; + private final boolean isV2Enabled = !"true".equals(System.getProperty("disable.v2.api", "false")); public HttpClient getHttpClient() { try { @@ -128,7 +126,7 @@ public SolrDispatchFilter() { @Override public void init(FilterConfig config) throws ServletException { try { - coreService = CoreService.serviceForContext(config.getServletContext()); + coreService = CoreContainerProvider.serviceForContext(config.getServletContext()); SSLConfigurationsFactory.current().init(); if (log.isTraceEnabled()) { log.trace("SolrDispatchFilter.init(): {}", this.getClass().getClassLoader()); @@ -192,13 +190,8 @@ private static Span getSpan(HttpServletRequest req) { private void dispatch(FilterChain chain, HttpServletRequest request, HttpServletResponse response, boolean retry) throws IOException, ServletException, SolrAuthenticationException { - - AtomicReference wrappedRequest = new AtomicReference<>(); - if (!authenticateRequest(request, response, wrappedRequest)) { // the response and status code have already been sent - throw new SolrAuthenticationException(); - } - + authenticateRequest(request, response, wrappedRequest); if (wrappedRequest.get() != null) { request = wrappedRequest.get(); } @@ -259,7 +252,7 @@ protected HttpSolrCall getHttpSolrCall(HttpServletRequest request, HttpServletRe } } - private boolean authenticateRequest(HttpServletRequest request, HttpServletResponse response, final AtomicReference wrappedRequest) throws IOException { + private void authenticateRequest(HttpServletRequest request, HttpServletResponse response, final AtomicReference wrappedRequest) throws IOException,SolrAuthenticationException { boolean requestContinues; final AtomicBoolean isAuthenticated = new AtomicBoolean(false); CoreContainer cores; @@ -273,18 +266,18 @@ private boolean authenticateRequest(HttpServletRequest request, HttpServletRespo if (shouldAudit(EventType.ANONYMOUS)) { cores.getAuditLoggerPlugin().doAudit(new AuditEvent(EventType.ANONYMOUS, request)); } - return true; + return; } else { // /admin/info/key must be always open. see SOLR-9188 String requestPath = ServletUtils.getPathAfterContext(request); if (PublicKeyHandler.PATH.equals(requestPath)) { log.debug("Pass through PKI authentication endpoint"); - return true; + return; } // /solr/ (Admin UI) must be always open to allow displaying Admin UI with login page if ("/solr/".equals(requestPath) || "/".equals(requestPath)) { log.debug("Pass through Admin UI entry point"); - return true; + return; } String header = request.getHeader(PKIAuthenticationPlugin.HEADER); if (header != null && cores.getPkiAuthenticationPlugin() != null) @@ -293,7 +286,17 @@ private boolean authenticateRequest(HttpServletRequest request, HttpServletRespo if (log.isDebugEnabled()) { log.debug("Request to authenticate: {}, domain: {}, port: {}", request, request.getLocalName(), request.getLocalPort()); } - // upon successful authentication, this should call the chain's next filter. + // For legacy reasons, upon successful authentication this wants to call the chain's next filter, which + // obfuscates the layout of the code since one usually expects to be able to find the call to doFilter() + // in the implementation of javax.servlet.Filter. Supplying a trivial impl here to keep existing code happy + // while making the flow clearer. Chain will be called after this method completes. Eventually auth all + // moves to its own filter (hopefully). Most auth plugins simply return true after calling this anyway + // so they obviously don't care. Kerberos plugins seem to mostly use it to satisfy the api of a wrapped + // instance of javax.servlet.Filter and neither of those seem to be doing anything fancy with the filter chain, + // so this would seem to be a hack brought on by the fact that our auth code has been forced to be code + // within dispatch filter, rather than being a filter itself. The HadoopAuthPlugin has a suspicious amount + // of code after the call to doFilter() which seems to imply that anything in this chain can get executed before + // authentication completes, and I can't figure out how that's a good idea in the first place. requestContinues = authenticationPlugin.authenticate(request, response, (req, rsp) -> { isAuthenticated.set(true); wrappedRequest.set((HttpServletRequest) req); @@ -314,12 +317,12 @@ private boolean authenticateRequest(HttpServletRequest request, HttpServletRespo if (shouldAudit(EventType.REJECTED)) { cores.getAuditLoggerPlugin().doAudit(new AuditEvent(EventType.REJECTED, request)); } - return false; + throw new SolrAuthenticationException(); } if (shouldAudit(EventType.AUTHENTICATED)) { cores.getAuditLoggerPlugin().doAudit(new AuditEvent(EventType.AUTHENTICATED, request)); } - return true; + // Auth Success } diff --git a/solr/core/src/test/org/apache/solr/cloud/SolrXmlInZkTest.java b/solr/core/src/test/org/apache/solr/cloud/SolrXmlInZkTest.java index e3437f0ff5f..55fcab8acab 100644 --- a/solr/core/src/test/org/apache/solr/cloud/SolrXmlInZkTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/SolrXmlInZkTest.java @@ -29,7 +29,6 @@ import org.apache.solr.common.cloud.SolrZkClient; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.core.NodeConfig; -import org.apache.solr.servlet.SolrCoreUtils; import org.junit.Rule; import org.junit.Test; import org.junit.rules.RuleChain; @@ -90,7 +89,7 @@ private void setUpZkAndDiskXml(boolean toZk, boolean leaveOnLocal) throws Except props.setProperty("solr.test.sys.prop1", "propone"); props.setProperty("solr.test.sys.prop2", "proptwo"); - cfg = SolrCoreUtils.loadNodeConfig(solrHome, props); + cfg = NodeConfig.loadNodeConfig(solrHome, props); if (log.isInfoEnabled()) { log.info("####SETUP_END {}", getTestName()); } diff --git a/solr/core/src/test/org/apache/solr/core/TestLazyCores.java b/solr/core/src/test/org/apache/solr/core/TestLazyCores.java index e790a825026..81889a8b310 100644 --- a/solr/core/src/test/org/apache/solr/core/TestLazyCores.java +++ b/solr/core/src/test/org/apache/solr/core/TestLazyCores.java @@ -40,7 +40,6 @@ import org.apache.solr.request.LocalSolrQueryRequest; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; -import org.apache.solr.servlet.SolrCoreUtils; import org.apache.solr.update.AddUpdateCommand; import org.apache.solr.update.CommitUpdateCommand; import org.apache.solr.update.UpdateHandler; @@ -94,7 +93,7 @@ private CoreContainer init() throws Exception { copyMinConf(new File(solrHomeDirectory, "collection" + idx)); } - NodeConfig cfg = SolrCoreUtils.loadNodeConfig(solrHomeDirectory.toPath(), null); + NodeConfig cfg = NodeConfig.loadNodeConfig(solrHomeDirectory.toPath(), null); return createCoreContainer(cfg, testCores); } diff --git a/solr/webapp/web/WEB-INF/web.xml b/solr/webapp/web/WEB-INF/web.xml index f4b9544a4b0..e8f51347571 100644 --- a/solr/webapp/web/WEB-INF/web.xml +++ b/solr/webapp/web/WEB-INF/web.xml @@ -22,7 +22,7 @@ metadata-complete="true" > - org.apache.solr.servlet.CoreService + org.apache.solr.servlet.CoreContainerProvider From 7096a0dcc7c1ad70c7b9be304caecbd13c31e9a8 Mon Sep 17 00:00:00 2001 From: Gus Heck Date: Sun, 7 Nov 2021 13:16:57 -0500 Subject: [PATCH 6/9] Cleanup and comments --- .../solrj/embedded/JettySolrRunner.java | 31 +++----- .../org/apache/solr/servlet/AdminServlet.java | 77 ------------------- .../solr/servlet/CoreContainerProvider.java | 37 ++++----- .../org/apache/solr/servlet/ServletUtils.java | 15 +++- .../solr/servlet/SolrDispatchFilter.java | 14 ++-- .../org/apache/solr/util/BaseTestHarness.java | 7 -- solr/webapp/web/WEB-INF/web.xml | 10 --- 7 files changed, 50 insertions(+), 141 deletions(-) delete mode 100644 solr/core/src/java/org/apache/solr/servlet/AdminServlet.java diff --git a/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java b/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java index 03a4beb064b..a2ece25d0a3 100644 --- a/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java +++ b/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java @@ -144,6 +144,7 @@ public class JettySolrRunner { private String host; private volatile boolean started = false; + private CoreContainerProvider coreContainerProvider; public static class DebugFilter implements Filter { @@ -466,6 +467,8 @@ public CoreContainer getCoreContainer() { } return getSolrDispatchFilter().getCores(); } catch (UnavailableException e) { + // Since this is only used in tests, this is just a straight-up failure + // If this is converted for other use something else might be better here throw new RuntimeException(e); } } @@ -640,24 +643,6 @@ public void stop() throws Exception { MDC.clear(); try { Filter filter = dispatchFilter.getFilter(); - - // we want to shutdown outside of jetty cutting us off - SolrDispatchFilter sdf = getSolrDispatchFilter(); - ExecutorService customThreadPool = null; - if (sdf != null) { - customThreadPool = ExecutorUtil.newMDCAwareCachedThreadPool(new SolrNamedThreadFactory("jettyShutDown")); - - sdf.closeOnDestroy(); -// customThreadPool.submit(() -> { -// try { -// sdf.close(); -// } catch (Throwable t) { -// log.error("Error shutting down Solr", t); -// } -// }); - - } - QueuedThreadPool qtp = (QueuedThreadPool) server.getThreadPool(); ReservedThreadExecutor rte = qtp.getBean(ReservedThreadExecutor.class); @@ -695,8 +680,10 @@ public void stop() throws Exception { timeout.waitFor("Timeout waiting for reserved executor to stop.", rte::isStopped); } - if (customThreadPool != null) { - ExecutorUtil.shutdownAndAwaitTermination(customThreadPool); + // we want to shutdown outside of jetty cutting us off + SolrDispatchFilter sdf = getSolrDispatchFilter(); + if (sdf != null) { + ExecutorUtil.shutdownAndAwaitTermination(getJettyShutDownThreadPool()); } do { @@ -720,6 +707,10 @@ public void stop() throws Exception { } } + private ExecutorService getJettyShutDownThreadPool() { + return ExecutorUtil.newMDCAwareCachedThreadPool(new SolrNamedThreadFactory("jettyShutDown")); + } + public void outputMetrics(File outputDirectory, String fileName) throws IOException { if (getCoreContainer() != null) { diff --git a/solr/core/src/java/org/apache/solr/servlet/AdminServlet.java b/solr/core/src/java/org/apache/solr/servlet/AdminServlet.java deleted file mode 100644 index 034578429a6..00000000000 --- a/solr/core/src/java/org/apache/solr/servlet/AdminServlet.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * 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.http.HttpServlet; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import java.io.IOException; -import java.util.List; -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; - -/** - * WARNING. NOT A REAL CLASS (yet) - * - * This class is NOT yet functional it's serving as a foil for other development anything and everything - * might change here and it presently completes without doing anything useful at all. It might also - * be abandoned/deleted in the future too. No guarantees - */ -//@WebServlet //<=== uncomment for testing purposes only -public class AdminServlet extends HttpServlet implements PathExcluder { - private List 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,() -> { - // stuff from HttpSolrCallHere - },false); - // todo: enable tracing - } - - @Override - protected void doPost(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,() -> { - // stuff from HttpSolrCallHere - },false); - //todo: enable tracing - } - - @Override - public void setExcludePatterns(List excludePatterns) { - this.excludes = excludePatterns; - } -} diff --git a/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java b/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java index c7eee2004ab..4628acb2274 100644 --- a/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java +++ b/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java @@ -95,9 +95,9 @@ public class CoreContainerProvider implements ServletContextListener { private static final Map 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). + // todo: dependency injection instead, but for now this method and the associated map will have to suffice. + // Note that this relies on ServletContext.equals() not implementing anything significantly different + // than Object.equals for its .equals method (I've found no implementation that even implements it). public static ServiceHolder serviceForContext(ServletContext ctx) throws InterruptedException { ContextInitializationKey key = new ContextInitializationKey(ctx); return services.computeIfAbsent(key, ServiceHolder::new); @@ -147,20 +147,21 @@ private static void waitForCoreContainer(Supplier provider, Count 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()); - } - } +// 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 if that's really what it is, why we +// // did it and if there's room for improvement. +// +// zkController.publishNodeAsDown(zkController.getNodeName()); +// } +// } cores = null; try { if (metricManager != null) { @@ -188,7 +189,7 @@ public void init(ServletContext servletContext) { } CoreContainer coresInit = null; try { - // "extra" properties must be init'ed first so we know things like "do we have a zkHost" + // "extra" properties must be initialized 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)); diff --git a/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java b/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java index 373f72a7197..e866b08dbe1 100644 --- a/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java +++ b/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java @@ -78,7 +78,8 @@ public static String getPathAfterContext(HttpServletRequest request) { * Wrap the request's input stream with a close shield. If this is a * retry, we will assume that the stream has already been wrapped and do nothing. * - * Only the container should ever actually close the servlet output stream. + * Only the container should ever actually close the servlet output stream. This method possibly + * should be turned into a servlet filter * * @param request The request to wrap. * @param retry If this is an original request or a retry. @@ -176,6 +177,15 @@ static void configExcludes(PathExcluder excluder, String patternConfig) { } } + /** + * Enforces rate limiting for a request. Should be converted to a servlet filter at some point. Currently, + * this is tightly coupled with request tracing which is not ideal either. + * + * @param request The request to limit + * @param response The associated response + * @param limitedExecution code that will be traced + * @param trace a boolean that turns tracing on or off + */ static void rateLimitRequest(HttpServletRequest request, HttpServletResponse response, Runnable limitedExecution, boolean trace) throws ServletException, IOException { boolean accepted = false; RateLimitManager rateLimitManager = getRateLimitManager(request); @@ -203,7 +213,7 @@ static void rateLimitRequest(HttpServletRequest request, HttpServletResponse res } /** - * Sets up tracing for an HTTP request. + * Sets up tracing for an HTTP request. Perhaps should be converted to a servlet filter at some point. * * @param tracedExecution the executed code */ @@ -284,6 +294,7 @@ protected static Span buildSpan(Tracer tracer, HttpServletRequest request) { private static void consumeInputFully(HttpServletRequest req, HttpServletResponse response) { try { ServletInputStream is = req.getInputStream(); + //noinspection StatementWithEmptyBody while (!is.isFinished() && is.read() != -1) {} } catch (IOException e) { if (req.getHeader(HttpHeaders.EXPECT) != null && response.isCommitted()) { diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java index a68281fa166..ffa9ece10a7 100644 --- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java +++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java @@ -64,7 +64,8 @@ * @since solr 1.2 */ // todo: get rid of this class entirely! Request dispatch is the container's responsibility. Much of what we have here -// should be several separate but composable servlet Filters, and a ServletContextListener for startup/shutdown +// should be several separate but composable servlet Filters, wrapping multiple servlets that are more focused in +// scope. This should become possible now that we have a ServletContextListener for startup/shutdown of CoreContainer // that sets up a service from which things like CoreContainer can be requested. (or better yet injected) public class SolrDispatchFilter extends BaseSolrFilter implements PathExcluder { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); @@ -72,7 +73,8 @@ public class SolrDispatchFilter extends BaseSolrFilter implements PathExcluder { public static final String ATTR_TRACING_TRACER = Tracer.class.getName(); public static final String ATTR_RATELIMIT_MANAGER = RateLimitManager.class.getName(); - // TODO: see if we can get rid of the holder here (Servlet spec actually guarantees ContextListeners run before filter init) + // TODO: see if we can get rid of the holder here (Servlet spec actually guarantees ContextListeners run + // before filter init, but JettySolrRunner that we use for tests is complicated) private ServiceHolder coreService; protected final CountDownLatch init = new CountDownLatch(1); @@ -252,6 +254,7 @@ protected HttpSolrCall getHttpSolrCall(HttpServletRequest request, HttpServletRe } } + // TODO: make this a servlet filter private void authenticateRequest(HttpServletRequest request, HttpServletResponse response, final AtomicReference wrappedRequest) throws IOException,SolrAuthenticationException { boolean requestContinues; final AtomicBoolean isAuthenticated = new AtomicBoolean(false); @@ -289,8 +292,8 @@ private void authenticateRequest(HttpServletRequest request, HttpServletResponse // For legacy reasons, upon successful authentication this wants to call the chain's next filter, which // obfuscates the layout of the code since one usually expects to be able to find the call to doFilter() // in the implementation of javax.servlet.Filter. Supplying a trivial impl here to keep existing code happy - // while making the flow clearer. Chain will be called after this method completes. Eventually auth all - // moves to its own filter (hopefully). Most auth plugins simply return true after calling this anyway + // while making the flow clearer. Chain will be called after this method completes. Eventually auth all + // moves to its own filter (hopefully). Most auth plugins simply return true after calling this anyway, // so they obviously don't care. Kerberos plugins seem to mostly use it to satisfy the api of a wrapped // instance of javax.servlet.Filter and neither of those seem to be doing anything fancy with the filter chain, // so this would seem to be a hack brought on by the fact that our auth code has been forced to be code @@ -340,9 +343,6 @@ private boolean shouldAudit(AuditEvent.EventType eventType) { return cores.getAuditLoggerPlugin() != null && cores.getAuditLoggerPlugin().shouldLog(eventType); } - public void closeOnDestroy() { - } - @VisibleForTesting void replaceRateLimitManager(RateLimitManager rateLimitManager) { coreService.getService().setRateLimitManager(rateLimitManager); diff --git a/solr/test-framework/src/java/org/apache/solr/util/BaseTestHarness.java b/solr/test-framework/src/java/org/apache/solr/util/BaseTestHarness.java index 33f439db7fd..2f825e33e99 100644 --- a/solr/test-framework/src/java/org/apache/solr/util/BaseTestHarness.java +++ b/solr/test-framework/src/java/org/apache/solr/util/BaseTestHarness.java @@ -79,19 +79,12 @@ public static String validateXPath(String xml, String... tests) Document document; try { -// if (xml.startsWith("")) { -// // probably got an error page -// System.err.println(xml); -// } document = getXmlDocumentBuilder().parse(new ByteArrayInputStream (xml.getBytes(StandardCharsets.UTF_8))); } catch (UnsupportedEncodingException e1) { throw new RuntimeException("Totally weird UTF-8 exception", e1); } catch (IOException e2) { throw new RuntimeException("Totally weird io exception", e2); -// } catch (SAXParseException spe) { -// System.err.println(xml); -// throw spe; } for (String xp : tests) { diff --git a/solr/webapp/web/WEB-INF/web.xml b/solr/webapp/web/WEB-INF/web.xml index e8f51347571..33cad1f47e2 100644 --- a/solr/webapp/web/WEB-INF/web.xml +++ b/solr/webapp/web/WEB-INF/web.xml @@ -44,16 +44,6 @@ /* - - AdminServlet - org.apache.solr.servlet.AdminServlet - - - - AdminServlet - /admin2 - - LoadAdminUI org.apache.solr.servlet.LoadAdminUiServlet From 6363f205993ad764be50d9cee59609c4a5375f17 Mon Sep 17 00:00:00 2001 From: Gus Heck Date: Sun, 7 Nov 2021 16:43:32 -0500 Subject: [PATCH 7/9] Address a couple sonatype-lift warnings --- .../apache/solr/client/solrj/embedded/JettySolrRunner.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java b/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java index a2ece25d0a3..63df1f3d50f 100644 --- a/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java +++ b/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java @@ -368,7 +368,7 @@ public void lifeCycleStopping(LifeCycle arg0) { } @Override - public void lifeCycleStopped(LifeCycle arg0) { + public synchronized void lifeCycleStopped(LifeCycle arg0) { coreContainerProvider.close(); } @@ -378,7 +378,7 @@ public void lifeCycleStarting(LifeCycle arg0) { } @Override - public void lifeCycleStarted(LifeCycle arg0) { + public synchronized void lifeCycleStarted(LifeCycle arg0) { jettyPort = getFirstConnectorPort(); int port = jettyPort; if (proxyPort != -1) port = proxyPort; From 6f49f80e69ff53bc161b326fdf32303ea7c52c98 Mon Sep 17 00:00:00 2001 From: Gus Heck Date: Sun, 7 Nov 2021 20:21:10 -0500 Subject: [PATCH 8/9] More sonatype-lift inspired thread safety touch-up --- .../solr/client/solrj/embedded/JettySolrRunner.java | 6 +++--- .../org/apache/solr/servlet/CoreContainerProvider.java | 8 ++------ 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java b/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java index 63df1f3d50f..ec8d3c77f0a 100644 --- a/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java +++ b/solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java @@ -115,7 +115,7 @@ public class JettySolrRunner { // NOTE: needs to be larger than SolrHttpClient.threadPoolSweeperMaxIdleTime private static final int THREAD_POOL_MAX_IDLE_TIME_MS = 260000; - Server server; + private Server server; volatile FilterHolder dispatchFilter; volatile FilterHolder debugFilter; @@ -512,7 +512,7 @@ public void start() throws Exception { * * @throws Exception if an error occurs on startup */ - public void start(boolean reusePort) throws Exception { + public synchronized void start(boolean reusePort) throws Exception { // Do not let Jetty/Solr pollute the MDC for this thread Map prevContext = MDC.getCopyOfContextMap(); MDC.clear(); @@ -637,7 +637,7 @@ Exception lookForBindException(IOException ioe) { * * @throws Exception if an error occurs on shutdown */ - public void stop() throws Exception { + public synchronized void stop() throws Exception { // Do not let Jetty/Solr pollute the MDC for this thread Map prevContext = MDC.getCopyOfContextMap(); MDC.clear(); diff --git a/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java b/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java index 4628acb2274..08c05a3a6c8 100644 --- a/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java +++ b/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java @@ -399,8 +399,8 @@ public boolean isV2Enabled() { } private static class ContextInitializationKey { - private ServletContext ctx; - volatile CountDownLatch initializing = new CountDownLatch(1); + private final ServletContext ctx; + private final CountDownLatch initializing = new CountDownLatch(1); private ContextInitializationKey(ServletContext ctx) { if (ctx == null) { @@ -417,10 +417,6 @@ public synchronized ServletContext getCtx() { return ctx; } - public synchronized void setCtx(ServletContext ctx) { - this.ctx = ctx; - } - synchronized void makeReady() { this.initializing.countDown(); } From feeb89eda1826346ec6741c570cab8500b1cf950 Mon Sep 17 00:00:00 2001 From: Gus Heck Date: Mon, 8 Nov 2021 20:25:51 -0500 Subject: [PATCH 9/9] Touch up ArrayList usage --- solr/core/src/java/org/apache/solr/servlet/ServletUtils.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java b/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java index e866b08dbe1..9e75d7250a6 100644 --- a/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java +++ b/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java @@ -162,14 +162,14 @@ static boolean excludedPath(List excludePatterns, HttpServletRequest re return false; } - static boolean excludedPath(ArrayList excludePatterns, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { + static boolean excludedPath(List excludePatterns, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { return excludedPath(excludePatterns,request,response, null); } static void configExcludes(PathExcluder excluder, String patternConfig) { if(patternConfig != null) { String[] excludeArray = patternConfig.split(","); - ArrayList patterns = new ArrayList<>(); + List patterns = new ArrayList<>(); excluder.setExcludePatterns(patterns); for (String element : excludeArray) { patterns.add(Pattern.compile(element));