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/15 04:41:19 UTC
[lucene-solr] branch master updated: SOLR-14722: timeAllowed should
track from req creation (#1726)
This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/master by this push:
new 0ce2d61 SOLR-14722: timeAllowed should track from req creation (#1726)
0ce2d61 is described below
commit 0ce2d610c2e3524b1cb819d9cd2a5e2ccb190bd0
Author: David Smiley <ds...@apache.org>
AuthorDate: Sat Aug 15 00:41:07 2020 -0400
SOLR-14722: timeAllowed should track from req creation (#1726)
* set(long) instead of set(Long).
* Fix javadocs CommonParams.TIME_ALLOWED
---
solr/CHANGES.txt | 2 ++
.../org/apache/solr/handler/AnalyticsHandler.java | 7 ++---
.../apache/solr/handler/MoreLikeThisHandler.java | 5 +--
.../solr/handler/component/SearchHandler.java | 5 +--
.../apache/solr/search/SolrQueryTimeoutImpl.java | 36 ++++++++++++++++------
.../apache/solr/common/params/CommonParams.java | 2 +-
6 files changed, 33 insertions(+), 24 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 5858456..4e4a34b 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 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 c2bd1b6..3934721 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 c41b416..adf3b93 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,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(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) {
+ 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 a78face..0a18e60 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";