Skip to content

Cleanup IndexFieldData visibility #25900

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 26, 2017
Merged

Cleanup IndexFieldData visibility #25900

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
20968bb
Cleanup IndexFieldData visibility
s1monw Jul 26, 2017
dc67452
Merge branch 'master' into cleanup_fiel_data_cache_access
s1monw Jul 26, 2017
882cda5
fix compile errors
s1monw Jul 26, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion buildSrc/src/main/resources/checkstyle_suppressions.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,6 @@
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]deps[/\\]joda[/\\]SimpleJodaTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]discovery[/\\]BlockingClusterStatePublishResponseHandlerTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]discovery[/\\]zen[/\\]ZenDiscoveryUnitTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]document[/\\]DocumentActionsIT.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]env[/\\]EnvironmentTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]env[/\\]NodeEnvironmentTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]explain[/\\]ExplainActionIT.java" checks="LineLength" />
Expand Down
26 changes: 11 additions & 15 deletions ...ain/java/org/elasticsearch/action/admin/indices/cache/clear/ClearIndicesCacheRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@

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

import org.elasticsearch.Version;
import org.elasticsearch.action.support.broadcast.BroadcastRequest;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;

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

private boolean queryCache = false;
private boolean fieldDataCache = false;
private boolean recycler = false;
private boolean requestCache = false;
private String[] fields = null;
private String[] fields = Strings.EMPTY_ARRAY;
Copy link
Contributor

@jpountz jpountz Jul 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that change alone makes me happy



public ClearIndicesCacheRequest() {
}
Expand Down Expand Up @@ -69,29 +70,22 @@ public ClearIndicesCacheRequest fieldDataCache(boolean fieldDataCache) {
}

public ClearIndicesCacheRequest fields(String... fields) {
this.fields = fields;
this.fields = fields == null ? Strings.EMPTY_ARRAY : fields;
return this;
}

public String[] fields() {
return this.fields;
}

public ClearIndicesCacheRequest recycler(boolean recycler) {
this.recycler = recycler;
return this;
}

public boolean recycler() {
return this.recycler;
}

