You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ds...@apache.org on 2020/08/16 04:21:43 UTC

[lucene-solr] branch branch_8x updated: SOLR-14722: timeAllowed should track from req creation (#1726) This 8x backport left some methods named as-is for back-compat sake.

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

dsmiley pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/branch_8x by this push:
     new 36a08fc  SOLR-14722: timeAllowed should track from req creation (#1726) This 8x backport left some methods named as-is for back-compat sake.
36a08fc is described below

commit 36a08fc47bbd19a95d5d89e004b77f2ac730741d
Author: David Smiley <ds...@salesforce.com>
AuthorDate: Sun Aug 16 00:21:21 2020 -0400

    SOLR-14722: timeAllowed should track from req creation (#1726)
    This 8x backport left some methods named as-is for back-compat sake.
---
 solr/CHANGES.txt                                   |  4 +++-
 .../org/apache/solr/handler/AnalyticsHandler.java  |  7 ++----
 .../apache/solr/handler/MoreLikeThisHandler.java   |  5 +----
 .../solr/handler/component/SearchHandler.java      |  5 +----
 .../apache/solr/search/SolrQueryTimeoutImpl.java   | 26 +++++++++++++++++-----
 .../apache/solr/common/params/CommonParams.java    |  2 +-
 6 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 47186bf..dc288f1 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -46,6 +46,8 @@ Improvements
 * SOLR-13205: Prevent StringIndexOutOfBoundsException when parsing field names in SolrQueryParserBase
   (pramodkumar9 via Jason Gerlowski)
 
+* SOLR-14722: Track timeAllowed from earlier in the request lifecycle. (David Smiley)
+
 Optimizations
 ---------------------
 
@@ -93,7 +95,7 @@ Other Changes
 
 * SOLR-11868: Deprecate CloudSolrClient.setIdField, use information from Zookeeper (Erick Erickson)
 
-* SOLR-14702: Most references to "master" and "slave" replaced with "leader" and "follower". Some references 
+* SOLR-14702: Most references to "master" and "slave" replaced with "leader" and "follower". Some references
   remain for compatibility purposes and were removed in master. (MarcusSorealheis, Erick Erickson, Tomás Fernández Löbbe)
 
 ==================  8.6.1 ==================
diff --git a/solr/contrib/analytics/src/java/org/apache/solr/handler/AnalyticsHandler.java b/solr/contrib/analytics/src/java/org/apache/solr/handler/AnalyticsHandler.java
index 9ffbaf4..ede5041 100644
--- a/solr/contrib/analytics/src/java/org/apache/solr/handler/AnalyticsHandler.java
+++ b/solr/contrib/analytics/src/java/org/apache/solr/handler/AnalyticsHandler.java
@@ -72,11 +72,8 @@ public class AnalyticsHandler extends RequestHandlerBase implements SolrCoreAwar
   }
 
   public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
-   
-    long timeAllowed = req.getParams().getLong(CommonParams.TIME_ALLOWED, -1L);
-    if (timeAllowed >= 0L) {
-      SolrQueryTimeoutImpl.set(timeAllowed);
-    }
+
+    SolrQueryTimeoutImpl.set(req);
     try {
       DocSet docs;
       try {
diff --git a/solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java b/solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java
index 7f55a3f..e972103 100644
--- a/solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java
@@ -100,10 +100,7 @@ public class MoreLikeThisHandler extends RequestHandlerBase
   {
     SolrParams params = req.getParams();
 
-    long timeAllowed = (long)params.getInt( CommonParams.TIME_ALLOWED, -1 );
-    if(timeAllowed > 0) {
-      SolrQueryTimeoutImpl.set(timeAllowed);
-    }
+    SolrQueryTimeoutImpl.set(req);
       try {
 
         // Set field flags
diff --git a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
index 5048608..f2087f3 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
@@ -322,10 +322,7 @@ public class SearchHandler extends RequestHandlerBase implements SolrCoreAware,
     if (!rb.isDistrib) {
       // a normal non-distributed request
 
-      long timeAllowed = req.getParams().getLong(CommonParams.TIME_ALLOWED, -1L);
-      if (timeAllowed >= 0L) {
-        SolrQueryTimeoutImpl.set(timeAllowed);
-      }
+      SolrQueryTimeoutImpl.set(req);
       try {
         // The semantics of debugging vs not debugging are different enough that
         // it makes sense to have two control loops
diff --git a/solr/core/src/java/org/apache/solr/search/SolrQueryTimeoutImpl.java b/solr/core/src/java/org/apache/solr/search/SolrQueryTimeoutImpl.java
index c41b416..ec3160a 100644
--- a/solr/core/src/java/org/apache/solr/search/SolrQueryTimeoutImpl.java
+++ b/solr/core/src/java/org/apache/solr/search/SolrQueryTimeoutImpl.java
@@ -21,6 +21,8 @@ import static java.lang.System.nanoTime;
 import java.util.concurrent.TimeUnit;
 
 import org.apache.lucene.index.QueryTimeout;
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.request.SolrQueryRequest;
 
 /**
  * Implementation of {@link QueryTimeout} that is used by Solr. 
@@ -31,10 +33,11 @@ public class SolrQueryTimeoutImpl implements QueryTimeout {
   /**
    * The ThreadLocal variable to store the time beyond which, the processing should exit.
    */
-  public static ThreadLocal<Long> timeoutAt = new ThreadLocal<Long>();
+  private static final ThreadLocal<Long> timeoutAt = new ThreadLocal<>();
+
+  private static final SolrQueryTimeoutImpl instance = new SolrQueryTimeoutImpl();
 
   private SolrQueryTimeoutImpl() { }
-  private static SolrQueryTimeoutImpl instance = new SolrQueryTimeoutImpl();
 
   /** Return singleton instance */
   public static SolrQueryTimeoutImpl getInstance() { 
@@ -42,7 +45,7 @@ public class SolrQueryTimeoutImpl implements QueryTimeout {
   }
 
   /**
-   * Get the current value of timeoutAt.
+   * The time (nanoseconds) at which the request should be considered timed out.
    */
   public static Long get() {
     return timeoutAt.get();
@@ -67,8 +70,21 @@ public class SolrQueryTimeoutImpl implements QueryTimeout {
   }
 
   /**
-   * Method to set the time at which the timeOut should happen.
-   * @param timeAllowed set the time at which this thread should timeout.
+   * Sets or clears the time allowed based on how much time remains from the start of the request plus the configured
+   * {@link CommonParams#TIME_ALLOWED}.
+   */
+  public static void set(SolrQueryRequest req) {
+    long timeAllowed = req.getParams().getLong(CommonParams.TIME_ALLOWED, -1L);
+    if (timeAllowed >= 0L) {
+      set(timeAllowed - (long)req.getRequestTimer().getTime()); // reduce by time already spent
+    } else {
+      reset();
+    }
+  }
+
+  /**
+   * Sets the time allowed (milliseconds), assuming we start a timer immediately.
+   * You should probably invoke {@link #set(SolrQueryRequest)} instead.
    */
   public static void set(Long timeAllowed) {
     long time = nanoTime() + TimeUnit.NANOSECONDS.convert(timeAllowed, TimeUnit.MILLISECONDS);
diff --git a/solr/solrj/src/java/org/apache/solr/common/params/CommonParams.java b/solr/solrj/src/java/org/apache/solr/common/params/CommonParams.java
index 9a95bf2..f3e49f6 100644
--- a/solr/solrj/src/java/org/apache/solr/common/params/CommonParams.java
+++ b/solr/solrj/src/java/org/apache/solr/common/params/CommonParams.java
@@ -162,7 +162,7 @@ public interface CommonParams {
   boolean SEGMENT_TERMINATE_EARLY_DEFAULT = false;
 
   /**
-   * Timeout value in milliseconds.  If not set, or the value is &gt;= 0, there is no timeout.
+   * Timeout value in milliseconds.  If not set, or the value is &gt; 0, there is no timeout.
    */
   String TIME_ALLOWED = "timeAllowed";