madrob commented on a change in pull request #1726:
URL: https://github.com/apache/lucene-solr/pull/1726#discussion_r468315778



##########
File path: solr/core/src/java/org/apache/solr/search/SolrQueryTimeoutImpl.java
##########
@@ -67,8 +69,21 @@ public boolean shouldExit() {
   }
 
   /**
-   * 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) {

Review comment:
       should this be a primitive instead of a boxed type?

##########
File path: solr/core/src/java/org/apache/solr/search/SolrQueryTimeoutImpl.java
##########
@@ -67,8 +69,21 @@ public boolean shouldExit() {
   }
 
   /**
-   * 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) {

Review comment:
       Should be `>`, not `>=`. Doc on time allowed state that zero is no 
timeout, not immediate timeout. Looks like we were previously inconsistent 
about this.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to