Remove needless contention in ContextIndexSearcher.timeoutOverwrites (#123609)

This doesn't have to be an instance variable nor does it need any
concurrency, it's per definition a thread-local variable so lets make it
one.
This commit is contained in:
Armin Braun 2025-02-27 17:50:17 +01:00 committed by GitHub
parent 17e35d431b
commit 2113a3c606
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -36,7 +36,6 @@ import org.apache.lucene.util.BitSet;
import org.apache.lucene.util.BitSetIterator;
import org.apache.lucene.util.Bits;
import org.apache.lucene.util.SparseFixedBitSet;
import org.elasticsearch.common.util.concurrent.ConcurrentCollections;
import org.elasticsearch.core.Releasable;
import org.elasticsearch.lucene.util.CombinedBitSet;
import org.elasticsearch.search.dfs.AggregatedDfs;
@ -53,7 +52,6 @@ import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.PriorityQueue;
import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.Executor;
import java.util.stream.Collectors;
@ -82,7 +80,6 @@ public class ContextIndexSearcher extends IndexSearcher implements Releasable {
// don't create slices with less than this number of docs
private final int minimumDocsPerSlice;
private final Set<Thread> timeoutOverwrites = ConcurrentCollections.newConcurrentSet();
private volatile boolean timeExceeded = false;
/** constructor for non-concurrent search */
@ -374,6 +371,8 @@ public class ContextIndexSearcher extends IndexSearcher implements Releasable {
}
}
private static final ThreadLocal<Boolean> timeoutOverwrites = ThreadLocal.withInitial(() -> false);
/**
* Similar to the lucene implementation, with the following changes made:
* 1) postCollection is performed after each segment is collected. This is needed for aggregations, performed by search threads
@ -397,12 +396,12 @@ public class ContextIndexSearcher extends IndexSearcher implements Releasable {
try {
// Search phase has finished, no longer need to check for timeout
// otherwise the aggregation post-collection phase might get cancelled.
boolean added = timeoutOverwrites.add(Thread.currentThread());
assert added;
assert timeoutOverwrites.get() == false;
timeoutOverwrites.set(true);
doAggregationPostCollection(collector);
} finally {
boolean removed = timeoutOverwrites.remove(Thread.currentThread());
assert removed;
assert timeoutOverwrites.get();
timeoutOverwrites.set(false);
}
}
}
@ -420,7 +419,7 @@ public class ContextIndexSearcher extends IndexSearcher implements Releasable {
}
public void throwTimeExceededException() {
if (timeoutOverwrites.contains(Thread.currentThread()) == false) {
if (timeoutOverwrites.get() == false) {
throw new TimeExceededException();
}
}