Skip to content

Commit b72c710

Browse files
s1monws1monw
authored
Cleanup IndexFieldData visibility (#25900)
Today we expose `IndexFieldDataService` outside of IndexService to do maintenance or lookup field data in different ways. Yet, we have a streamlined way to access IndexFieldData via `QueryShardContext` that should encapsulate all access to it. This also ensures that we control all other functionality like cache clearing etc. This change also removes the `recycler` option from `ClearIndicesCacheRequest` this option is a no-op and should have been removed long ago.
1 parent 6d02b45 commit b72c710

File tree

21 files changed

+142
-134
lines changed

21 files changed

+142
-134
lines changed

‎buildSrc/src/main/resources/checkstyle_suppressions.xml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -544,7 +544,6 @@
544544
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]deps[/\\]joda[/\\]SimpleJodaTests.java" checks="LineLength" />
545545
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]discovery[/\\]BlockingClusterStatePublishResponseHandlerTests.java" checks="LineLength" />
546546
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]discovery[/\\]zen[/\\]ZenDiscoveryUnitTests.java" checks="LineLength" />
547-
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]document[/\\]DocumentActionsIT.java" checks="LineLength" />
548547
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]env[/\\]EnvironmentTests.java" checks="LineLength" />
549548
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]env[/\\]NodeEnvironmentTests.java" checks="LineLength" />
550549
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]explain[/\\]ExplainActionIT.java" checks="LineLength" />

‎core/src/main/java/org/elasticsearch/action/admin/indices/cache/clear/ClearIndicesCacheRequest.java‎

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919

2020
package org.elasticsearch.action.admin.indices.cache.clear;
2121

22+
import org.elasticsearch.Version;
2223
import org.elasticsearch.action.support.broadcast.BroadcastRequest;
24+
import org.elasticsearch.common.Strings;
2325
import org.elasticsearch.common.io.stream.StreamInput;
2426
import org.elasticsearch.common.io.stream.StreamOutput;
2527

@@ -29,10 +31,9 @@ public class ClearIndicesCacheRequest extends BroadcastRequest<ClearIndicesCache
2931

3032
private boolean queryCache = false;
3133
private boolean fieldDataCache = false;
32-
private boolean recycler = false;
3334
private boolean requestCache = false;
34-
private String[] fields = null;
35-
35+
private String[] fields = Strings.EMPTY_ARRAY;
36+
3637

3738
public ClearIndicesCacheRequest() {
3839
}
@@ -69,29 +70,22 @@ public ClearIndicesCacheRequest fieldDataCache(boolean fieldDataCache) {
6970
}
7071

7172
public ClearIndicesCacheRequest fields(String... fields) {
72-
this.fields = fields;
73+
this.fields = fields == null ? Strings.EMPTY_ARRAY : fields;
7374
return this;
7475
}
7576

7677
public String[] fields() {
7778
return this.fields;
7879
}
7980

80-
public ClearIndicesCacheRequest recycler(boolean recycler) {
81-
this.recycler = recycler;
82-
return this;
83-
}
84-
85-
public boolean recycler() {
86-
return this.recycler;
87-
}
88-
8981
@Override
9082
public void readFrom(StreamInput in) throws IOException {
9183
super.readFrom(in);
9284
queryCache = in.readBoolean();
9385
fieldDataCache = in.readBoolean();
94-
recycler = in.readBoolean();
86+
if (in.getVersion().before(Version.V_6_0_0_beta1)) {
87+
in.readBoolean(); // recycler
88+
}
9589
fields = in.readStringArray();
9690
requestCache = in.readBoolean();
9791
}
@@ -101,7 +95,9 @@ public void writeTo(StreamOutput out) throws IOException {
10195
super.writeTo(out);
10296
out.writeBoolean(queryCache);
10397
out.writeBoolean(fieldDataCache);
104-
out.writeBoolean(recycler);
98+
if (out.getVersion().before(Version.V_6_0_0_beta1)) {
99+
out.writeBoolean(false); // recycler
100+
}
105101
out.writeStringArrayNullable(fields);
106102
out.writeBoolean(requestCache);
107103
}

‎core/src/main/java/org/elasticsearch/action/admin/indices/cache/clear/TransportClearIndicesCacheAction.java‎