@Override
public void readFrom(StreamInput in) throws IOException {
super.readFrom(in);
queryCache = in.readBoolean();
fieldDataCache = in.readBoolean();
recycler = in.readBoolean();
if (in.getVersion().before(Version.V_6_0_0_beta1)) {
in.readBoolean(); // recycler
}
fields = in.readStringArray();
requestCache = in.readBoolean();
}
Expand All @@ -101,7 +95,9 @@ public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeBoolean(queryCache);
out.writeBoolean(fieldDataCache);
out.writeBoolean(recycler);
if (out.getVersion().before(Version.V_6_0_0_beta1)) {
out.writeBoolean(false); // recycler
}
out.writeStringArrayNullable(fields);
out.writeBoolean(requestCache);
}
Expand Down
55 changes: 9 additions & 46 deletions .../org/elasticsearch/action/admin/indices/cache/clear/TransportClearIndicesCacheAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.IndexService;
import org.elasticsearch.index.shard.IndexShard;
import org.elasticsearch.indices.IndicesService;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService;
Expand All @@ -45,16 +43,17 @@
/**
* Indices clear cache action.
*/
public class TransportClearIndicesCacheAction extends TransportBroadcastByNodeAction<ClearIndicesCacheRequest, ClearIndicesCacheResponse, TransportBroadcastByNodeAction.EmptyResult> {
public class TransportClearIndicesCacheAction extends TransportBroadcastByNodeAction<ClearIndicesCacheRequest, ClearIndicesCacheResponse,
TransportBroadcastByNodeAction.EmptyResult> {

private final IndicesService indicesService;

@Inject
public TransportClearIndicesCacheAction(Settings settings, ThreadPool threadPool, ClusterService clusterService,
TransportService transportService, IndicesService indicesService, ActionFilters actionFilters,
IndexNameExpressionResolver indexNameExpressionResolver) {
super(settings, ClearIndicesCacheAction.NAME, threadPool, clusterService, transportService, actionFilters, indexNameExpressionResolver,
ClearIndicesCacheRequest::new, ThreadPool.Names.MANAGEMENT, false);
super(settings, ClearIndicesCacheAction.NAME, threadPool, clusterService, transportService, actionFilters,
indexNameExpressionResolver, ClearIndicesCacheRequest::new, ThreadPool.Names.MANAGEMENT, false);
this.indicesService = indicesService;
}

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

@Override
protected ClearIndicesCacheResponse newResponse(ClearIndicesCacheRequest request, int totalShards, int successfulShards, int failedShards, List<EmptyResult> responses, List<ShardOperationFailedException> shardFailures, ClusterState clusterState) {
protected ClearIndicesCacheResponse newResponse(ClearIndicesCacheRequest request, int totalShards, int successfulShards,
int failedShards, List<EmptyResult> responses,
List<ShardOperationFailedException> shardFailures, ClusterState clusterState) {
return new ClearIndicesCacheResponse(totalShards, successfulShards, failedShards, shardFailures);
}

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

@Override
protected EmptyResult shardOperation(ClearIndicesCacheRequest request, ShardRouting shardRouting) {
IndexService service = indicesService.indexService(shardRouting.index());
if (service != null) {
IndexShard shard = service.getShardOrNull(shardRouting.id());
boolean clearedAtLeastOne = false;
if (request.queryCache()) {
clearedAtLeastOne = true;
service.cache().query().clear("api");
}
if (request.fieldDataCache()) {
clearedAtLeastOne = true;
if (request.fields() == null || request.fields().length == 0) {
service.fieldData().clear();
} else {
for (String field : request.fields()) {
service.fieldData().clearField(field);
}
}
}
if (request.requestCache()) {
clearedAtLeastOne = true;
indicesService.clearRequestCache(shard);
}
if (request.recycler()) {
logger.debug("Clear CacheRecycler on index [{}]", service.index());
clearedAtLeastOne = true;
// cacheRecycler.clear();
}
if (!clearedAtLeastOne) {
if (request.fields() != null && request.fields().length > 0) {
// only clear caches relating to the specified fields
for (String field : request.fields()) {
service.fieldData().clearField(field);
}
} else {
service.cache().clear("api");
service.fieldData().clear();
indicesService.clearRequestCache(shard);
}
}
}
indicesService.clearIndexShardCache(shardRouting.shardId(), request.queryCache(), request.fieldDataCache(), request.requestCache(),
request.fields());
return EmptyResult.INSTANCE;
}

Expand Down
41 changes: 35 additions & 6 deletions core/src/main/java/org/elasticsearch/index/IndexService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ public IndexService(
this.indexStore = indexStore;
indexFieldData.setListener(new FieldDataCacheListener(this));
this.bitsetFilterCache = new BitsetFilterCache(indexSettings, new BitsetCacheListener(this));
this.warmer = new IndexWarmer(indexSettings.getSettings(), threadPool,
this.warmer = new IndexWarmer(indexSettings.getSettings(), threadPool, indexFieldData,
bitsetFilterCache.createListener(threadPool));
this.indexCache = new IndexCache(indexSettings, queryCache, bitsetFilterCache);
this.engineFactory = engineFactory;
Expand Down Expand Up @@ -231,10 +231,6 @@ public IndexCache cache() {
return indexCache;
}

public IndexFieldDataService fieldData() {
return indexFieldData;
}

public IndexAnalyzers getIndexAnalyzers() {
return this.mapperService.getIndexAnalyzers();
}
Expand Down Expand Up @@ -363,7 +359,7 @@ public synchronized IndexShard createShard(ShardRouting routing) throws IOExcept
store = new Store(shardId, this.indexSettings, indexStore.newDirectoryService(path), lock,
new StoreCloseListener(shardId, () -> eventListener.onStoreClosed(shardId)));
indexShard = new IndexShard(routing, this.indexSettings, path, store, indexSortSupplier,
indexCache, mapperService, similarityService, indexFieldData, engineFactory,
indexCache, mapperService, similarityService, engineFactory,
eventListener, searcherWrapper, threadPool, bigArrays, engineWarmer,
searchOperationListeners, indexingOperationListeners);
eventListener.indexShardStateChanged(indexShard, null, indexShard.state(), "shard created");
Expand Down Expand Up @@ -892,4 +888,37 @@ AsyncTranslogFSync getFsyncTask() { // for tests
return fsyncTask;
}

/**
* Clears the caches for the given shard id if the shard is still allocated on this node
*/
public boolean clearCaches(boolean queryCache, boolean fieldDataCache, String...fields) {
boolean clearedAtLeastOne = false;
if (queryCache) {
clearedAtLeastOne = true;
indexCache.query().clear("api");
}
if (fieldDataCache) {
clearedAtLeastOne = true;
if (fields.length == 0) {
indexFieldData.clear();
} else {
for (String field : fields) {
indexFieldData.clearField(field);
}
}
}
if (clearedAtLeastOne == false) {
if (fields.length == 0) {
indexCache.clear("api");
indexFieldData.clear();
} else {
// only clear caches relating to the specified fields
for (String field : fields) {
indexFieldData.clearField(field);
}
}
}
Copy link
Contributor

@jpountz jpountz Jul 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you did not change that logic but it confuses me a bit as it seems we always clear fileddata even when fieldDataCache is false?

Copy link
Contributor Author

@s1monw s1monw Jul 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah we are on the same page. I will open a followup with a BWC break

return clearedAtLeastOne;
}

}
11 changes: 7 additions & 4 deletions core/src/main/java/org/elasticsearch/index/IndexWarmer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,12 @@ public final class IndexWarmer extends AbstractComponent {

private final List<Listener> listeners;

IndexWarmer(Settings settings, ThreadPool threadPool, Listener... listeners) {
IndexWarmer(Settings settings, ThreadPool threadPool, IndexFieldDataService indexFieldDataService,
Listener... listeners) {
super(settings);
ArrayList<Listener> list = new ArrayList<>();
final Executor executor = threadPool.executor(ThreadPool.Names.WARMER);
list.add(new FieldDataWarmer(executor));
list.add(new FieldDataWarmer(executor, indexFieldDataService));

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

private final Executor executor;
FieldDataWarmer(Executor executor) {
private final IndexFieldDataService indexFieldDataService;

FieldDataWarmer(Executor executor, IndexFieldDataService indexFieldDataService) {
this.executor = executor;
this.indexFieldDataService = indexFieldDataService;
}

@Override
Expand All @@ -128,7 +132,6 @@ public TerminationHandle warmReader(final IndexShard indexShard, final Engine.Se
warmUpGlobalOrdinals.put(indexName, fieldType);
}
}
final IndexFieldDataService indexFieldDataService = indexShard.indexFieldDataService();
final CountDownLatch latch = new CountDownLatch(warmUpGlobalOrdinals.size());
for (final MappedFieldType fieldType : warmUpGlobalOrdinals.values()) {
executor.execute(() -> {
Expand Down
9 changes: 1 addition & 8 deletions core/src/main/java/org/elasticsearch/index/shard/IndexShard.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@
import org.elasticsearch.index.engine.Segment;
import org.elasticsearch.index.engine.SegmentsStats;
import org.elasticsearch.index.fielddata.FieldDataStats;
import org.elasticsearch.index.fielddata.IndexFieldDataService;
import org.elasticsearch.index.fielddata.ShardFieldData;
import org.elasticsearch.index.flush.FlushStats;
import org.elasticsearch.index.get.GetStats;
Expand Down Expand Up @@ -170,7 +169,6 @@ public class IndexShard extends AbstractIndexShardComponent implements IndicesCl
private final ShardIndexWarmerService shardWarmerService;
private final ShardRequestCache requestCacheStats;
private final ShardFieldData shardFieldData;
private final IndexFieldDataService indexFieldDataService;
private final ShardBitsetFilterCache shardBitsetFilterCache;
private final Object mutex = new Object();
private final String checkIndexOnStartup;
Expand Down Expand Up @@ -235,7 +233,7 @@ public class IndexShard extends AbstractIndexShardComponent implements IndicesCl

public IndexShard(ShardRouting shardRouting, IndexSettings indexSettings, ShardPath path, Store store,
Supplier<Sort> indexSortSupplier, IndexCache indexCache, MapperService mapperService, SimilarityService similarityService,
IndexFieldDataService indexFieldDataService, @Nullable EngineFactory engineFactory,
@Nullable EngineFactory engineFactory,
IndexEventListener indexEventListener, IndexSearcherWrapper indexSearcherWrapper, ThreadPool threadPool, BigArrays bigArrays,
Engine.Warmer warmer, List<SearchOperationListener> searchOperationListener, List<IndexingOperationListener> listeners) throws IOException {
super(shardRouting.shardId(), indexSettings);
Expand Down Expand Up @@ -264,7 +262,6 @@ public IndexShard(ShardRouting shardRouting, IndexSettings indexSettings, ShardP
this.shardWarmerService = new ShardIndexWarmerService(shardId, indexSettings);
this.requestCacheStats = new ShardRequestCache();
this.shardFieldData = new ShardFieldData();
this.indexFieldDataService = indexFieldDataService;
this.shardBitsetFilterCache = new ShardBitsetFilterCache(shardId, indexSettings);
state = IndexShardState.CREATED;
this.path = path;
Expand Down Expand Up @@ -320,10 +317,6 @@ public ShardBitsetFilterCache shardBitsetFilterCache() {
return shardBitsetFilterCache;
}

public IndexFieldDataService indexFieldDataService() {
return indexFieldDataService;
}

public MapperService mapperService() {
return mapperService;
}
Expand Down
23 changes: 16 additions & 7 deletions core/src/main/java/org/elasticsearch/indices/IndicesService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.elasticsearch.cluster.routing.ShardRouting;
import org.elasticsearch.common.CheckedFunction;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.breaker.CircuitBreaker;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.component.AbstractLifecycleComponent;
Expand Down Expand Up @@ -1099,13 +1100,6 @@ public boolean canCache(ShardSearchRequest request, SearchContext context) {

}

public void clearRequestCache(IndexShard shard) {
if (shard == null) {
return;
}
indicesRequestCache.clear(new IndexShardCacheEntity(shard));
logger.trace("{} explicit cache clear", shard.shardId());
}

/**
* Loads the cache result, computing it if needed by executing the query phase and otherwise deserializing the cached
Expand Down Expand Up @@ -1240,4 +1234,19 @@ public AliasFilter buildAliasFilter(ClusterState state, String index, String...
public QueryRewriteContext getRewriteContext(LongSupplier nowInMillis) {
return new QueryRewriteContext(xContentRegistry, client, nowInMillis);
}

/**
* Clears the caches for the given shard id if the shard is still allocated on this node
*/
public void clearIndexShardCache(ShardId shardId, boolean queryCache, boolean fieldDataCache, boolean requestCache,
String...fields) {
final IndexService service = indexService(shardId.getIndex());
if (service != null) {
IndexShard shard = service.getShardOrNull(shardId.id());
final boolean clearedAtLeastOne = service.clearCaches(queryCache, fieldDataCache, fields);
if ((requestCache || (clearedAtLeastOne == false && fields.length == 0)) && shard != null) {
indicesRequestCache.clear(new IndexShardCacheEntity(shard));
}
}
}
}
13 changes: 3 additions & 10 deletions ...rc/main/java/org/elasticsearch/rest/action/admin/indices/RestClearIndicesCacheAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,11 @@ public static ClearIndicesCacheRequest fromRequest(final RestRequest request, Cl
for (Map.Entry<String, String> entry : request.params().entrySet()) {
if (Fields.QUERY.match(entry.getKey())) {
clearIndicesCacheRequest.queryCache(request.paramAsBoolean(entry.getKey(), clearIndicesCacheRequest.queryCache()));
}
if (Fields.REQUEST.match(entry.getKey())) {
} else if (Fields.REQUEST.match(entry.getKey())) {
clearIndicesCacheRequest.requestCache(request.paramAsBoolean(entry.getKey(), clearIndicesCacheRequest.requestCache()));
}
if (Fields.FIELD_DATA.match(entry.getKey())) {
} else if (Fields.FIELD_DATA.match(entry.getKey())) {
clearIndicesCacheRequest.fieldDataCache(request.paramAsBoolean(entry.getKey(), clearIndicesCacheRequest.fieldDataCache()));
}
if (Fields.RECYCLER.match(entry.getKey())) {
clearIndicesCacheRequest.recycler(request.paramAsBoolean(entry.getKey(), clearIndicesCacheRequest.recycler()));
}
if (Fields.FIELDS.match(entry.getKey())) {
} else if (Fields.FIELDS.match(entry.getKey())) {
clearIndicesCacheRequest.fields(request.paramAsStringArray(entry.getKey(), clearIndicesCacheRequest.fields()));
}
}
Expand All @@ -107,7 +101,6 @@ public static class Fields {
public static final ParseField QUERY = new ParseField("query", "filter", "filter_cache");
public static final ParseField REQUEST = new ParseField("request", "request_cache");
public static final ParseField FIELD_DATA = new ParseField("field_data", "fielddata");
public static final ParseField RECYCLER = new ParseField("recycler");
public static final ParseField FIELDS = new ParseField("fields");
}

Expand Down
1 change: 0 additions & 1 deletion core/src/main/java/org/elasticsearch/search/DefaultSearchContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,6 @@ public BitsetFilterCache bitsetFilterCache() {
return indexService.cache().bitsetFilterCache();
}


@Override
public <IFD extends IndexFieldData<?>> IFD getForField(MappedFieldType fieldType) {
return queryShardContext.getForField(fieldType);
Expand Down
1 change: 0 additions & 1 deletion core/src/test/java/org/elasticsearch/action/IndicesRequestIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

package org.elasticsearch.action;

import org.apache.lucene.util.LuceneTestCase.AwaitsFix;
import org.elasticsearch.action.admin.indices.alias.Alias;
import org.elasticsearch.action.admin.indices.analyze.AnalyzeAction;
import org.elasticsearch.action.admin.indices.analyze.AnalyzeRequest;
Expand Down
Loading