SOLR-14722: timeAllowed should track from req creation (#1726)

* set(long) instead of set(Long).
* Fix javadocs CommonParams.TIME_ALLOWED

solr/CHANGES.txt

@@ -156,6 +156,8 @@ Improvements
 * SOLR-14651: The MetricsHistoryHandler can more completely disable itself when you tell it to.
   Also, it now shuts down more thoroughly. (David Smiley)
 
+* SOLR-14722: Track timeAllowed from earlier in the request lifecycle. (David Smiley)
+
 Optimizations
 ---------------------
 

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 {

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

solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java

@@ -350,10 +350,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

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,15 +45,15 @@ 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() {
+  public static Long getTimeoutAtNs() {
     return timeoutAt.get();
   }
 
   @Override
   public boolean isTimeoutEnabled() {
-    return get() != null;
+    return getTimeoutAtNs() != null;
   }
 
   /**
@@ -58,7 +61,7 @@ public class SolrQueryTimeoutImpl implements QueryTimeout {
    */
   @Override
   public boolean shouldExit() {
-    Long timeoutAt = get();
+    Long timeoutAt = getTimeoutAtNs();
     if (timeoutAt == null) {
       // timeout unset
       return false;
@@ -67,10 +70,23 @@ 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(Long timeAllowed) {
+  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);
     timeoutAt.set(time);
   }
@@ -84,7 +100,7 @@ public class SolrQueryTimeoutImpl implements QueryTimeout {
 
   @Override
   public String toString() {
-    return "timeoutAt: " + get() + " (System.nanoTime(): " + nanoTime() + ")";
+    return "timeoutAt: " + getTimeoutAtNs() + " (System.nanoTime(): " + nanoTime() + ")";
   }
 }
 

solr/solrj/src/java/org/apache/solr/common/params/CommonParams.java

@@ -159,7 +159,7 @@ public interface CommonParams {
   boolean SEGMENT_TERMINATE_EARLY_DEFAULT = false;
 
   /**
-   * Timeout value in milliseconds.  If not set, or the value is >= 0, there is no timeout.
+   * Timeout value in milliseconds.  If not set, or the value is > 0, there is no timeout.
    */
   String TIME_ALLOWED = "timeAllowed";