From d99c1667a51a787db7eac3f70350533b10c4f9cd Mon Sep 17 00:00:00 2001 From: Shalin Shekhar Mangar Date: Wed, 2 Dec 2020 10:11:23 +0530 Subject: [PATCH] SOLR-14942: Move request registration to ContentStreamHandlerBase (#2112) This addresses review feedback from David Smiley on Jira. It moves the request registration to the ContentStreamHandlerBase class instead of doing a hack-ish instanceof check inside HttpSolrCall. --- .../handler/ContentStreamHandlerBase.java | 65 ++++++++++------ .../org/apache/solr/servlet/HttpSolrCall.java | 77 +++++++------------ 2 files changed, 70 insertions(+), 72 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/ContentStreamHandlerBase.java b/solr/core/src/java/org/apache/solr/handler/ContentStreamHandlerBase.java index 7f4feac3bc2..c4ad16349f8 100644 --- a/solr/core/src/java/org/apache/solr/handler/ContentStreamHandlerBase.java +++ b/solr/core/src/java/org/apache/solr/handler/ContentStreamHandlerBase.java @@ -22,6 +22,7 @@ import org.apache.solr.common.util.NamedList; import org.apache.solr.handler.loader.ContentStreamLoader; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; +import org.apache.solr.update.SolrCoreState; import org.apache.solr.update.processor.UpdateRequestProcessor; import org.apache.solr.update.processor.UpdateRequestProcessorChain; @@ -47,38 +48,54 @@ public abstract class ContentStreamHandlerBase extends RequestHandlerBase { @Override public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception { - SolrParams params = req.getParams(); - UpdateRequestProcessorChain processorChain = - req.getCore().getUpdateProcessorChain(params); - - UpdateRequestProcessor processor = processorChain.createProcessor(req, rsp); + /* + We track update requests so that we can preserve consistency by waiting for them to complete + on a node shutdown and then immediately trigger a leader election without waiting for the core to close. + See how the SolrCoreState#pauseUpdatesAndAwaitInflightRequests() method is used in CoreContainer#shutdown() + Also see https://issues.apache.org/jira/browse/SOLR-14942 for details on why we do not care for + other kinds of requests. + */ + SolrCoreState solrCoreState = req.getCore().getSolrCoreState(); + if (!solrCoreState.registerInFlightUpdate()) { + throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Updates are temporarily paused for core: " + req.getCore().getName()); + } try { - ContentStreamLoader documentLoader = newLoader(req, processor); + SolrParams params = req.getParams(); + UpdateRequestProcessorChain processorChain = + req.getCore().getUpdateProcessorChain(params); + + UpdateRequestProcessor processor = processorChain.createProcessor(req, rsp); + + try { + ContentStreamLoader documentLoader = newLoader(req, processor); - Iterable streams = req.getContentStreams(); - if (streams == null) { - if (!RequestHandlerUtils.handleCommit(req, processor, params, false) && !RequestHandlerUtils.handleRollback(req, processor, params, false)) { - throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "missing content stream"); + Iterable streams = req.getContentStreams(); + if (streams == null) { + if (!RequestHandlerUtils.handleCommit(req, processor, params, false) && !RequestHandlerUtils.handleRollback(req, processor, params, false)) { + throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "missing content stream"); + } + } else { + + for (ContentStream stream : streams) { + documentLoader.load(req, rsp, stream, processor); + } + + // Perhaps commit from the parameters + RequestHandlerUtils.handleCommit(req, processor, params, false); + RequestHandlerUtils.handleRollback(req, processor, params, false); } - } else { - - for (ContentStream stream : streams) { - documentLoader.load(req, rsp, stream, processor); + } finally { + // finish the request + try { + processor.finish(); + } finally { + processor.close(); } - - // Perhaps commit from the parameters - RequestHandlerUtils.handleCommit(req, processor, params, false); - RequestHandlerUtils.handleRollback(req, processor, params, false); } } finally { - // finish the request - try { - processor.finish(); - } finally { - processor.close(); - } + solrCoreState.deregisterInFlightUpdate(); } } 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 6d3a3ab0e03..ee7c9f4b5ac 100644 --- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java +++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java @@ -86,7 +86,6 @@ import org.apache.solr.core.CoreContainer; import org.apache.solr.core.SolrConfig; import org.apache.solr.core.SolrCore; import org.apache.solr.handler.ContentStreamHandlerBase; -import org.apache.solr.handler.UpdateRequestHandler; import org.apache.solr.logging.MDCLoggingContext; import org.apache.solr.request.LocalSolrQueryRequest; import org.apache.solr.request.SolrQueryRequest; @@ -551,57 +550,39 @@ public class HttpSolrCall { remoteQuery(coreUrl + path, resp); return RETURN; case PROCESS: - /* - We track update requests so that we can preserve consistency by waiting for them to complete - on a node shutdown and then immediately trigger a leader election without waiting for the core to close. - See how the SolrCoreState#pauseUpdatesAndAwaitInflightRequests() method is used in CoreContainer#shutdown() - - Also see https://issues.apache.org/jira/browse/SOLR-14942 for details on why we do not care for - other kinds of requests. - */ - if (handler instanceof UpdateRequestHandler && !core.getSolrCoreState().registerInFlightUpdate()) { - throw new SolrException(ErrorCode.SERVER_ERROR, "Updates are temporarily paused for core: " + core.getName()); - } - try { - final Method reqMethod = Method.getMethod(req.getMethod()); - HttpCacheHeaderUtil.setCacheControlHeader(config, resp, reqMethod); - // unless we have been explicitly told not to, do cache validation - // if we fail cache validation, execute the query - if (config.getHttpCachingConfig().isNever304() || - !HttpCacheHeaderUtil.doCacheHeaderValidation(solrReq, req, reqMethod, resp)) { - SolrQueryResponse solrRsp = new SolrQueryResponse(); - /* even for HEAD requests, we need to execute the handler to - * ensure we don't get an error (and to make sure the correct - * QueryResponseWriter is selected and we get the correct - * Content-Type) - */ - SolrRequestInfo.setRequestInfo(new SolrRequestInfo(solrReq, solrRsp, action)); - mustClearSolrRequestInfo = true; - execute(solrRsp); - if (shouldAudit()) { - EventType eventType = solrRsp.getException() == null ? EventType.COMPLETED : EventType.ERROR; - if (shouldAudit(eventType)) { - cores.getAuditLoggerPlugin().doAudit( - new AuditEvent(eventType, req, getAuthCtx(), solrReq.getRequestTimer().getTime(), solrRsp.getException())); - } + final Method reqMethod = Method.getMethod(req.getMethod()); + HttpCacheHeaderUtil.setCacheControlHeader(config, resp, reqMethod); + // unless we have been explicitly told not to, do cache validation + // if we fail cache validation, execute the query + if (config.getHttpCachingConfig().isNever304() || + !HttpCacheHeaderUtil.doCacheHeaderValidation(solrReq, req, reqMethod, resp)) { + SolrQueryResponse solrRsp = new SolrQueryResponse(); + /* even for HEAD requests, we need to execute the handler to + * ensure we don't get an error (and to make sure the correct + * QueryResponseWriter is selected and we get the correct + * Content-Type) + */ + SolrRequestInfo.setRequestInfo(new SolrRequestInfo(solrReq, solrRsp, action)); + mustClearSolrRequestInfo = true; + execute(solrRsp); + if (shouldAudit()) { + EventType eventType = solrRsp.getException() == null ? EventType.COMPLETED : EventType.ERROR; + if (shouldAudit(eventType)) { + cores.getAuditLoggerPlugin().doAudit( + new AuditEvent(eventType, req, getAuthCtx(), solrReq.getRequestTimer().getTime(), solrRsp.getException())); } - HttpCacheHeaderUtil.checkHttpCachingVeto(solrRsp, resp, reqMethod); - Iterator> headers = solrRsp.httpHeaders(); - while (headers.hasNext()) { - Map.Entry entry = headers.next(); - resp.addHeader(entry.getKey(), entry.getValue()); - } - QueryResponseWriter responseWriter = getResponseWriter(); - if (invalidStates != null) solrReq.getContext().put(CloudSolrClient.STATE_VERSION, invalidStates); - writeResponse(solrRsp, responseWriter, reqMethod); } - return RETURN; - } finally { - if (handler instanceof UpdateRequestHandler) { - // every registered request must also be de-registered - core.getSolrCoreState().deregisterInFlightUpdate(); + HttpCacheHeaderUtil.checkHttpCachingVeto(solrRsp, resp, reqMethod); + Iterator> headers = solrRsp.httpHeaders(); + while (headers.hasNext()) { + Map.Entry entry = headers.next(); + resp.addHeader(entry.getKey(), entry.getValue()); } + QueryResponseWriter responseWriter = getResponseWriter(); + if (invalidStates != null) solrReq.getContext().put(CloudSolrClient.STATE_VERSION, invalidStates); + writeResponse(solrRsp, responseWriter, reqMethod); } + return RETURN; default: return action; } } catch (Throwable ex) {