From 4bb11184623239a72da13a47b4667962e6ea6963 Mon Sep 17 00:00:00 2001 From: jimczi Date: Tue, 12 Oct 2021 14:42:42 +0200 Subject: [PATCH 1/3] Apply the reader wrapper on can_match source 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. --- server/src/main/java/org/elasticsearch/index/engine/Engine.java | 2 +- .../core/security/authz/accesscontrol/DocumentSubsetReader.java | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/Engine.java b/server/src/main/java/org/elasticsearch/index/engine/Engine.java index aacf9917ab008..53719a236a7da 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/Engine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/Engine.java @@ -1134,7 +1134,7 @@ public final Searcher acquireSearcher(String source) { throw new AlreadyClosedException("SearcherSupplier was closed"); } final Searcher searcher = acquireSearcherInternal(source); - return CAN_MATCH_SEARCH_SOURCE.equals(source) ? searcher : wrapper.apply(searcher); + return wrapper.apply(searcher); } @Override diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetReader.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetReader.java index ad5b91a5f128a..01dbe0d75e38d 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetReader.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetReader.java @@ -24,6 +24,7 @@ import org.elasticsearch.common.cache.CacheBuilder; import org.elasticsearch.common.logging.LoggerMessageFormat; import org.elasticsearch.common.lucene.index.SequentialStoredFieldsLeafReader; +import org.elasticsearch.transport.Transports; import java.io.IOException; import java.io.UncheckedIOException; @@ -177,6 +178,7 @@ private void computeNumDocsIfNeeded() { if (numDocs == -1) { synchronized (this) { if (numDocs == -1) { + assert Transports.assertNotTransportThread("resolving role query"); try { roleQueryBits = bitsetCache.getBitSet(roleQuery, in.getContext()); numDocs = getNumDocs(in, roleQuery, roleQueryBits); From ed7de5b497ef6eab9a4354d0289cc9336d7a1daa Mon Sep 17 00:00:00 2001 From: jimczi Date: Tue, 12 Oct 2021 15:40:13 +0200 Subject: [PATCH 2/3] fix UOE --- .../index/engine/frozen/RewriteCachingDirectoryReader.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/index/engine/frozen/RewriteCachingDirectoryReader.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/index/engine/frozen/RewriteCachingDirectoryReader.java index a53ab745a4c89..5e7dbfff564e7 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/index/engine/frozen/RewriteCachingDirectoryReader.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/index/engine/frozen/RewriteCachingDirectoryReader.java @@ -94,7 +94,8 @@ protected void doClose() { @Override public CacheHelper getReaderCacheHelper() { - throw new UnsupportedOperationException(); + // this reader is used for fast operations that don't require caching + return null; } // except of a couple of selected methods everything else will From 7e05234a3b0db71fce04a6dd355a7beff8a45761 Mon Sep 17 00:00:00 2001 From: jimczi Date: Tue, 12 Oct 2021 15:45:09 +0200 Subject: [PATCH 3/3] fix expectation in tests --- .../java/org/elasticsearch/search/SearchServiceTests.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java index 3f7a2b30e4099..5704521505e4f 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java @@ -761,7 +761,7 @@ public void testCanMatch() throws Exception { searchRequest.source(new SearchSourceBuilder().query(new MatchNoneQueryBuilder())); assertFalse(service.canMatch(new ShardSearchRequest(OriginalIndices.NONE, searchRequest, indexShard.shardId(), 0, 1, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, -1, null)).canMatch()); - assertEquals(0, numWrapInvocations.get()); + assertEquals(6, numWrapInvocations.get()); ShardSearchRequest request = new ShardSearchRequest(OriginalIndices.NONE, searchRequest, indexShard.shardId(), 0, 1, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, -1, null); @@ -782,12 +782,13 @@ public void testCanMatch() throws Exception { CountDownLatch latch = new CountDownLatch(1); SearchShardTask task = new SearchShardTask(123L, "", "", "", null, Collections.emptyMap()); + assertEquals(8, numWrapInvocations.get()); service.executeQueryPhase(request, task, new ActionListener() { @Override public void onResponse(SearchPhaseResult searchPhaseResult) { try { // make sure that the wrapper is called when the query is actually executed - assertEquals(1, numWrapInvocations.get()); + assertEquals(9, numWrapInvocations.get()); } finally { latch.countDown(); }