From 8493cf18cbfcbf020c467737e28c55e4b05c60f9 Mon Sep 17 00:00:00 2001 From: Chris Hostetter Date: Tue, 17 Dec 2019 13:54:33 -0700 Subject: [PATCH] Harden (Cloud)ExitableDirectoryReaderTest Thread.sleep() is "subject to the precision and accuracy of system timers and schedulers." But tests using DelayingSearchComponent need to ensure that it sleeps *at least* as long as they request, in order to trigger the timeAllowed constraint --- .../solr/search/DelayingSearchComponent.java | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/solr/core/src/test/org/apache/solr/search/DelayingSearchComponent.java b/solr/core/src/test/org/apache/solr/search/DelayingSearchComponent.java index eb640637f7b..b7e8e4ca570 100644 --- a/solr/core/src/test/org/apache/solr/search/DelayingSearchComponent.java +++ b/solr/core/src/test/org/apache/solr/search/DelayingSearchComponent.java @@ -16,11 +16,12 @@ */ package org.apache.solr.search; +import java.io.IOException; +import java.util.concurrent.TimeUnit; + import org.apache.solr.handler.component.ResponseBuilder; import org.apache.solr.handler.component.SearchComponent; -import java.io.IOException; - /** * Search component used to add delay to each request. */ @@ -33,13 +34,27 @@ public class DelayingSearchComponent extends SearchComponent{ @Override public void process(ResponseBuilder rb) throws IOException { - int sleep = rb.req.getParams().getInt("sleep",0); - try { - if (sleep > 0) { - Thread.sleep(sleep); + final long totalSleepMillis = rb.req.getParams().getLong("sleep",0); + if (totalSleepMillis > 0) { + final long totalSleepNanos = TimeUnit.NANOSECONDS.convert(totalSleepMillis, TimeUnit.MILLISECONDS); + final long startNanos = System.nanoTime(); + try { + // Thread.sleep() (and derivatives) are not garunteed to sleep the full amount: + // "subject to the precision and accuracy of system timers and schedulers." + // This is particularly problematic on Windows VMs, so we do a retry loop + // to ensure we sleep a total of at least as long as requested + // + // (Tests using this component do so explicitly to ensure 'timeAllowed' + // has exceeded in order to get their expected results, we would rather over-sleep + // then under sleep) + for (long sleepNanos = totalSleepNanos; + 0 < sleepNanos; + sleepNanos = totalSleepNanos - (System.nanoTime() - startNanos)) { + TimeUnit.NANOSECONDS.sleep(sleepNanos); + } + } catch (InterruptedException e) { + // Do nothing? } - } catch (InterruptedException e) { - // Do nothing? } }