Skip to content

Commit d1013ac

Browse files
jimcziAdam Locke
authored andcommitted
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 64c78df commit d1013ac

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
@@ -1147,7 +1147,7 @@ public final Searcher acquireSearcher(String source) {
11471147
throw new AlreadyClosedException("SearcherSupplier was closed");
11481148
}
11491149
final Searcher searcher = acquireSearcherInternal(source);
1150-
return CAN_MATCH_SEARCH_SOURCE.equals(source) ? searcher : wrapper.apply(searcher);
1150+
return wrapper.apply(searcher);
11511151
}
11521152

11531153
@Override

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

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

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

786786
CountDownLatch latch = new CountDownLatch(1);
787787
SearchShardTask task = new SearchShardTask(123L, "", "", "", null, Collections.emptyMap());
788+
assertEquals(8, numWrapInvocations.get());
788789
service.executeQueryPhase(request, task, new ActionListener<SearchPhaseResult>() {
789790
@Override
790791
public void onResponse(SearchPhaseResult searchPhaseResult) {
791792
try {
792793
// make sure that the wrapper is called when the query is actually executed
793-
assertEquals(1, numWrapInvocations.get());
794+
assertEquals(9, numWrapInvocations.get());
794795
} finally {
795796
latch.countDown();
796797
}

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
@@ -94,7 +94,8 @@ protected void doClose() {
9494

9595
@Override
9696
public CacheHelper getReaderCacheHelper() {
97-
throw new UnsupportedOperationException();
97+
// this reader is used for fast operations that don't require caching
98+
return null;
9899
}
99100

100101
// 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)