From 1316825f52b42f0b73197e9532a51cf33a3ea34f Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Thu, 21 Feb 2019 12:42:54 +0200 Subject: [PATCH] Replace superfluous usage of Counter with Supplier (#39048) (#39225) `Counter` was used as a means of a functional argument to pass the relative cached time before `Supplier` iface was introduced. --- .../forbidden/es-server-signatures.txt | 4 ++-- .../search/DefaultSearchContext.java | 12 +++++------ .../elasticsearch/search/SearchService.java | 2 +- .../internal/FilteredSearchContext.java | 5 ++--- .../search/internal/SearchContext.java | 7 +++++-- .../search/internal/SubSearchContext.java | 3 +-- .../search/query/QueryPhase.java | 6 ++---- .../elasticsearch/threadpool/ThreadPool.java | 20 ------------------- .../versioning/SimpleVersioningIT.java | 2 +- .../coordination/DeterministicTaskQueue.java | 16 --------------- .../elasticsearch/test/TestSearchContext.java | 6 ++---- 11 files changed, 22 insertions(+), 61 deletions(-) diff --git a/buildSrc/src/main/resources/forbidden/es-server-signatures.txt b/buildSrc/src/main/resources/forbidden/es-server-signatures.txt index f884e7c1a48e0..7009883e689d7 100644 --- a/buildSrc/src/main/resources/forbidden/es-server-signatures.txt +++ b/buildSrc/src/main/resources/forbidden/es-server-signatures.txt @@ -61,7 +61,7 @@ java.nio.channels.FileChannel#read(java.nio.ByteBuffer, long) @defaultMessage Use Lucene.parseLenient instead it strips off minor version org.apache.lucene.util.Version#parseLeniently(java.lang.String) -@defaultMessage Spawns a new thread which is solely under lucenes control use ThreadPool#estimatedTimeInMillisCounter instead +@defaultMessage Spawns a new thread which is solely under lucenes control use ThreadPool#relativeTimeInMillis instead org.apache.lucene.search.TimeLimitingCollector#getGlobalTimerThread() org.apache.lucene.search.TimeLimitingCollector#getGlobalCounter() @@ -146,4 +146,4 @@ org.apache.logging.log4j.Logger#warn(java.lang.Object, java.lang.Throwable) org.apache.logging.log4j.Logger#error(java.lang.Object) org.apache.logging.log4j.Logger#error(java.lang.Object, java.lang.Throwable) org.apache.logging.log4j.Logger#fatal(java.lang.Object) -org.apache.logging.log4j.Logger#fatal(java.lang.Object, java.lang.Throwable) \ No newline at end of file +org.apache.logging.log4j.Logger#fatal(java.lang.Object, java.lang.Throwable) diff --git a/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java b/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java index 4b82e23e42061..d9a7fdb831efd 100644 --- a/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java +++ b/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java @@ -24,7 +24,6 @@ import org.apache.lucene.search.Collector; import org.apache.lucene.search.FieldDoc; import org.apache.lucene.search.Query; -import org.apache.lucene.util.Counter; import org.elasticsearch.Version; import org.elasticsearch.action.search.SearchTask; import org.elasticsearch.action.search.SearchType; @@ -82,13 +81,14 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.LongSupplier; final class DefaultSearchContext extends SearchContext { private final long id; private final ShardSearchRequest request; private final SearchShardTarget shardTarget; - private final Counter timeEstimateCounter; + private final LongSupplier relativeTimeSupplier; private SearchType searchType; private final Engine.Searcher engineSearcher; private final BigArrays bigArrays; @@ -159,7 +159,7 @@ final class DefaultSearchContext extends SearchContext { DefaultSearchContext(long id, ShardSearchRequest request, SearchShardTarget shardTarget, Engine.Searcher engineSearcher, ClusterService clusterService, IndexService indexService, - IndexShard indexShard, BigArrays bigArrays, Counter timeEstimateCounter, TimeValue timeout, + IndexShard indexShard, BigArrays bigArrays, LongSupplier relativeTimeSupplier, TimeValue timeout, FetchPhase fetchPhase, Version minNodeVersion) { this.id = id; this.request = request; @@ -176,7 +176,7 @@ final class DefaultSearchContext extends SearchContext { this.indexService = indexService; this.clusterService = clusterService; this.searcher = new ContextIndexSearcher(engineSearcher, indexService.cache().query(), indexShard.getQueryCachingPolicy()); - this.timeEstimateCounter = timeEstimateCounter; + this.relativeTimeSupplier = relativeTimeSupplier; this.timeout = timeout; this.minNodeVersion = minNodeVersion; queryShardContext = indexService.newQueryShardContext(request.shardId().id(), searcher.getIndexReader(), request::nowInMillis, @@ -804,8 +804,8 @@ public ObjectMapper getObjectMapper(String name) { } @Override - public Counter timeEstimateCounter() { - return timeEstimateCounter; + public long getRelativeTimeInMillis() { + return relativeTimeSupplier.getAsLong(); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/SearchService.java b/server/src/main/java/org/elasticsearch/search/SearchService.java index a14b4a328775c..06968250e9c88 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchService.java +++ b/server/src/main/java/org/elasticsearch/search/SearchService.java @@ -648,7 +648,7 @@ private DefaultSearchContext createSearchContext(ShardSearchRequest request, Tim Engine.Searcher engineSearcher = indexShard.acquireSearcher(source); final DefaultSearchContext searchContext = new DefaultSearchContext(idGenerator.incrementAndGet(), request, shardTarget, - engineSearcher, clusterService, indexService, indexShard, bigArrays, threadPool.estimatedTimeInMillisCounter(), timeout, + engineSearcher, clusterService, indexService, indexShard, bigArrays, threadPool::relativeTimeInMillis, timeout, fetchPhase, clusterService.state().nodes().getMinNodeVersion()); boolean success = false; try { diff --git a/server/src/main/java/org/elasticsearch/search/internal/FilteredSearchContext.java b/server/src/main/java/org/elasticsearch/search/internal/FilteredSearchContext.java index ecde28719cec6..10eb90afc04fe 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/FilteredSearchContext.java +++ b/server/src/main/java/org/elasticsearch/search/internal/FilteredSearchContext.java @@ -22,7 +22,6 @@ import org.apache.lucene.search.Collector; import org.apache.lucene.search.FieldDoc; import org.apache.lucene.search.Query; -import org.apache.lucene.util.Counter; import org.elasticsearch.action.search.SearchTask; import org.elasticsearch.action.search.SearchType; import org.elasticsearch.common.unit.TimeValue; @@ -508,8 +507,8 @@ public ObjectMapper getObjectMapper(String name) { } @Override - public Counter timeEstimateCounter() { - return in.timeEstimateCounter(); + public long getRelativeTimeInMillis() { + return in.getRelativeTimeInMillis(); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/internal/SearchContext.java b/server/src/main/java/org/elasticsearch/search/internal/SearchContext.java index 2c2aedfcf7484..fba80d5f3c6e0 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/SearchContext.java +++ b/server/src/main/java/org/elasticsearch/search/internal/SearchContext.java @@ -22,7 +22,6 @@ import org.apache.lucene.search.Collector; import org.apache.lucene.search.FieldDoc; import org.apache.lucene.search.Query; -import org.apache.lucene.util.Counter; import org.elasticsearch.action.search.SearchTask; import org.elasticsearch.action.search.SearchType; import org.elasticsearch.common.Nullable; @@ -395,7 +394,11 @@ public final boolean hasOnlySuggest() { public abstract ObjectMapper getObjectMapper(String name); - public abstract Counter timeEstimateCounter(); + /** + * Returns time in milliseconds that can be used for relative time calculations. + * WARN: This is not the epoch time. + */ + public abstract long getRelativeTimeInMillis(); /** Return a view of the additional query collectors that should be run for this context. */ public abstract Map, Collector> queryCollectors(); diff --git a/server/src/main/java/org/elasticsearch/search/internal/SubSearchContext.java b/server/src/main/java/org/elasticsearch/search/internal/SubSearchContext.java index fb4d233f10ee8..7d61eb6870b4a 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/SubSearchContext.java +++ b/server/src/main/java/org/elasticsearch/search/internal/SubSearchContext.java @@ -19,7 +19,6 @@ package org.elasticsearch.search.internal; import org.apache.lucene.search.Query; -import org.apache.lucene.util.Counter; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.index.query.ParsedQuery; import org.elasticsearch.search.aggregations.SearchContextAggregations; @@ -354,7 +353,7 @@ public FetchSearchResult fetchResult() { } @Override - public Counter timeEstimateCounter() { + public long getRelativeTimeInMillis() { throw new UnsupportedOperationException("Not supported"); } diff --git a/server/src/main/java/org/elasticsearch/search/query/QueryPhase.java b/server/src/main/java/org/elasticsearch/search/query/QueryPhase.java index 9ccde2468227a..f0fdbf0c72670 100644 --- a/server/src/main/java/org/elasticsearch/search/query/QueryPhase.java +++ b/server/src/main/java/org/elasticsearch/search/query/QueryPhase.java @@ -37,7 +37,6 @@ import org.apache.lucene.search.Sort; import org.apache.lucene.search.TopDocs; import org.apache.lucene.search.TotalHits; -import org.apache.lucene.util.Counter; import org.elasticsearch.action.search.SearchTask; import org.elasticsearch.common.lucene.Lucene; import org.elasticsearch.common.lucene.search.TopDocsAndMaxScore; @@ -213,12 +212,11 @@ static boolean execute(SearchContext searchContext, final Runnable timeoutRunnable; if (timeoutSet) { - final Counter counter = searchContext.timeEstimateCounter(); - final long startTime = counter.get(); + final long startTime = searchContext.getRelativeTimeInMillis(); final long timeout = searchContext.timeout().millis(); final long maxTime = startTime + timeout; timeoutRunnable = () -> { - final long time = counter.get(); + final long time = searchContext.getRelativeTimeInMillis(); if (time > maxTime) { throw new TimeExceededException(); } diff --git a/server/src/main/java/org/elasticsearch/threadpool/ThreadPool.java b/server/src/main/java/org/elasticsearch/threadpool/ThreadPool.java index 5ca2b15d6ffe0..2d52b03a2c0dc 100644 --- a/server/src/main/java/org/elasticsearch/threadpool/ThreadPool.java +++ b/server/src/main/java/org/elasticsearch/threadpool/ThreadPool.java @@ -22,7 +22,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; -import org.apache.lucene.util.Counter; import org.elasticsearch.Version; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.io.stream.StreamInput; @@ -252,10 +251,6 @@ public long absoluteTimeInMillis() { return cachedTimeThread.absoluteTimeInMillis(); } - public Counter estimatedTimeInMillisCounter() { - return cachedTimeThread.counter; - } - public ThreadPoolInfo info() { return threadPoolInfo; } @@ -538,7 +533,6 @@ public String toString() { static class CachedTimeThread extends Thread { final long interval; - final TimeCounter counter; volatile boolean running = true; volatile long relativeMillis; volatile long absoluteMillis; @@ -548,7 +542,6 @@ static class CachedTimeThread extends Thread { this.interval = interval; this.relativeMillis = TimeValue.nsecToMSec(System.nanoTime()); this.absoluteMillis = System.currentTimeMillis(); - this.counter = new TimeCounter(); setDaemon(true); } @@ -581,19 +574,6 @@ public void run() { } } } - - private class TimeCounter extends Counter { - - @Override - public long addAndGet(long delta) { - throw new UnsupportedOperationException(); - } - - @Override - public long get() { - return relativeMillis; - } - } } static class ExecutorHolder { diff --git a/server/src/test/java/org/elasticsearch/versioning/SimpleVersioningIT.java b/server/src/test/java/org/elasticsearch/versioning/SimpleVersioningIT.java index ffd876383ad9e..f42857216f8ca 100644 --- a/server/src/test/java/org/elasticsearch/versioning/SimpleVersioningIT.java +++ b/server/src/test/java/org/elasticsearch/versioning/SimpleVersioningIT.java @@ -741,7 +741,7 @@ public void testDeleteNotLost() throws Exception { .getVersion(), equalTo(-1L)); - // ThreadPool.estimatedTimeInMillis has default granularity of 200 msec, so we must sleep at least that long; sleep much longer in + // ThreadPool.relativeTimeInMillis has default granularity of 200 msec, so we must sleep at least that long; sleep much longer in // case system is busy: Thread.sleep(1000); diff --git a/test/framework/src/main/java/org/elasticsearch/cluster/coordination/DeterministicTaskQueue.java b/test/framework/src/main/java/org/elasticsearch/cluster/coordination/DeterministicTaskQueue.java index f14afe3c36534..4567b97700604 100644 --- a/test/framework/src/main/java/org/elasticsearch/cluster/coordination/DeterministicTaskQueue.java +++ b/test/framework/src/main/java/org/elasticsearch/cluster/coordination/DeterministicTaskQueue.java @@ -22,7 +22,6 @@ import com.carrotsearch.randomizedtesting.generators.RandomNumbers; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.apache.lucene.util.Counter; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.threadpool.ThreadPool; @@ -303,21 +302,6 @@ public long absoluteTimeInMillis() { return currentTimeMillis; } - @Override - public Counter estimatedTimeInMillisCounter() { - return new Counter() { - @Override - public long addAndGet(long delta) { - throw new UnsupportedOperationException(); - } - - @Override - public long get() { - return currentTimeMillis; - } - }; - } - @Override public ThreadPoolInfo info() { throw new UnsupportedOperationException(); diff --git a/test/framework/src/main/java/org/elasticsearch/test/TestSearchContext.java b/test/framework/src/main/java/org/elasticsearch/test/TestSearchContext.java index 6a17f65790368..bba2f1e4015a3 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/TestSearchContext.java +++ b/test/framework/src/main/java/org/elasticsearch/test/TestSearchContext.java @@ -21,7 +21,6 @@ import org.apache.lucene.search.Collector; import org.apache.lucene.search.FieldDoc; import org.apache.lucene.search.Query; -import org.apache.lucene.util.Counter; import org.elasticsearch.action.search.SearchTask; import org.elasticsearch.action.search.SearchType; import org.elasticsearch.common.unit.TimeValue; @@ -73,7 +72,6 @@ public class TestSearchContext extends SearchContext { final ThreadPool threadPool; final Map, Collector> queryCollectors = new HashMap<>(); final IndexShard indexShard; - final Counter timeEstimateCounter = Counter.newCounter(); final QuerySearchResult queryResult = new QuerySearchResult(); final QueryShardContext queryShardContext; ParsedQuery originalQuery; @@ -593,8 +591,8 @@ public void doClose() { } @Override - public Counter timeEstimateCounter() { - return timeEstimateCounter; + public long getRelativeTimeInMillis() { + return 0L; } @Override