You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ma...@apache.org on 2016/02/16 22:01:34 UTC
[2/2] lucene-solr git commit: SOLR-8683: Always consume the full
request on the server, not just in the case of an error.
SOLR-8683: Always consume the full request on the server, not just in the case of an error.
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/9184d52f
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/9184d52f
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/9184d52f
Branch: refs/heads/master
Commit: 9184d52f68b43a741238ad8798fb2cbdbe81cf27
Parents: a40118c
Author: markrmiller <ma...@apache.org>
Authored: Tue Feb 16 15:48:11 2016 -0500
Committer: markrmiller <ma...@apache.org>
Committed: Tue Feb 16 16:01:10 2016 -0500
----------------------------------------------------------------------
solr/CHANGES.txt | 3 +
.../org/apache/solr/servlet/HttpSolrCall.java | 29 +----
.../apache/solr/servlet/SolrDispatchFilter.java | 107 +++++++++++--------
3 files changed, 69 insertions(+), 70 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9184d52f/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 828727f..cbc1997 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -172,6 +172,9 @@ Bug Fixes
* SOLR-8578: Successful or not, requests are not always fully consumed by Solrj clients and we
count on HttpClient or the JVM. (Mark Miller)
+* SOLR-8683: Always consume the full request on the server, not just in the case of an error.
+ (Mark Miller)
+
Optimizations
----------------------
* SOLR-7876: Speed up queries and operations that use many terms when timeAllowed has not been
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9184d52f/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
----------------------------------------------------------------------
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 7227a8b..d87eb69 100644
--- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
+++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
@@ -621,13 +621,9 @@ public class HttpSolrCall {
} finally {
try {
if (exp != null) {
- try {
- SimpleOrderedMap info = new SimpleOrderedMap();
- int code = ResponseUtils.getErrorInfo(ex, info, log);
- sendError(code, info.toString());
- } finally {
- consumeInput(req);
- }
+ SimpleOrderedMap info = new SimpleOrderedMap();
+ int code = ResponseUtils.getErrorInfo(ex, info, log);
+ sendError(code, info.toString());
}
} finally {
if (core == null && localCore != null) {
@@ -642,21 +638,6 @@ public class HttpSolrCall {
response.sendError(code, message);
} catch (EOFException e) {
log.info("Unable to write error response, client closed connection or we are shutting down", e);
- } finally {
- consumeInput(req);
- }
- }
-
- // when we send back an error, 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
- private void consumeInput(HttpServletRequest req) {
- try {
- ServletInputStream is = req.getInputStream();
- while (!is.isFinished() && is.read() != -1) {}
- } catch (IOException e) {
- log.info("Could not consume full client request", e);
}
}
@@ -743,10 +724,6 @@ public class HttpSolrCall {
//else http HEAD request, nothing to write out, waited this long just to get ContentType
} catch (EOFException e) {
log.info("Unable to write response, client closed connection or we are shutting down", e);
- } finally {
- if (solrRsp.getException() != null) {
- consumeInput(req);
- }
}
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9184d52f/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
----------------------------------------------------------------------
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 baab3eb..7a0e4ef 100644
--- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
+++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
@@ -19,6 +19,7 @@ package org.apache.solr.servlet;
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.http.HttpServletRequest;
@@ -185,58 +186,76 @@ public class SolrDispatchFilter extends BaseSolrFilter {
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain, boolean retry) throws IOException, ServletException {
if (!(request instanceof HttpServletRequest)) return;
+ try {
- if (cores == null || cores.isShutDown()) {
- log.error("Error processing the request. CoreContainer is either not initialized or shutting down.");
- throw new SolrException(ErrorCode.SERVICE_UNAVAILABLE,
- "Error processing the request. CoreContainer is either not initialized or shutting down.");
- }
+ if (cores == null || cores.isShutDown()) {
+ log.error("Error processing the request. CoreContainer is either not initialized or shutting down.");
+ throw new SolrException(ErrorCode.SERVICE_UNAVAILABLE,
+ "Error processing the request. CoreContainer is either not initialized or shutting down.");
+ }
- AtomicReference<ServletRequest> wrappedRequest = new AtomicReference<>();
- if (!authenticateRequest(request, response, wrappedRequest)) { // the response and status code have already been sent
- return;
- }
- if (wrappedRequest.get() != null) {
- request = wrappedRequest.get();
- }
- if (cores.getAuthenticationPlugin() != null) {
- log.debug("User principal: {}", ((HttpServletRequest)request).getUserPrincipal());
- }
+ AtomicReference<ServletRequest> wrappedRequest = new AtomicReference<>();
+ if (!authenticateRequest(request, response, wrappedRequest)) { // the response and status code have already been
+ // sent
+ return;
+ }
+ if (wrappedRequest.get() != null) {
+ request = wrappedRequest.get();
+ }
+ if (cores.getAuthenticationPlugin() != null) {
+ log.debug("User principal: {}", ((HttpServletRequest) request).getUserPrincipal());
+ }
- // No need to even create the HttpSolrCall object if this path is excluded.
- if(excludePatterns != null) {
- String requestPath = ((HttpServletRequest) request).getServletPath();
- String extraPath = ((HttpServletRequest)request).getPathInfo();
- if (extraPath != null) { // In embedded mode, servlet path is empty - include all post-context path here for testing
- requestPath += extraPath;
+ // No need to even create the HttpSolrCall object if this path is excluded.
+ if (excludePatterns != null) {
+ String requestPath = ((HttpServletRequest) request).getServletPath();
+ String extraPath = ((HttpServletRequest) request).getPathInfo();
+ if (extraPath != null) { // In embedded mode, servlet path is empty - include all post-context path here for
+ // testing
+ requestPath += extraPath;
+ }
+ for (Pattern p : excludePatterns) {
+ Matcher matcher = p.matcher(requestPath);
+ if (matcher.lookingAt()) {
+ chain.doFilter(request, response);
+ return;
+ }
+ }
}
- for (Pattern p : excludePatterns) {
- Matcher matcher = p.matcher(requestPath);
- if (matcher.lookingAt()) {
- chain.doFilter(request, response);
- return;
+
+ HttpSolrCall call = getHttpSolrCall((HttpServletRequest) request, (HttpServletResponse) response, retry);
+ ExecutorUtil.setServerThreadFlag(Boolean.TRUE);
+ try {
+ Action result = call.call();
+ switch (result) {
+ case PASSTHROUGH:
+ chain.doFilter(request, response);
+ break;
+ case RETRY:
+ doFilter(request, response, chain, true);
+ break;
+ case FORWARD:
+ request.getRequestDispatcher(call.getPath()).forward(request, response);
+ break;
}
+ } finally {
+ call.destroy();
+ ExecutorUtil.setServerThreadFlag(null);
}
+ } finally {
+ consumeInputFully((HttpServletRequest) request);
}
-
- HttpSolrCall call = getHttpSolrCall((HttpServletRequest) request, (HttpServletResponse) response, retry);
- ExecutorUtil.setServerThreadFlag(Boolean.TRUE);
+ }
+
+ // 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) {
try {
- Action result = call.call();
- switch (result) {
- case PASSTHROUGH:
- chain.doFilter(request, response);
- break;
- case RETRY:
- doFilter(request, response, chain, true);
- break;
- case FORWARD:
- request.getRequestDispatcher(call.getPath()).forward(request, response);
- break;
- }
- } finally {
- call.destroy();
- ExecutorUtil.setServerThreadFlag(null);
+ ServletInputStream is = req.getInputStream();
+ while (!is.isFinished() && is.read() != -1) {}
+ } catch (IOException e) {
+ log.info("Could not consume full client request", e);
}
}