Skip to content

Commit 81815f0

Browse files
original-brownbearoriginal-brownbear
authored
Simplify QueryCache Construction (#88173)
We don't need to keep references to index settings in all of these, nor do we need per-index loggers or deprecation loggers here. Making the construction here a little cheaper helps with the performance of some cluster state operations that create temporary index-services. Also, removing references to `IndexSettings` and other per-index state is helpful in identifying spots to reduce the per-index overhead on data-nodes down the road.
1 parent d945fce commit 81815f0

File tree

12 files changed

+73
-82
lines changed

12 files changed

+73
-82
lines changed

‎server/src/main/java/org/elasticsearch/index/IndexModule.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
package org.elasticsearch.index;
1010

11+
import org.apache.logging.log4j.LogManager;
12+
import org.apache.logging.log4j.Logger;
1113
import org.apache.lucene.index.DirectoryReader;
1214
import org.apache.lucene.index.FilterDirectoryReader;
1315
import org.apache.lucene.index.IndexReader;
@@ -88,6 +90,8 @@
8890
*/
8991
public final class IndexModule {
9092

93+
private static final Logger logger = LogManager.getLogger(IndexModule.class);
94+
9195
public static final Setting<Boolean> NODE_STORE_ALLOW_MMAP = Setting.boolSetting("node.store.allow_mmap", true, Property.NodeScope);
9296

9397
private static final FsDirectoryFactory DEFAULT_DIRECTORY_FACTORY = new FsDirectoryFactory();
@@ -452,12 +456,13 @@ public IndexService newIndexService(
452456
if (indexSettings.getValue(INDEX_QUERY_CACHE_ENABLED_SETTING)) {
453457
BiFunction<IndexSettings, IndicesQueryCache, QueryCache> queryCacheProvider = forceQueryCacheProvider.get();
454458
if (queryCacheProvider == null) {
455-
queryCache = new IndexQueryCache(indexSettings, indicesQueryCache);
459+
queryCache = new IndexQueryCache(indexSettings.getIndex(), indicesQueryCache);
456460
} else {
457461
queryCache = queryCacheProvider.apply(indexSettings, indicesQueryCache);
458462
}
459463
} else {
460-
queryCache = new DisabledQueryCache(indexSettings);
464+
logger.debug("Using no query cache for [{}]", indexSettings.getIndex());
465+
queryCache = DisabledQueryCache.INSTANCE;
461466
}
462467
if (IndexService.needsMapperService(indexSettings, indexCreationContext)) {
463468
indexAnalyzers = analysisRegistry.build(indexSettings);

‎server/src/main/java/org/elasticsearch/index/IndexService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ public IndexService(
214214
indexFieldData.setListener(new FieldDataCacheListener(this));
215215
this.bitsetFilterCache = new BitsetFilterCache(indexSettings, new BitsetCacheListener(this));
216216
this.warmer = new IndexWarmer(threadPool, indexFieldData, bitsetFilterCache.createListener(threadPool));
217-
this.indexCache = new IndexCache(indexSettings, queryCache, bitsetFilterCache);
217+
this.indexCache = new IndexCache(queryCache, bitsetFilterCache);
218218
} else {
219219
assert indexAnalyzers == null;
220220
this.mapperService = null;

‎server/src/main/java/org/elasticsearch/index/cache/IndexCache.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,18 @@
99
package org.elasticsearch.index.cache;
1010

1111
import org.elasticsearch.core.IOUtils;
12-
import org.elasticsearch.index.AbstractIndexComponent;
13-
import org.elasticsearch.index.IndexSettings;
1412
import org.elasticsearch.index.cache.bitset.BitsetFilterCache;
1513
import org.elasticsearch.index.cache.query.QueryCache;
1614

1715
import java.io.Closeable;
1816
import java.io.IOException;
1917

20-
public class IndexCache extends AbstractIndexComponent implements Closeable {
18+
public class IndexCache implements Closeable {
2119

2220
private final QueryCache queryCache;
2321
private final BitsetFilterCache bitsetFilterCache;
2422

25-
public IndexCache(IndexSettings indexSettings, QueryCache queryCache, BitsetFilterCache bitsetFilterCache) {
26-
super(indexSettings);
23+
public IndexCache(QueryCache queryCache, BitsetFilterCache bitsetFilterCache) {
2724
this.queryCache = queryCache;
2825
this.bitsetFilterCache = bitsetFilterCache;
2926
}

‎server/src/main/java/org/elasticsearch/index/cache/bitset/BitsetFilterCache.java‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
package org.elasticsearch.index.cache.bitset;
1010

11+
import org.apache.logging.log4j.LogManager;
12+
import org.apache.logging.log4j.Logger;
1113
import org.apache.lucene.index.IndexReader;
1214
import org.apache.lucene.index.IndexReaderContext;
1315
import org.apache.lucene.index.LeafReaderContext;
@@ -31,7 +33,7 @@
3133
import org.elasticsearch.common.settings.Setting;
3234
import org.elasticsearch.common.settings.Setting.Property;
3335
import org.elasticsearch.core.TimeValue;
34-
import org.elasticsearch.index.AbstractIndexComponent;
36+
import org.elasticsearch.index.Index;
3537
import org.elasticsearch.index.IndexSettings;
3638
import org.elasticsearch.index.IndexWarmer;
3739
import org.elasticsearch.index.IndexWarmer.TerminationHandle;
@@ -59,7 +61,7 @@
5961
* and require that it should always be around should use this cache, otherwise the
6062
* {@link org.elasticsearch.index.cache.query.QueryCache} should be used instead.
6163
*/
62-
public final class BitsetFilterCache extends AbstractIndexComponent
64+
public final class BitsetFilterCache
6365
implements
6466
IndexReader.ClosedListener,
6567
RemovalListener<IndexReader.CacheKey, Cache<Query, BitsetFilterCache.Value>>,
@@ -71,6 +73,8 @@ public final class BitsetFilterCache extends AbstractIndexComponent
7173
Property.IndexScope
7274
);
7375

76+
private static final Logger logger = LogManager.getLogger(BitsetFilterCache.class);
77+
7478
private final boolean loadRandomAccessFiltersEagerly;
7579

7680
/**
@@ -80,12 +84,14 @@ public final class BitsetFilterCache extends AbstractIndexComponent
8084
private volatile Cache<IndexReader.CacheKey, Cache<Query, Value>> loadedFilters;
8185
private final Listener listener;
8286

87+
private final Index index;
88+
8389
public BitsetFilterCache(IndexSettings indexSettings, Listener listener) {
84-
super(indexSettings);
8590
if (listener == null) {
8691
throw new IllegalArgumentException("listener must not be null");
8792
}
88-
this.loadRandomAccessFiltersEagerly = this.indexSettings.getValue(INDEX_LOAD_RANDOM_ACCESS_FILTERS_EAGERLY_SETTING);
93+
this.index = indexSettings.getIndex();
94+
this.loadRandomAccessFiltersEagerly = indexSettings.getValue(INDEX_LOAD_RANDOM_ACCESS_FILTERS_EAGERLY_SETTING);
8995
this.listener = listener;
9096
}
9197

@@ -124,7 +130,7 @@ public void close() {
124130
}
125131

126132
public void clear(String reason) {
127-
logger.debug("clearing all bitsets because [{}]", reason);
133+
logger.debug("clearing all bitsets for [{}] because [{}]", index, reason);
128134
var filters = loadedFilters;
129135
if (filters != null) {
130136
filters.invalidateAll();
@@ -144,11 +150,9 @@ private BitSet getAndLoadIfNotPresent(final Query query, final LeafReaderContext
144150
+ "see for example AggregatorTestCase#wrapInMockESDirectoryReader. If you got here in production, please file a bug."
145151
);
146152
}
147-
if (indexSettings.getIndex().equals(shardId.getIndex()) == false) {
153+
if (index.equals(shardId.getIndex()) == false) {
148154
// insanity
149-
throw new IllegalStateException(
150-
"Trying to load bit set for index " + shardId.getIndex() + " with cache of index " + indexSettings.getIndex()
151-
);
155+
throw new IllegalStateException("Trying to load bit set for index " + shardId.getIndex() + " with cache of index " + index);
152156
}
153157
var filters = loadedFilters;
154158
if (filters == null) {
@@ -249,7 +253,7 @@ final class BitSetProducerWarmer implements IndexWarmer.Listener {
249253

250254
@Override
251255
public IndexWarmer.TerminationHandle warmReader(final IndexShard indexShard, final ElasticsearchDirectoryReader reader) {
252-
if (indexSettings.getIndex().equals(indexShard.indexSettings().getIndex()) == false) {
256+
if (index.equals(indexShard.indexSettings().getIndex()) == false) {
253257
// this is from a different index
254258
return TerminationHandle.NO_WAIT;
255259
}

‎server/src/main/java/org/elasticsearch/index/cache/query/DisabledQueryCache.java‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,12 @@
1010

1111
import org.apache.lucene.search.QueryCachingPolicy;
1212
import org.apache.lucene.search.Weight;
13-
import org.elasticsearch.index.AbstractIndexComponent;
14-
import org.elasticsearch.index.IndexSettings;
1513

16-
public class DisabledQueryCache extends AbstractIndexComponent implements QueryCache {
14+
public class DisabledQueryCache implements QueryCache {
1715

18-
public DisabledQueryCache(IndexSettings indexSettings) {
19-
super(indexSettings);
20-
logger.debug("Using no query cache");
21-
}
16+
public static final DisabledQueryCache INSTANCE = new DisabledQueryCache();
17+
18+
private DisabledQueryCache() {}
2219

2320
@Override
2421
public void close() {

‎server/src/main/java/org/elasticsearch/index/cache/query/IndexQueryCache.java‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,24 +8,29 @@
88

99
package org.elasticsearch.index.cache.query;
1010

11+
import org.apache.logging.log4j.LogManager;
12+
import org.apache.logging.log4j.Logger;
1113
import org.apache.lucene.search.QueryCachingPolicy;
1214
import org.apache.lucene.search.Weight;
1315
import org.elasticsearch.ElasticsearchException;
14-
import org.elasticsearch.index.AbstractIndexComponent;
15-
import org.elasticsearch.index.IndexSettings;
16+
import org.elasticsearch.index.Index;
1617
import org.elasticsearch.indices.IndicesQueryCache;
1718

1819
/**
1920
* The index-level query cache. This class mostly delegates to the node-level
2021
* query cache: {@link IndicesQueryCache}.
2122
*/
22-
public class IndexQueryCache extends AbstractIndexComponent implements QueryCache {
23+
public class IndexQueryCache implements QueryCache {
2324

24-
final IndicesQueryCache indicesQueryCache;
25+
private static final Logger logger = LogManager.getLogger(IndexQueryCache.class);
2526

26-
public IndexQueryCache(IndexSettings indexSettings, IndicesQueryCache indicesQueryCache) {
27-
super(indexSettings);
27+
private final IndicesQueryCache indicesQueryCache;
28+
29+
protected final Index index;
30+
31+
public IndexQueryCache(Index index, IndicesQueryCache indicesQueryCache) {
2832
this.indicesQueryCache = indicesQueryCache;
33+
this.index = index;
2934
}
3035

3136
@Override
@@ -35,8 +40,8 @@ public void close() throws ElasticsearchException {
3540

3641
@Override
3742
public void clear(String reason) {
38-
logger.debug("full cache clear, reason [{}]", reason);
39-
indicesQueryCache.clearIndex(index().getName());
43+
logger.debug("full cache clear for [{}], reason [{}]", index, reason);
44+
indicesQueryCache.clearIndex(index.getName());
4045
}
4146

4247
@Override

‎server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorTests.java‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.elasticsearch.common.lucene.search.Queries;
3737
import org.elasticsearch.common.time.DateFormatter;
3838
import org.elasticsearch.core.CheckedConsumer;
39+
import org.elasticsearch.index.IndexSettings;
3940
import org.elasticsearch.index.cache.bitset.BitsetFilterCache;
4041
import org.elasticsearch.index.mapper.DateFieldMapper;
4142
import org.elasticsearch.index.mapper.DateFieldMapper.Resolution;
@@ -650,15 +651,16 @@ public void testMatchAllOnFilteredIndex() throws IOException {
650651
indexWriter.close();
651652

652653
try (DirectoryReader directoryReader = DirectoryReader.open(directory)) {
653-
BitsetFilterCache bitsetFilterCache = new BitsetFilterCache(createIndexSettings(), new BitsetFilterCache.Listener() {
654+
final IndexSettings indexSettings = createIndexSettings();
655+
BitsetFilterCache bitsetFilterCache = new BitsetFilterCache(indexSettings, new BitsetFilterCache.Listener() {
654656
@Override
655657
public void onRemoval(ShardId shardId, Accountable accountable) {}
656658

657659
@Override
658660
public void onCache(ShardId shardId, Accountable accountable) {}
659661
});
660662
IndexReader limitedReader = new DocumentSubsetDirectoryReader(
661-
ElasticsearchDirectoryReader.wrap(directoryReader, new ShardId(bitsetFilterCache.index(), 0)),
663+
ElasticsearchDirectoryReader.wrap(directoryReader, new ShardId(indexSettings.getIndex(), 0)),
662664
bitsetFilterCache,
663665
LongPoint.newRangeQuery("t", 5, Long.MAX_VALUE)
664666
);
@@ -726,15 +728,16 @@ public void testTermOnFilteredIndex() throws IOException {
726728
indexWriter.close();
727729

728730
try (DirectoryReader directoryReader = DirectoryReader.open(directory)) {
729-
BitsetFilterCache bitsetFilterCache = new BitsetFilterCache(createIndexSettings(), new BitsetFilterCache.Listener() {
731+
final IndexSettings indexSettings = createIndexSettings();
732+
BitsetFilterCache bitsetFilterCache = new BitsetFilterCache(indexSettings, new BitsetFilterCache.Listener() {
730733
@Override
731734
public void onRemoval(ShardId shardId, Accountable accountable) {}
732735

733736
@Override
734737
public void onCache(ShardId shardId, Accountable accountable) {}
735738
});
736739
IndexReader limitedReader = new DocumentSubsetDirectoryReader(
737-
ElasticsearchDirectoryReader.wrap(directoryReader, new ShardId(bitsetFilterCache.index(), 0)),
740+
ElasticsearchDirectoryReader.wrap(directoryReader, new ShardId(indexSettings.getIndex(), 0)),
738741
bitsetFilterCache,
739742
LongPoint.newRangeQuery("t", 5, Long.MAX_VALUE)
740743
);
@@ -791,15 +794,16 @@ public void testTermOnFilterWithMatchAll() throws IOException {
791794
indexWriter.close();
792795

793796
try (DirectoryReader directoryReader = DirectoryReader.open(directory)) {
794-
BitsetFilterCache bitsetFilterCache = new BitsetFilterCache(createIndexSettings(), new BitsetFilterCache.Listener() {
797+
final IndexSettings indexSettings = createIndexSettings();
798+
BitsetFilterCache bitsetFilterCache = new BitsetFilterCache(indexSettings, new BitsetFilterCache.Listener() {
795799
@Override
796800
public void onRemoval(ShardId shardId, Accountable accountable) {}
797801

798802
@Override
799803
public void onCache(ShardId shardId, Accountable accountable) {}
800804
});
801805
IndexReader limitedReader = new DocumentSubsetDirectoryReader(
802-
ElasticsearchDirectoryReader.wrap(directoryReader, new ShardId(bitsetFilterCache.index(), 0)),
806+
ElasticsearchDirectoryReader.wrap(directoryReader, new ShardId(indexSettings.getIndex(), 0)),
803807
bitsetFilterCache,
804808
LongPoint.newRangeQuery("t", Long.MIN_VALUE, Long.MAX_VALUE)
805809
);

‎test/framework/src/main/java/org/elasticsearch/index/shard/IndexShardTestCase.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ protected IndexShard newShard(
425425
}
426426
boolean success = false;
427427
try {
428-
IndexCache indexCache = new IndexCache(indexSettings, new DisabledQueryCache(indexSettings), null);
428+
IndexCache indexCache = new IndexCache(DisabledQueryCache.INSTANCE, null);
429429
MapperService mapperService = MapperTestUtils.newMapperService(
430430
xContentRegistry(),
431431
createTempDir(),

‎test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import org.apache.lucene.search.IndexSearcher;
2828
import org.apache.lucene.search.MatchAllDocsQuery;
2929
import org.apache.lucene.search.Query;
30-
import org.apache.lucene.search.QueryCache;
3130
import org.apache.lucene.search.ScoreMode;
3231
import org.apache.lucene.search.Sort;
3332
import org.apache.lucene.search.SortField;
@@ -375,13 +374,12 @@ private SubSearchContext buildSubSearchContext(
375374
BitsetFilterCache bitsetFilterCache
376375
) {
377376
SearchContext ctx = mock(SearchContext.class);
378-
QueryCache queryCache = new DisabledQueryCache(indexSettings);
379377
try {
380378
when(ctx.searcher()).thenReturn(
381379
new ContextIndexSearcher(
382380
searchExecutionContext.searcher().getIndexReader(),
383381
searchExecutionContext.searcher().getSimilarity(),
384-
queryCache,
382+
DisabledQueryCache.INSTANCE,
385383
TrivialQueryCachingPolicy.NEVER,
386384
false
387385
)

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1160,11 +1160,9 @@ public void onIndexModule(IndexModule module) {
11601160
* implementation disables the query cache if field level security is used for a particular request. We have to forcefully
11611161
* overwrite the query cache implementation to prevent data leakage to unauthorized users.
11621162
*/
1163-
module.forceQueryCacheProvider((indexSettings, cache) -> {
1164-
final OptOutQueryCache queryCache = new OptOutQueryCache(indexSettings, cache, threadContext.get());
1165-
1166-
return queryCache;
1167-
});
1163+
module.forceQueryCacheProvider(
1164+
(indexSettings, cache) -> new OptOutQueryCache(indexSettings.getIndex(), cache, threadContext.get())
1165+
);
11681166
}
11691167

11701168
// in order to prevent scroll ids from being maliciously crafted and/or guessed, a listener is added that

0 commit comments

Comments
 (0)