Skip to content

Commit 2cc2e29

Browse files
committed
Apply the reader wrapper on can_match source (elastic#78988)
Today we don't wrap the original index reader with the reader wrapper plugin if it is acquired from the can_match source. We've made this distinction to ensure that the wrapper plugin doesn't perform any IO during the can_match phase. Now that the security layer loads the role query lazily, this change removes the special path and applies the wrapper in all cases. It also adds an assert to ensure that the loading of the role query is never done on a transport thread.
1 parent 2606e77 commit 2cc2e29

File tree

4 files changed

+8
-4
lines changed

4 files changed

+8
-4
lines changed

server/src/main/java/org/elasticsearch/index/engine/Engine.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1193,7 +1193,7 @@ public final Searcher acquireSearcher(String source) {
11931193
throw new AlreadyClosedException("SearcherSupplier was closed");
11941194
}
11951195
final Searcher searcher = acquireSearcherInternal(source);
1196-
return CAN_MATCH_SEARCH_SOURCE.equals(source) ? searcher : wrapper.apply(searcher);
1196+
return wrapper.apply(searcher);
11971197
}
11981198

11991199
@Override

server/src/test/java/org/elasticsearch/search/SearchServiceTests.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -770,7 +770,7 @@ public void testCanMatch() throws Exception {
770770
searchRequest.source(new SearchSourceBuilder().query(new MatchNoneQueryBuilder()));
771771
assertFalse(service.canMatch(new ShardSearchRequest(OriginalIndices.NONE, searchRequest, indexShard.shardId(), 0, 1,
772772
new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, -1, null)).canMatch());
773-
assertEquals(0, numWrapInvocations.get());
773+
assertEquals(6, numWrapInvocations.get());
774774

775775
ShardSearchRequest request = new ShardSearchRequest(OriginalIndices.NONE, searchRequest, indexShard.shardId(), 0, 1,
776776
new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, -1, null);
@@ -791,12 +791,13 @@ public void testCanMatch() throws Exception {
791791

792792
CountDownLatch latch = new CountDownLatch(1);
793793
SearchShardTask task = new SearchShardTask(123L, "", "", "", null, Collections.emptyMap());
794+
assertEquals(8, numWrapInvocations.get());
794795
service.executeQueryPhase(request, task, new ActionListener<SearchPhaseResult>() {
795796
@Override
796797
public void onResponse(SearchPhaseResult searchPhaseResult) {
797798
try {
798799
// make sure that the wrapper is called when the query is actually executed
799-
assertEquals(1, numWrapInvocations.get());
800+
assertEquals(9, numWrapInvocations.get());
800801
} finally {
801802
latch.countDown();
802803
}

x-pack/plugin/core/src/main/java/org/elasticsearch/index/engine/frozen/RewriteCachingDirectoryReader.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,8 @@ protected void doClose() {
9292

9393
@Override
9494
public CacheHelper getReaderCacheHelper() {
95-
throw new UnsupportedOperationException();
95+
// this reader is used for fast operations that don't require caching
96+
return null;
9697
}
9798

9899
// except of a couple of selected methods everything else will

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetReader.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.elasticsearch.common.cache.CacheBuilder;
2525
import org.elasticsearch.common.logging.LoggerMessageFormat;
2626
import org.elasticsearch.common.lucene.index.SequentialStoredFieldsLeafReader;
27+
import org.elasticsearch.transport.Transports;
2728

2829
import java.io.IOException;
2930
import java.io.UncheckedIOException;
@@ -177,6 +178,7 @@ private void computeNumDocsIfNeeded() {
177178
if (numDocs == -1) {
178179
synchronized (this) {
179180
if (numDocs == -1) {
181+
assert Transports.assertNotTransportThread("resolving role query");
180182
try {
181183
roleQueryBits = bitsetCache.getBitSet(roleQuery, in.getContext());
182184
numDocs = getNumDocs(in, roleQuery, roleQueryBits);

0 commit comments

Comments
 (0)