diff --git a/changelog/unreleased/SOLR-18046.yml b/changelog/unreleased/SOLR-18046.yml new file mode 100644 index 000000000000..6c03a3b395ad --- /dev/null +++ b/changelog/unreleased/SOLR-18046.yml @@ -0,0 +1,8 @@ +# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc +title: Ratelimiting now exists in it's own Servlet filter which can be customized or removed. +type: other # added, changed, fixed, deprecated, removed, dependency_update, security, other +authors: + - name: Gus Heck +links: + - name: SOLR-18046 + url: https://issues.apache.org/jira/browse/SOLR-18046 diff --git a/solr/core/src/java/org/apache/solr/core/RateLimiterConfig.java b/solr/core/src/java/org/apache/solr/core/RateLimiterConfig.java index b3c00cf4cf07..e0444df43759 100644 --- a/solr/core/src/java/org/apache/solr/core/RateLimiterConfig.java +++ b/solr/core/src/java/org/apache/solr/core/RateLimiterConfig.java @@ -17,14 +17,14 @@ package org.apache.solr.core; -import static org.apache.solr.servlet.RateLimitManager.DEFAULT_CONCURRENT_REQUESTS; -import static org.apache.solr.servlet.RateLimitManager.DEFAULT_SLOT_ACQUISITION_TIMEOUT_MS; - import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.request.beans.RateLimiterPayload; public class RateLimiterConfig { public static final String RL_CONFIG_KEY = "rate-limiters"; + public static final int DEFAULT_CONCURRENT_REQUESTS = + (Runtime.getRuntime().availableProcessors()) * 3; + public static final long DEFAULT_SLOT_ACQUISITION_TIMEOUT_MS = -1; public final SolrRequest.SolrRequestType requestType; public final boolean isEnabled; diff --git a/solr/core/src/java/org/apache/solr/servlet/CoreContainerAwareHttpFilter.java b/solr/core/src/java/org/apache/solr/servlet/CoreContainerAwareHttpFilter.java index bfddf0357b72..36226ccb0016 100644 --- a/solr/core/src/java/org/apache/solr/servlet/CoreContainerAwareHttpFilter.java +++ b/solr/core/src/java/org/apache/solr/servlet/CoreContainerAwareHttpFilter.java @@ -33,7 +33,6 @@ public abstract class CoreContainerAwareHttpFilter extends HttpFilter { @Override public void init(FilterConfig config) throws ServletException { - super.init(config); containerProvider = CoreContainerProvider.serviceForContext(config.getServletContext()); if (log.isTraceEnabled()) { log.trace("{}.init(): {}", this.getClass().getName(), this.getClass().getClassLoader()); @@ -47,13 +46,4 @@ public void init(FilterConfig config) throws ServletException { public CoreContainer getCores() throws UnavailableException { return containerProvider.getCoreContainer(); } - - // TODO: not fond of having these here, but RateLimiter initialization can be sorted out later - RateLimitManager getRateLimitManager() { - return containerProvider.getRateLimitManager(); - } - - void replaceRateLimitManager(RateLimitManager rlm) { - containerProvider.setRateLimitManager(rlm); - } } 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 d7e6adad9422..c718ef0e569f 100644 --- a/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java +++ b/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java @@ -23,7 +23,6 @@ import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_LEVEL; import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_MUTECONSOLE; -import com.google.common.annotations.VisibleForTesting; import jakarta.servlet.ServletContext; import jakarta.servlet.ServletContextEvent; import jakarta.servlet.ServletContextListener; @@ -45,16 +44,13 @@ import org.apache.lucene.store.MMapDirectory; import org.apache.lucene.util.VectorUtil; import org.apache.solr.client.api.util.SolrVersion; -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.EnvUtils; import org.apache.solr.core.CoreContainer; import org.apache.solr.core.NodeConfig; import org.apache.solr.core.SolrCore; import org.apache.solr.core.SolrXmlConfig; -import org.apache.solr.servlet.RateLimitManager.Builder; import org.apache.solr.util.StartupLoggingUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -67,8 +63,6 @@ public class CoreContainerProvider implements ServletContextListener { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private CoreContainer cores; - // TODO: this probably should not live here... - private RateLimitManager rateLimitManager; /** * Acquires an instance from the context. Never null. @@ -187,21 +181,6 @@ private void init(ServletContext servletContext) { coresInit = createCoreContainer(computeSolrHome(servletContext), extraProperties); - 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")); } @@ -364,13 +343,4 @@ protected CoreContainer createCoreContainer(Path solrHome, Properties nodeProps) coreContainer.load(); return coreContainer; } - - public RateLimitManager getRateLimitManager() { - return rateLimitManager; - } - - @VisibleForTesting - void setRateLimitManager(RateLimitManager rateLimitManager) { - this.rateLimitManager = rateLimitManager; - } } 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 24b0d6e10636..1c381f06f8cb 100644 --- a/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java +++ b/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java @@ -16,7 +16,7 @@ */ package org.apache.solr.servlet; -import static org.apache.solr.servlet.EssentialSolrRequestFilter.CORE_CONTAINER_REQUEST_ATTRIBUTE; +import static org.apache.solr.servlet.RequiredSolrRequestFilter.CORE_CONTAINER_REQUEST_ATTRIBUTE; import com.google.common.net.HttpHeaders; import jakarta.servlet.http.HttpServlet; diff --git a/solr/core/src/java/org/apache/solr/servlet/RateLimitFilter.java b/solr/core/src/java/org/apache/solr/servlet/RateLimitFilter.java new file mode 100644 index 000000000000..6f0d301e086e --- /dev/null +++ b/solr/core/src/java/org/apache/solr/servlet/RateLimitFilter.java @@ -0,0 +1,76 @@ +/* + * 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.google.common.annotations.VisibleForTesting; +import jakarta.servlet.FilterChain; +import jakarta.servlet.FilterConfig; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import java.io.IOException; +import org.apache.solr.common.SolrException; + +public class RateLimitFilter extends CoreContainerAwareHttpFilter { + private RateLimitManager rateLimitManager; + + @Override + public void init(FilterConfig config) throws ServletException { + super.init(config); + RateLimitManager.Builder builder = new RateLimitManager.Builder(); + builder.withZk(getCores().getZkController()); + this.rateLimitManager = builder.build(); + } + + @Override + protected void doFilter(HttpServletRequest req, HttpServletResponse res, FilterChain chain) + throws IOException, ServletException { + RateLimitManager rateLimitManager = getRateLimitManager(); + try (RequestRateLimiter.SlotReservation accepted = rateLimitManager.handleRequest(req)) { + if (accepted == null) { + res.sendError( + SolrException.ErrorCode.TOO_MANY_REQUESTS.code, RateLimitManager.ERROR_MESSAGE); + return; + } + // todo: this shouldn't be required, tracing and rate limiting should be independently + // composable + ServletUtils.traceHttpRequestExecution2( + req, + res, + () -> { + try { + chain.doFilter(req, res); + } catch (Exception e) { + throw new ExceptionWhileTracing(e); + } + }); + } catch (InterruptedException e1) { + Thread.currentThread().interrupt(); + throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e1.getMessage()); + } + } + + public RateLimitManager getRateLimitManager() { + return rateLimitManager; + } + + @VisibleForTesting + void setRateLimitManager(RateLimitManager rateLimitManager) { + this.rateLimitManager = rateLimitManager; + } +} diff --git a/solr/core/src/java/org/apache/solr/servlet/RateLimitManager.java b/solr/core/src/java/org/apache/solr/servlet/RateLimitManager.java index 3a5da1f39e95..8ca04c086a3f 100644 --- a/solr/core/src/java/org/apache/solr/servlet/RateLimitManager.java +++ b/solr/core/src/java/org/apache/solr/servlet/RateLimitManager.java @@ -28,8 +28,8 @@ import java.util.concurrent.ConcurrentHashMap; import net.jcip.annotations.ThreadSafe; import org.apache.solr.client.solrj.SolrRequest; +import org.apache.solr.cloud.ZkController; import org.apache.solr.common.cloud.ClusterPropertiesListener; -import org.apache.solr.common.cloud.SolrZkClient; import org.apache.solr.core.RateLimiterConfig; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -49,9 +49,6 @@ public class RateLimitManager implements ClusterPropertiesListener { public static final String ERROR_MESSAGE = "Too many requests for this request type. Please try after some time or increase the quota for this request type"; - public static final int DEFAULT_CONCURRENT_REQUESTS = - (Runtime.getRuntime().availableProcessors()) * 3; - public static final long DEFAULT_SLOT_ACQUISITION_TIMEOUT_MS = -1; private final ConcurrentHashMap requestRateLimiterMap; public RateLimitManager() { @@ -175,18 +172,24 @@ public RequestRateLimiter getRequestRateLimiter(SolrRequest.SolrRequestType requ } public static class Builder { - protected SolrZkClient solrZkClient; + protected ZkController solrZk; - public Builder(SolrZkClient solrZkClient) { - this.solrZkClient = solrZkClient; + public Builder() {} + + @SuppressWarnings("UnusedReturnValue") + public Builder withZk(ZkController zkClient) { + solrZk = zkClient; + return this; } public RateLimitManager build() { RateLimitManager rateLimitManager = new RateLimitManager(); - rateLimitManager.registerRequestRateLimiter( - new QueryRateLimiter(solrZkClient), SolrRequest.SolrRequestType.QUERY); - + new QueryRateLimiter(solrZk == null ? null : solrZk.getZkClient()), + SolrRequest.SolrRequestType.QUERY); + if (solrZk != null) { + solrZk.zkStateReader.registerClusterPropertiesListener(rateLimitManager); + } return rateLimitManager; } } diff --git a/solr/core/src/java/org/apache/solr/servlet/EssentialSolrRequestFilter.java b/solr/core/src/java/org/apache/solr/servlet/RequiredSolrRequestFilter.java similarity index 78% rename from solr/core/src/java/org/apache/solr/servlet/EssentialSolrRequestFilter.java rename to solr/core/src/java/org/apache/solr/servlet/RequiredSolrRequestFilter.java index 076d7caae6ce..50ec9decdc91 100644 --- a/solr/core/src/java/org/apache/solr/servlet/EssentialSolrRequestFilter.java +++ b/solr/core/src/java/org/apache/solr/servlet/RequiredSolrRequestFilter.java @@ -35,10 +35,10 @@ * It is expected that solr will fail to function as intended without the conditions initialized in * this filter. Before adding anything to this filter ask yourself if a user could run without the * feature you are adding (either with an alternative implementation or without it at all to reduce - * cpu/memory/dependencies). If it is not essential, it should have its own separate filter. Also, - * please include a comment indicating why the thing added is essential. + * cpu/memory/dependencies). If it is not required, it should have its own separate filter. Also, + * please include a comment indicating why the thing added here is required. */ -public class EssentialSolrRequestFilter extends CoreContainerAwareHttpFilter { +public class RequiredSolrRequestFilter extends CoreContainerAwareHttpFilter { // Best to put constant here because solr is not supposed to be functional (or compile) // without this filter. @@ -55,25 +55,22 @@ protected void doFilter(HttpServletRequest req, HttpServletResponse res, FilterC // this autocloseable is here to invoke MDCSnapshot.close() which restores captured state try (var mdcSnapshot = MDCSnapshot.create()) { - // MDC logging *shouldn't* be essential but currently is, see SOLR-18050. - // Our use of SLF4J indicates that we intend logging to be pluggable, and - // some implementations won't have an MDC, so having it as essential limits - // logging implementations. + // MDC logging *shouldn't* be required but currently is, see SOLR-18050. log.trace("MDC snapshot recorded {}", mdcSnapshot); // avoid both compiler and ide warning. MDCLoggingContext.reset(); MDCLoggingContext.setNode(getCores()); - // This is essential to accommodate libraries that (annoyingly) use + // This is required to accommodate libraries that (annoyingly) use // Thread.currentThread().getContextClassLoader() Thread.currentThread().setContextClassLoader(getCores().getResourceLoader().getClassLoader()); // set a request timer which can be reused by requests if needed - // Request Timer is essential for QueryLimits functionality as well as + // Request Timer is required for QueryLimits functionality as well as // timing our requests. req.setAttribute(SolrRequestParsers.REQUEST_TIMER_SERVLET_ATTRIBUTE, new RTimerTree()); // put the core container in request attribute - // This is essential for the LoadAdminUiServlet class. Removing it will cause 404 + // This is required for the LoadAdminUiServlet class. Removing it will cause 404 req.setAttribute(CORE_CONTAINER_REQUEST_ATTRIBUTE, getCores()); chain.doFilter(req, res); } finally { @@ -81,15 +78,15 @@ protected void doFilter(HttpServletRequest req, HttpServletResponse res, FilterC MDCLoggingContext.reset(); Thread.currentThread().setContextClassLoader(contextClassLoader); - // This is an essential safety valve to ensure we don't accidentally bleed information + // This is a required safety valve to ensure we don't accidentally bleed information // between requests. SolrRequestInfo.reset(); if (!req.isAsyncStarted()) { // jetty's proxy uses this - // essential to avoid SOLR-8453 and SOLR-8683 + // required to avoid SOLR-8453 and SOLR-8683 ServletUtils.consumeInputFully(req, res); - // essential to remove temporary files created during multipart requests. + // required to remove temporary files created during multipart requests. SolrRequestParsers.cleanupMultipartFiles(req); } } 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 f63a16187d25..97af38d8b274 100644 --- a/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java +++ b/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java @@ -32,8 +32,6 @@ import java.io.InputStream; import java.io.OutputStream; import java.lang.invoke.MethodHandles; -import org.apache.solr.common.SolrException; -import org.apache.solr.common.SolrException.ErrorCode; import org.apache.solr.common.util.Utils; import org.apache.solr.logging.MDCLoggingContext; import org.apache.solr.util.tracing.TraceUtils; @@ -126,34 +124,6 @@ public void close() { }; } - /** - * 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 - */ - static void rateLimitRequest( - RateLimitManager rateLimitManager, - HttpServletRequest request, - HttpServletResponse response, - Runnable limitedExecution) - throws ServletException, IOException { - try (RequestRateLimiter.SlotReservation accepted = rateLimitManager.handleRequest(request)) { - if (accepted == null) { - response.sendError(ErrorCode.TOO_MANY_REQUESTS.code, RateLimitManager.ERROR_MESSAGE); - return; - } - // todo: this shouldn't be required, tracing and rate limiting should be independently - // composable - traceHttpRequestExecution2(request, response, limitedExecution); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new SolrException(ErrorCode.SERVER_ERROR, e.getMessage()); - } - } - /** * Sets up tracing for an HTTP request. Perhaps should be converted to a servlet filter at some * point. @@ -162,7 +132,7 @@ static void rateLimitRequest( * @param response The associated response * @param tracedExecution the executed code */ - private static void traceHttpRequestExecution2( + static void traceHttpRequestExecution2( HttpServletRequest request, HttpServletResponse response, Runnable tracedExecution) throws ServletException, IOException { Context context = TraceUtils.extractContext(request); @@ -181,10 +151,6 @@ private static void traceHttpRequestExecution2( } tracedExecution.run(); } catch (ExceptionWhileTracing e) { - if (e.e instanceof SolrAuthenticationException) { - // done, the response and status code have already been sent - return; - } if (e.e instanceof ServletException) { throw (ServletException) e.e; } 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 43de6e2bd074..0d0bef9b6ce1 100644 --- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java +++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java @@ -18,7 +18,6 @@ import static org.apache.solr.servlet.ServletUtils.closeShield; import static org.apache.solr.util.tracing.TraceUtils.getSpan; -import static org.apache.solr.util.tracing.TraceUtils.setTracer; import jakarta.servlet.FilterChain; import jakarta.servlet.FilterConfig; @@ -125,7 +124,8 @@ public void init(FilterConfig config) throws ServletException { public void doFilter(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { // internal version of doFilter that tracks if we are in a retry - doFilterRetry(closeShield(request), closeShield(response), chain, false); + + dispatch(chain, closeShield(request), closeShield(response), false); } /* @@ -140,30 +140,25 @@ public void doFilter(HttpServletRequest request, HttpServletResponse response, F before adding anything else to it. */ - private void doFilterRetry( - HttpServletRequest request, HttpServletResponse response, FilterChain chain, boolean retry) - throws IOException, ServletException { - setTracer(request, getCores().getTracer()); - RateLimitManager rateLimitManager = getRateLimitManager(); - ServletUtils.rateLimitRequest( - rateLimitManager, - request, - response, - () -> { - try { - dispatch(chain, request, response, retry); - } catch (IOException | ServletException | SolrAuthenticationException e) { - throw new ExceptionWhileTracing(e); - } - }); - } - private void dispatch( FilterChain chain, HttpServletRequest request, HttpServletResponse response, boolean retry) - throws IOException, ServletException, SolrAuthenticationException { + throws IOException, ServletException { AtomicReference wrappedRequest = new AtomicReference<>(); - authenticateRequest(request, response, wrappedRequest); + try { + authenticateRequest(request, response, wrappedRequest); + } catch (SolrAuthenticationException e) { + // it seems our auth system expects the plugin to set the status on the request. + // If this hasn't happened make sure it does happen now rather than throwing an + // exception, that formerly went on to be ignored in + // org.apache.solr.servlet.ServletUtils.traceHttpRequestExecution2 + if (response.getStatus() < 400) { + log.error( + "Authentication Plugin threw SolrAuthenticationException without setting request status >= 400"); + response.sendError(401, "Authentication Plugin rejected credentials."); + } + return; // Nothing more to do, chain.doFilter(req,res) doesn't get called. + } if (wrappedRequest.get() != null) { request = wrappedRequest.get(); } @@ -193,7 +188,8 @@ private void dispatch( break; case RETRY: span.addEvent("SolrDispatchFilter RETRY"); - doFilterRetry(request, response, chain, true); // RECURSION + // RECURSION + dispatch(chain, request, response, true); break; case FORWARD: span.addEvent("SolrDispatchFilter FORWARD"); diff --git a/solr/core/src/java/org/apache/solr/util/tracing/TraceUtils.java b/solr/core/src/java/org/apache/solr/util/tracing/TraceUtils.java index 645a412f6c1b..9936dfbb2320 100644 --- a/solr/core/src/java/org/apache/solr/util/tracing/TraceUtils.java +++ b/solr/core/src/java/org/apache/solr/util/tracing/TraceUtils.java @@ -30,7 +30,9 @@ import java.util.List; import java.util.function.Consumer; import java.util.function.Predicate; +import org.apache.solr.core.CoreContainer; import org.apache.solr.request.SolrQueryRequest; +import org.apache.solr.servlet.RequiredSolrRequestFilter; import org.eclipse.jetty.client.Request; /** Utilities for distributed tracing. */ @@ -134,12 +136,12 @@ public static Span getSpan(HttpServletRequest req) { return (Span) req.getAttribute(REQ_ATTR_TRACING_SPAN); } - public static void setTracer(HttpServletRequest req, Tracer t) { - req.setAttribute(REQ_ATTR_TRACING_TRACER, t); - } - public static Tracer getTracer(HttpServletRequest req) { - return (Tracer) req.getAttribute(REQ_ATTR_TRACING_TRACER); + // This attribute is required to be not null, this method should only be called after + // requiredSolrRequestFilter has invoked chain.doFilter(req, res) + return ((CoreContainer) + req.getAttribute(RequiredSolrRequestFilter.CORE_CONTAINER_REQUEST_ATTRIBUTE)) + .getTracer(); } public static Context extractContext(HttpServletRequest req) { diff --git a/solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java b/solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java index fd29c164b02b..1c7c49b1c418 100644 --- a/solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java +++ b/solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java @@ -19,7 +19,7 @@ import static org.apache.solr.common.params.CommonParams.SOLR_REQUEST_CONTEXT_PARAM; import static org.apache.solr.common.params.CommonParams.SOLR_REQUEST_TYPE_PARAM; -import static org.apache.solr.servlet.RateLimitManager.DEFAULT_SLOT_ACQUISITION_TIMEOUT_MS; +import static org.apache.solr.core.RateLimiterConfig.DEFAULT_SLOT_ACQUISITION_TIMEOUT_MS; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.instanceOf; @@ -49,7 +49,6 @@ import org.apache.solr.cloud.SolrCloudTestCase; import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrInputDocument; -import org.apache.solr.common.cloud.SolrZkClient; import org.apache.solr.common.util.ExecutorUtil; import org.apache.solr.core.RateLimiterConfig; import org.junit.BeforeClass; @@ -74,7 +73,7 @@ public void testConcurrentQueries() throws Exception { CollectionAdminRequest.createCollection(FIRST_COLLECTION, 1, 1).process(client); cluster.waitForActiveCollection(FIRST_COLLECTION, 1, 1); - SolrDispatchFilter solrDispatchFilter = cluster.getJettySolrRunner(0).getSolrDispatchFilter(); + RateLimitFilter rateLimitFilter = cluster.getJettySolrRunner(0).getSolrRateLimitFilter(); RateLimiterConfig rateLimiterConfig = new RateLimiterConfig( @@ -87,11 +86,10 @@ public void testConcurrentQueries() throws Exception { // We are fine with a null FilterConfig here since we ensure that MockBuilder never invokes // its parent here RateLimitManager.Builder builder = - new MockBuilder( - null /* dummy SolrZkClient */, new MockRequestRateLimiter(rateLimiterConfig)); + new MockBuilder(new MockRequestRateLimiter(rateLimiterConfig)); RateLimitManager rateLimitManager = builder.build(); - solrDispatchFilter.replaceRateLimitManager(rateLimitManager); + rateLimitFilter.setRateLimitManager(rateLimitManager); int numDocs = TEST_NIGHTLY ? 10000 : 100; @@ -295,7 +293,7 @@ public void testSlotBorrowing() throws Exception { CollectionAdminRequest.createCollection(SECOND_COLLECTION, 1, 1).process(client); cluster.waitForActiveCollection(SECOND_COLLECTION, 1, 1); - SolrDispatchFilter solrDispatchFilter = cluster.getJettySolrRunner(0).getSolrDispatchFilter(); + RateLimitFilter rateLimitFilter = cluster.getJettySolrRunner(0).getSolrRateLimitFilter(); RateLimiterConfig queryRateLimiterConfig = new RateLimiterConfig( @@ -317,12 +315,11 @@ public void testSlotBorrowing() throws Exception { // its parent RateLimitManager.Builder builder = new MockBuilder( - null /*dummy SolrZkClient */, new MockRequestRateLimiter(queryRateLimiterConfig), new MockRequestRateLimiter(indexRateLimiterConfig)); RateLimitManager rateLimitManager = builder.build(); - solrDispatchFilter.replaceRateLimitManager(rateLimitManager); + rateLimitFilter.setRateLimitManager(rateLimitManager); int numDocs = 10000; @@ -435,18 +432,16 @@ private static class MockBuilder extends RateLimitManager.Builder { private final RequestRateLimiter queryRequestRateLimiter; private final RequestRateLimiter indexRequestRateLimiter; - public MockBuilder(SolrZkClient zkClient, RequestRateLimiter queryRequestRateLimiter) { - super(zkClient); + public MockBuilder(RequestRateLimiter queryRequestRateLimiter) { + super(); this.queryRequestRateLimiter = queryRequestRateLimiter; this.indexRequestRateLimiter = null; } public MockBuilder( - SolrZkClient zkClient, - RequestRateLimiter queryRequestRateLimiter, - RequestRateLimiter indexRequestRateLimiter) { - super(zkClient); + RequestRateLimiter queryRequestRateLimiter, RequestRateLimiter indexRequestRateLimiter) { + super(); this.queryRequestRateLimiter = queryRequestRateLimiter; this.indexRequestRateLimiter = indexRequestRateLimiter; diff --git a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java index 99ebe39fe678..2530ed7acd4a 100644 --- a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java +++ b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java @@ -61,8 +61,9 @@ import org.apache.solr.core.CoreContainer; import org.apache.solr.metrics.SolrMetricManager; import org.apache.solr.servlet.CoreContainerProvider; -import org.apache.solr.servlet.EssentialSolrRequestFilter; import org.apache.solr.servlet.PathExclusionFilter; +import org.apache.solr.servlet.RateLimitFilter; +import org.apache.solr.servlet.RequiredSolrRequestFilter; import org.apache.solr.servlet.SolrDispatchFilter; import org.apache.solr.util.SocketProxy; import org.apache.solr.util.TimeOut; @@ -113,7 +114,8 @@ public class JettySolrRunner { volatile FilterHolder debugFilter; volatile FilterHolder pathExcludeFilter; - volatile FilterHolder essentialFilter; + volatile FilterHolder requiredFilter; + volatile FilterHolder rateLimitFilter; volatile FilterHolder dispatchFilter; private int jettyPort = -1; @@ -411,9 +413,13 @@ public void contextInitialized(ServletContextEvent event) { pathExcludeFilter.setHeldClass(PathExclusionFilter.class); pathExcludeFilter.setInitParameter("excludePatterns", excludePatterns); - // logging context setup - essentialFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED); - essentialFilter.setHeldClass(EssentialSolrRequestFilter.class); + // required request setup + requiredFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED); + requiredFilter.setHeldClass(RequiredSolrRequestFilter.class); + + // Ratelimit Requests + rateLimitFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED); + rateLimitFilter.setHeldClass(RateLimitFilter.class); // This is our main workhorse dispatchFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED); @@ -421,7 +427,8 @@ public void contextInitialized(ServletContextEvent event) { // Map dispatchFilter in same path as in web.xml root.addFilter(pathExcludeFilter, "/*", EnumSet.of(DispatcherType.REQUEST)); - root.addFilter(essentialFilter, "/*", EnumSet.of(DispatcherType.REQUEST)); + root.addFilter(requiredFilter, "/*", EnumSet.of(DispatcherType.REQUEST)); + root.addFilter(rateLimitFilter, "/*", EnumSet.of(DispatcherType.REQUEST)); root.addFilter(dispatchFilter, "/*", EnumSet.of(DispatcherType.REQUEST)); // Default servlet as a fall-through @@ -478,6 +485,13 @@ public SolrDispatchFilter getSolrDispatchFilter() { return (SolrDispatchFilter) dispatchFilter.getFilter(); } + /** + * @return the {@link SolrDispatchFilter} for this node + */ + public RateLimitFilter getSolrRateLimitFilter() { + return (RateLimitFilter) rateLimitFilter.getFilter(); + } + /** * @return the {@link CoreContainer} for this node */ diff --git a/solr/webapp/web/WEB-INF/web.xml b/solr/webapp/web/WEB-INF/web.xml index af5e733526ce..e383133ea6dc 100644 --- a/solr/webapp/web/WEB-INF/web.xml +++ b/solr/webapp/web/WEB-INF/web.xml @@ -46,7 +46,7 @@ MDCLoggingFilter - org.apache.solr.servlet.EssentialSolrRequestFilter + org.apache.solr.servlet.RequiredSolrRequestFilter @@ -54,6 +54,16 @@ /* + + RateLimitFilter + org.apache.solr.servlet.RateLimitFilter + + + + RateLimitFilter + /* + + SolrRequestFilter org.apache.solr.servlet.SolrDispatchFilter