Skip to content

Commit 99990cd

Browse files
authored
Resolve the role query and the number of docs lazily (#48036)
This commit ensures that the creation of a DocumentSubsetReader does not eagerly resolve the role query and the number of docs that match. We want to delay this expensive operation in order to ensure that we really need this information when we build it. For this reason the role query and the number of docs are now resolved on demand. This commit also depends on https://issues.apache.org/jira/browse/LUCENE-9003 that will also compute the global number of docs lazily.
1 parent a1ae813 commit 99990cd

File tree

1 file changed

+34
-9
lines changed

1 file changed

+34
-9
lines changed

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

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.apache.lucene.util.BitSetIterator;
1818
import org.apache.lucene.util.Bits;
1919
import org.apache.lucene.util.CombinedBitSet;
20+
import org.elasticsearch.ElasticsearchException;
2021
import org.elasticsearch.ExceptionsHelper;
2122
import org.elasticsearch.common.cache.Cache;
2223
import org.elasticsearch.common.cache.CacheBuilder;
@@ -51,7 +52,7 @@ public static DocumentSubsetDirectoryReader wrap(DirectoryReader in, DocumentSub
5152
/**
5253
* Compute the number of live documents. This method is SLOW.
5354
*/
54-
private static int computeNumDocs(LeafReader reader, Query roleQuery, BitSet roleQueryBits) {
55+
private static int computeNumDocs(LeafReader reader, BitSet roleQueryBits) {
5556
final Bits liveDocs = reader.getLiveDocs();
5657
if (roleQueryBits == null) {
5758
return 0;
@@ -103,7 +104,7 @@ private static int getNumDocs(LeafReader reader, Query roleQuery, BitSet roleQue
103104
throw e;
104105
}
105106
}
106-
return perReaderCache.computeIfAbsent(roleQuery, q -> computeNumDocs(reader, roleQuery, roleQueryBits));
107+
return perReaderCache.computeIfAbsent(roleQuery, q -> computeNumDocs(reader, roleQueryBits));
107108
}
108109

109110
public static final class DocumentSubsetDirectoryReader extends FilterDirectoryReader {
@@ -152,21 +153,44 @@ public CacheHelper getReaderCacheHelper() {
152153
}
153154
}
154155

155-
private final BitSet roleQueryBits;
156-
private final int numDocs;
156+
private final DocumentSubsetBitsetCache bitsetCache;
157+
private final Query roleQuery;
158+
// we don't use a volatile here because the bitset is resolved before numDocs in the synchronized block
159+
// so any thread that see numDocs != -1 should also see the true value of the roleQueryBits (happens-before).
160+
private BitSet roleQueryBits;
161+
private volatile int numDocs = -1;
157162

158-
private DocumentSubsetReader(final LeafReader in, DocumentSubsetBitsetCache bitsetCache, final Query roleQuery) throws Exception {
163+
private DocumentSubsetReader(final LeafReader in, DocumentSubsetBitsetCache bitsetCache, final Query roleQuery) {
159164
super(in);
160-
this.roleQueryBits = bitsetCache.getBitSet(roleQuery, in.getContext());
161-
this.numDocs = getNumDocs(in, roleQuery, roleQueryBits);
165+
this.bitsetCache = bitsetCache;
166+
this.roleQuery = roleQuery;
167+
}
168+
169+
/**
170+
* Resolve the role query and the number of docs lazily
171+
*/
172+
private void computeNumDocsIfNeeded() {
173+
if (numDocs == -1) {
174+
synchronized (this) {
175+
if (numDocs == -1) {
176+
try {
177+
roleQueryBits = bitsetCache.getBitSet(roleQuery, in.getContext());
178+
numDocs = getNumDocs(in, roleQuery, roleQueryBits);
179+
} catch (Exception e) {
180+
throw new ElasticsearchException("Failed to load role query", e);
181+
}
182+
}
183+
}
184+
}
162185
}
163186

164187
@Override
165188
public Bits getLiveDocs() {
189+
computeNumDocsIfNeeded();
166190
final Bits actualLiveDocs = in.getLiveDocs();
167191
if (roleQueryBits == null) {
168-
// If we would a <code>null</code> liveDocs then that would mean that no docs are marked as deleted,
169-
// but that isn't the case. No docs match with the role query and therefor all docs are marked as deleted
192+
// If we would return a <code>null</code> liveDocs then that would mean that no docs are marked as deleted,
193+
// but that isn't the case. No docs match with the role query and therefore all docs are marked as deleted
170194
return new Bits.MatchNoBits(in.maxDoc());
171195
} else if (actualLiveDocs == null) {
172196
return roleQueryBits;
@@ -178,6 +202,7 @@ public Bits getLiveDocs() {
178202

179203
@Override
180204
public int numDocs() {
205+
computeNumDocsIfNeeded();
181206
return numDocs;
182207
}
183208

0 commit comments

Comments
 (0)