Lines changed: 9 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,6 @@
3232
import org.elasticsearch.common.inject.Inject;
3333
import org.elasticsearch.common.io.stream.StreamInput;
3434
import org.elasticsearch.common.settings.Settings;
35-
import org.elasticsearch.index.IndexService;
36-
import org.elasticsearch.index.shard.IndexShard;
3735
import org.elasticsearch.indices.IndicesService;
3836
import org.elasticsearch.threadpool.ThreadPool;
3937
import org.elasticsearch.transport.TransportService;
@@ -45,16 +43,17 @@
4543
/**
4644
* Indices clear cache action.
4745
*/
48-
public class TransportClearIndicesCacheAction extends TransportBroadcastByNodeAction<ClearIndicesCacheRequest, ClearIndicesCacheResponse, TransportBroadcastByNodeAction.EmptyResult> {
46+
public class TransportClearIndicesCacheAction extends TransportBroadcastByNodeAction<ClearIndicesCacheRequest, ClearIndicesCacheResponse,
47+
TransportBroadcastByNodeAction.EmptyResult> {
4948

5049
private final IndicesService indicesService;
5150

5251
@Inject
5352
public TransportClearIndicesCacheAction(Settings settings, ThreadPool threadPool, ClusterService clusterService,
5453
TransportService transportService, IndicesService indicesService, ActionFilters actionFilters,
5554
IndexNameExpressionResolver indexNameExpressionResolver) {
56-
super(settings, ClearIndicesCacheAction.NAME, threadPool, clusterService, transportService, actionFilters, indexNameExpressionResolver,
57-
ClearIndicesCacheRequest::new, ThreadPool.Names.MANAGEMENT, false);
55+
super(settings, ClearIndicesCacheAction.NAME, threadPool, clusterService, transportService, actionFilters,
56+
indexNameExpressionResolver, ClearIndicesCacheRequest::new, ThreadPool.Names.MANAGEMENT, false);
5857
this.indicesService = indicesService;
5958
}
6059

@@ -64,7 +63,9 @@ protected EmptyResult readShardResult(StreamInput in) throws IOException {
6463
}
6564

6665
@Override
67-
protected ClearIndicesCacheResponse newResponse(ClearIndicesCacheRequest request, int totalShards, int successfulShards, int failedShards, List<EmptyResult> responses, List<ShardOperationFailedException> shardFailures, ClusterState clusterState) {
66+
protected ClearIndicesCacheResponse newResponse(ClearIndicesCacheRequest request, int totalShards, int successfulShards,
67+
int failedShards, List<EmptyResult> responses,
68+
List<ShardOperationFailedException> shardFailures, ClusterState clusterState) {
6869
return new ClearIndicesCacheResponse(totalShards, successfulShards, failedShards, shardFailures);
6970
}
7071

@@ -77,46 +78,8 @@ protected ClearIndicesCacheRequest readRequestFrom(StreamInput in) throws IOExce
7778

7879
@Override
7980
protected EmptyResult shardOperation(ClearIndicesCacheRequest request, ShardRouting shardRouting) {
80-
IndexService service = indicesService.indexService(shardRouting.index());
81-
if (service != null) {
82-
IndexShard shard = service.getShardOrNull(shardRouting.id());
83-
boolean clearedAtLeastOne = false;
84-
if (request.queryCache()) {
85-
clearedAtLeastOne = true;
86-
service.cache().query().clear("api");
87-
}
88-
if (request.fieldDataCache()) {
89-
clearedAtLeastOne = true;
90-
if (request.fields() == null || request.fields().length == 0) {
91-
service.fieldData().clear();
92-
} else {
93-
for (String field : request.fields()) {
94-
service.fieldData().clearField(field);
95-
}
96-
}
97-
}
98-
if (request.requestCache()) {
99-
clearedAtLeastOne = true;
100-
indicesService.clearRequestCache(shard);
101-
}
102-
if (request.recycler()) {
103-
logger.debug("Clear CacheRecycler on index [{}]", service.index());
104-
clearedAtLeastOne = true;
105-
// cacheRecycler.clear();
106-
}
107-
if (!clearedAtLeastOne) {
108-
if (request.fields() != null && request.fields().length > 0) {
109-
// only clear caches relating to the specified fields
110-
for (String field : request.fields()) {
111-
service.fieldData().clearField(field);
112-
}
113-
} else {
114-
service.cache().clear("api");
115-
service.fieldData().clear();
116-
indicesService.clearRequestCache(shard);
117-
}
118-
}
119-
}
81+
indicesService.clearIndexShardCache(shardRouting.shardId(), request.queryCache(), request.fieldDataCache(), request.requestCache(),
82+
request.fields());
12083
return EmptyResult.INSTANCE;
12184
}
12285

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

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ public IndexService(
172172
this.indexStore = indexStore;
173173
indexFieldData.setListener(new FieldDataCacheListener(this));
174174
this.bitsetFilterCache = new BitsetFilterCache(indexSettings, new BitsetCacheListener(this));
175-
this.warmer = new IndexWarmer(indexSettings.getSettings(), threadPool,
175+
this.warmer = new IndexWarmer(indexSettings.getSettings(), threadPool, indexFieldData,
176176
bitsetFilterCache.createListener(threadPool));
177177
this.indexCache = new IndexCache(indexSettings, queryCache, bitsetFilterCache);
178178
this.engineFactory = engineFactory;
@@ -231,10 +231,6 @@ public IndexCache cache() {
231231
return indexCache;
232232
}
233233

234-
public IndexFieldDataService fieldData() {
235-
return indexFieldData;
236-
}
237-
238234
public IndexAnalyzers getIndexAnalyzers() {
239235
return this.mapperService.getIndexAnalyzers();
240236
}
@@ -363,7 +359,7 @@ public synchronized IndexShard createShard(ShardRouting routing) throws IOExcept
363359
store = new Store(shardId, this.indexSettings, indexStore.newDirectoryService(path), lock,
364360
new StoreCloseListener(shardId, () -> eventListener.onStoreClosed(shardId)));
365361
indexShard = new IndexShard(routing, this.indexSettings, path, store, indexSortSupplier,
366-
indexCache, mapperService, similarityService, indexFieldData, engineFactory,
362+
indexCache, mapperService, similarityService, engineFactory,
367363
eventListener, searcherWrapper, threadPool, bigArrays, engineWarmer,
368364
searchOperationListeners, indexingOperationListeners);
369365
eventListener.indexShardStateChanged(indexShard, null, indexShard.state(), "shard created");
@@ -892,4 +888,37 @@ AsyncTranslogFSync getFsyncTask() { // for tests
892888
return fsyncTask;
893889
}
894890

891+
/**
892+
* Clears the caches for the given shard id if the shard is still allocated on this node
893+
*/
894+
public boolean clearCaches(boolean queryCache, boolean fieldDataCache, String...fields) {
895+
boolean clearedAtLeastOne = false;
896+
if (queryCache) {
897+
clearedAtLeastOne = true;
898+
indexCache.query().clear("api");
899+
}
900+
if (fieldDataCache) {
901+
clearedAtLeastOne = true;
902+
if (fields.length == 0) {
903+
indexFieldData.clear();
904+
} else {
905+
for (String field : fields) {
906+
indexFieldData.clearField(field);
907+
}
908+
}
909+
}
910+
if (clearedAtLeastOne == false) {
911+
if (fields.length == 0) {
912+
indexCache.clear("api");
913+
indexFieldData.clear();
914+
} else {
915+
// only clear caches relating to the specified fields
916+
for (String field : fields) {
917+
indexFieldData.clearField(field);
918+
}
919+
}
920+
}
921+
return clearedAtLeastOne;
922+
}
923+
895924
}

‎core/src/main/java/org/elasticsearch/index/IndexWarmer.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,12 @@ public final class IndexWarmer extends AbstractComponent {
4949

5050
private final List<Listener> listeners;
5151

52-
IndexWarmer(Settings settings, ThreadPool threadPool, Listener... listeners) {
52+
IndexWarmer(Settings settings, ThreadPool threadPool, IndexFieldDataService indexFieldDataService,
53+
Listener... listeners) {
5354
super(settings);
5455
ArrayList<Listener> list = new ArrayList<>();
5556
final Executor executor = threadPool.executor(ThreadPool.Names.WARMER);
56-
list.add(new FieldDataWarmer(executor));
57+
list.add(new FieldDataWarmer(executor, indexFieldDataService));
5758

5859
Collections.addAll(list, listeners);
5960
this.listeners = Collections.unmodifiableList(list);
@@ -110,8 +111,11 @@ public interface Listener {
110111
private static class FieldDataWarmer implements IndexWarmer.Listener {
111112

112113
private final Executor executor;
113-
FieldDataWarmer(Executor executor) {
114+
private final IndexFieldDataService indexFieldDataService;
115+
116+
FieldDataWarmer(Executor executor, IndexFieldDataService indexFieldDataService) {
114117
this.executor = executor;
118+
this.indexFieldDataService = indexFieldDataService;
115119
}
116120

117121
@Override
@@ -128,7 +132,6 @@ public TerminationHandle warmReader(final IndexShard indexShard, final Engine.Se
128132
warmUpGlobalOrdinals.put(indexName, fieldType);
129133
}
130134
}
131-
final IndexFieldDataService indexFieldDataService = indexShard.indexFieldDataService();
132135
final CountDownLatch latch = new CountDownLatch(warmUpGlobalOrdinals.size());
133136
for (final MappedFieldType fieldType : warmUpGlobalOrdinals.values()) {
134137
executor.execute(() -> {

‎core/src/main/java/org/elasticsearch/index/shard/IndexShard.java‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@
8585
import org.elasticsearch.index.engine.Segment;
8686
import org.elasticsearch.index.engine.SegmentsStats;
8787
import org.elasticsearch.index.fielddata.FieldDataStats;
88-
import org.elasticsearch.index.fielddata.IndexFieldDataService;
8988
import org.elasticsearch.index.fielddata.ShardFieldData;
9089
import org.elasticsearch.index.flush.FlushStats;
9190
import org.elasticsearch.index.get.GetStats;
@@ -170,7 +169,6 @@ public class IndexShard extends AbstractIndexShardComponent implements IndicesCl
170169
private final ShardIndexWarmerService shardWarmerService;
171170
private final ShardRequestCache requestCacheStats;
172171
private final ShardFieldData shardFieldData;
173-
private final IndexFieldDataService indexFieldDataService;
174172
private final ShardBitsetFilterCache shardBitsetFilterCache;
175173
private final Object mutex = new Object();
176174
private final String checkIndexOnStartup;
@@ -235,7 +233,7 @@ public class IndexShard extends AbstractIndexShardComponent implements IndicesCl
235233

236234
public IndexShard(ShardRouting shardRouting, IndexSettings indexSettings, ShardPath path, Store store,
237235
Supplier<Sort> indexSortSupplier, IndexCache indexCache, MapperService mapperService, SimilarityService similarityService,
238-
IndexFieldDataService indexFieldDataService, @Nullable EngineFactory engineFactory,
236+
@Nullable EngineFactory engineFactory,
239237
IndexEventListener indexEventListener, IndexSearcherWrapper indexSearcherWrapper, ThreadPool threadPool, BigArrays bigArrays,
240238
Engine.Warmer warmer, List<SearchOperationListener> searchOperationListener, List<IndexingOperationListener> listeners) throws IOException {
241239
super(shardRouting.shardId(), indexSettings);
@@ -264,7 +262,6 @@ public IndexShard(ShardRouting shardRouting, IndexSettings indexSettings, ShardP
264262
this.shardWarmerService = new ShardIndexWarmerService(shardId, indexSettings);
265263
this.requestCacheStats = new ShardRequestCache();
266264
this.shardFieldData = new ShardFieldData();
267-
this.indexFieldDataService = indexFieldDataService;
268265
this.shardBitsetFilterCache = new ShardBitsetFilterCache(shardId, indexSettings);
269266
state = IndexShardState.CREATED;
270267
this.path = path;
@@ -320,10 +317,6 @@ public ShardBitsetFilterCache shardBitsetFilterCache() {
320317
return shardBitsetFilterCache;
321318
}
322319

323-
public IndexFieldDataService indexFieldDataService() {
324-
return indexFieldDataService;
325-
}
326-
327320
public MapperService mapperService() {
328321
return mapperService;
329322
}

‎core/src/main/java/org/elasticsearch/indices/IndicesService.java‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import org.elasticsearch.cluster.routing.ShardRouting;
4444
import org.elasticsearch.common.CheckedFunction;
4545
import org.elasticsearch.common.Nullable;
46+
import org.elasticsearch.common.Strings;
4647
import org.elasticsearch.common.breaker.CircuitBreaker;
4748
import org.elasticsearch.common.bytes.BytesReference;
4849
import org.elasticsearch.common.component.AbstractLifecycleComponent;
@@ -1099,13 +1100,6 @@ public boolean canCache(ShardSearchRequest request, SearchContext context) {
10991100

11001101
}
11011102

1102-
public void clearRequestCache(IndexShard shard) {
1103-
if (shard == null) {
1104-
return;
1105-
}
1106-
indicesRequestCache.clear(new IndexShardCacheEntity(shard));
1107-
logger.trace("{} explicit cache clear", shard.shardId());
1108-
}
11091103

11101104
/**
11111105
* Loads the cache result, computing it if needed by executing the query phase and otherwise deserializing the cached
@@ -1240,4 +1234,19 @@ public AliasFilter buildAliasFilter(ClusterState state, String index, String...
12401234
public QueryRewriteContext getRewriteContext(LongSupplier nowInMillis) {
12411235
return new QueryRewriteContext(xContentRegistry, client, nowInMillis);
12421236
}
1237+
1238+
/**
1239+
* Clears the caches for the given shard id if the shard is still allocated on this node
1240+
*/
1241+
public void clearIndexShardCache(ShardId shardId, boolean queryCache, boolean fieldDataCache, boolean requestCache,
1242+
String...fields) {
1243+
final IndexService service = indexService(shardId.getIndex());
1244+
if (service != null) {
1245+
IndexShard shard = service.getShardOrNull(shardId.id());
1246+
final boolean clearedAtLeastOne = service.clearCaches(queryCache, fieldDataCache, fields);
1247+
if ((requestCache || (clearedAtLeastOne == false && fields.length == 0)) && shard != null) {
1248+
indicesRequestCache.clear(new IndexShardCacheEntity(shard));
1249+
}
1250+
}
1251+
}
12431252
}

‎core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestClearIndicesCacheAction.java‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -85,17 +85,11 @@ public static ClearIndicesCacheRequest fromRequest(final RestRequest request, Cl
8585
for (Map.Entry<String, String> entry : request.params().entrySet()) {
8686
if (Fields.QUERY.match(entry.getKey())) {
8787
clearIndicesCacheRequest.queryCache(request.paramAsBoolean(entry.getKey(), clearIndicesCacheRequest.queryCache()));
88-
}
89-
if (Fields.REQUEST.match(entry.getKey())) {
88+
} else if (Fields.REQUEST.match(entry.getKey())) {
9089
clearIndicesCacheRequest.requestCache(request.paramAsBoolean(entry.getKey(), clearIndicesCacheRequest.requestCache()));
91-
}
92-
if (Fields.FIELD_DATA.match(entry.getKey())) {
90+
} else if (Fields.FIELD_DATA.match(entry.getKey())) {
9391
clearIndicesCacheRequest.fieldDataCache(request.paramAsBoolean(entry.getKey(), clearIndicesCacheRequest.fieldDataCache()));
94-
}
95-
if (Fields.RECYCLER.match(entry.getKey())) {
96-
clearIndicesCacheRequest.recycler(request.paramAsBoolean(entry.getKey(), clearIndicesCacheRequest.recycler()));
97-
}
98-
if (Fields.FIELDS.match(entry.getKey())) {
92+
} else if (Fields.FIELDS.match(entry.getKey())) {
9993
clearIndicesCacheRequest.fields(request.paramAsStringArray(entry.getKey(), clearIndicesCacheRequest.fields()));
10094
}
10195
}
@@ -107,7 +101,6 @@ public static class Fields {
107101
public static final ParseField QUERY = new ParseField("query", "filter", "filter_cache");
108102
public static final ParseField REQUEST = new ParseField("request", "request_cache");
109103
public static final ParseField FIELD_DATA = new ParseField("field_data", "fielddata");
110-
public static final ParseField RECYCLER = new ParseField("recycler");
111104
public static final ParseField FIELDS = new ParseField("fields");
112105
}
113106

‎core/src/main/java/org/elasticsearch/search/DefaultSearchContext.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -493,7 +493,6 @@ public BitsetFilterCache bitsetFilterCache() {
493493
return indexService.cache().bitsetFilterCache();
494494
}
495495

496-
497496
@Override
498497
public <IFD extends IndexFieldData<?>> IFD getForField(MappedFieldType fieldType) {
499498
return queryShardContext.getForField(fieldType);

‎core/src/test/java/org/elasticsearch/action/IndicesRequestIT.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919

2020
package org.elasticsearch.action;
2121

22-
import org.apache.lucene.util.LuceneTestCase.AwaitsFix;
2322
import org.elasticsearch.action.admin.indices.alias.Alias;
2423
import org.elasticsearch.action.admin.indices.analyze.AnalyzeAction;
2524
import org.elasticsearch.action.admin.indices.analyze.AnalyzeRequest;

0 commit comments

Comments
 (0)