diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 5858456580d..4e4a34b4e32 100644 --- a/solr/CHANGES.txt +++ b/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 --------------------- 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 9ffbaf483e0..ede5041df09 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 7f55a3f6891..e97210362c6 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 c2bd1b68f4d..3934721ef1b 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 @@ -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 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 c41b4165e64..adf3b931bc4 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 timeoutAt = new ThreadLocal(); + private static final ThreadLocal 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() + ")"; } } 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 a78faceb553..0a18e604f40 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 @@ -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";