Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace superfluous usage of Counter with Supplier #39048

Merged
merged 5 commits into from
Feb 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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)
org.apache.logging.log4j.Logger#fatal(java.lang.Object, java.lang.Throwable)
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -804,8 +804,8 @@ public ObjectMapper getObjectMapper(String name) {
}

@Override
public Counter timeEstimateCounter() {
return timeEstimateCounter;
public long getRelativeTimeInMillis() {
return relativeTimeSupplier.getAsLong();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -508,8 +507,8 @@ public ObjectMapper getObjectMapper(String name) {
}

@Override
public Counter timeEstimateCounter() {
return in.timeEstimateCounter();
public long getRelativeTimeInMillis() {
return in.getRelativeTimeInMillis();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Class<?>, Collector> queryCollectors();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -354,7 +353,7 @@ public FetchSearchResult fetchResult() {
}

@Override
public Counter timeEstimateCounter() {
public long getRelativeTimeInMillis() {
throw new UnsupportedOperationException("Not supported");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
Expand Down
20 changes: 0 additions & 20 deletions server/src/main/java/org/elasticsearch/threadpool/ThreadPool.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -252,10 +251,6 @@ public long absoluteTimeInMillis() {
return cachedTimeThread.absoluteTimeInMillis();
}

public Counter estimatedTimeInMillisCounter() {
return cachedTimeThread.counter;
}

public ThreadPoolInfo info() {
return threadPoolInfo;
}
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}

Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -73,7 +72,6 @@ public class TestSearchContext extends SearchContext {
final ThreadPool threadPool;
final Map<Class<?>, Collector> queryCollectors = new HashMap<>();
final IndexShard indexShard;
final Counter timeEstimateCounter = Counter.newCounter();
final QuerySearchResult queryResult = new QuerySearchResult();
final QueryShardContext queryShardContext;
ParsedQuery originalQuery;
Expand Down Expand Up @@ -593,8 +591,8 @@ public void doClose() {
}

@Override
public Counter timeEstimateCounter() {
return timeEstimateCounter;
public long getRelativeTimeInMillis() {
return 0L;
}

@Override
Expand Down