You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by sh...@apache.org on 2020/12/01 13:43:38 UTC

[lucene-solr] branch jira/solr-14942-feedback created (now 4f3f8d7)

This is an automated email from the ASF dual-hosted git repository.

shalin pushed a change to branch jira/solr-14942-feedback
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git.


      at 4f3f8d7  SOLR-14942: Move request registration to ContentStreamHandlerBase

This branch includes the following new commits:

     new 4f3f8d7  SOLR-14942: Move request registration to ContentStreamHandlerBase

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[lucene-solr] 01/01: SOLR-14942: Move request registration to ContentStreamHandlerBase

Posted by sh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

shalin pushed a commit to branch jira/solr-14942-feedback
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit 4f3f8d7da77f9d0d30617dc154ecd23212a76379
Author: Shalin Shekhar Mangar <sh...@apache.org>
AuthorDate: Tue Dec 1 14:32:14 2020 +0530

    SOLR-14942: Move request registration to ContentStreamHandlerBase
    
    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.
---
 .../solr/handler/ContentStreamHandlerBase.java     | 65 +++++++++++-------
 .../java/org/apache/solr/servlet/HttpSolrCall.java | 76 +++++++++-------------
 2 files changed, 70 insertions(+), 71 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 7f4feac..c4ad163 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);
 
-      Iterable<ContentStream> 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 {
+      try {
+        ContentStreamLoader documentLoader = newLoader(req, processor);
 
-        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);
-      }
-    } finally {
-      // finish the request
-      try {
-        processor.finish();
+        Iterable<ContentStream> 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);
+        }
       } finally {
-        processor.close();
+        // finish the request
+        try {
+          processor.finish();
+        } finally {
+          processor.close();
+        }
       }
+    } finally {
+      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 6d3a3ab..80b9def 100644
--- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
+++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
@@ -551,57 +551,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()));
-                }
-              }
-              HttpCacheHeaderUtil.checkHttpCachingVeto(solrRsp, resp, reqMethod);
-              Iterator<Map.Entry<String, String>> headers = solrRsp.httpHeaders();
-              while (headers.hasNext()) {
-                Map.Entry<String, String> entry = headers.next();
-                resp.addHeader(entry.getKey(), entry.getValue());
+          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()));
               }
-              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<Map.Entry<String, String>> headers = solrRsp.httpHeaders();
+            while (headers.hasNext()) {
+              Map.Entry<String, String> 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) {