Skip to content

Commit

Permalink
Move SlowLogFieldProvider instantiation to node construction
Browse files Browse the repository at this point in the history
SPI from plugins should be created at node startup. This commit moves
creation of SlowLogFieldProvider into node construction and passes it in
to IndicesService so that it is not recreated on each index creation.

relates elastic#102103
  • Loading branch information
rjernst committed Dec 3, 2024
1 parent 25fd4fd commit 90aca9a
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 28 deletions.
31 changes: 4 additions & 27 deletions server/src/main/java/org/elasticsearch/indices/IndicesService.java
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ public class IndicesService extends AbstractLifecycleComponent
private final MapperMetrics mapperMetrics;
private final PostRecoveryMerger postRecoveryMerger;
private final List<SearchOperationListener> searchOperationListeners;
final SlowLogFieldProvider slowLogFieldProvider; // pkg-private for testing

@Override
protected void doStart() {
Expand Down Expand Up @@ -379,6 +380,7 @@ public void onRemoval(ShardId shardId, String fieldName, boolean wasEvicted, lon
this.timestampFieldMapperService = new TimestampFieldMapperService(settings, threadPool, this);
this.postRecoveryMerger = new PostRecoveryMerger(settings, threadPool.executor(ThreadPool.Names.FORCE_MERGE), this::getShardOrNull);
this.searchOperationListeners = builder.searchOperationListener;
this.slowLogFieldProvider = builder.slowLogFieldProvider;
}

private static final String DANGLING_INDICES_UPDATE_THREAD_NAME = "DanglingIndices#updateTask";
Expand Down Expand Up @@ -749,7 +751,7 @@ private synchronized IndexService createIndexService(
() -> allowExpensiveQueries,
indexNameExpressionResolver,
recoveryStateFactories,
loadSlowLogFieldProvider(),
slowLogFieldProvider,
mapperMetrics,
searchOperationListeners
);
Expand Down Expand Up @@ -828,7 +830,7 @@ public synchronized MapperService createIndexMapperServiceForValidation(IndexMet
() -> allowExpensiveQueries,
indexNameExpressionResolver,
recoveryStateFactories,
loadSlowLogFieldProvider(),
slowLogFieldProvider,
mapperMetrics,
searchOperationListeners
);
Expand Down Expand Up @@ -1434,31 +1436,6 @@ int numPendingDeletes(Index index) {
}
}

// pkg-private for testing
SlowLogFieldProvider loadSlowLogFieldProvider() {
List<? extends SlowLogFieldProvider> slowLogFieldProviders = pluginsService.loadServiceProviders(SlowLogFieldProvider.class);
return new SlowLogFieldProvider() {
@Override
public void init(IndexSettings indexSettings) {
slowLogFieldProviders.forEach(provider -> provider.init(indexSettings));
}

@Override
public Map<String, String> indexSlowLogFields() {
return slowLogFieldProviders.stream()
.flatMap(provider -> provider.indexSlowLogFields().entrySet().stream())
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}

@Override
public Map<String, String> searchSlowLogFields() {
return slowLogFieldProviders.stream()
.flatMap(provider -> provider.searchSlowLogFields().entrySet().stream())
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}
};
}

/**
* Checks if all pending deletes have completed. Used by tests to ensure we don't check directory contents
* while deletion still ongoing. * The reason is that, on Windows, browsing the directory contents can interfere
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.elasticsearch.features.FeatureService;
import org.elasticsearch.gateway.MetaStateService;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.SlowLogFieldProvider;
import org.elasticsearch.index.analysis.AnalysisRegistry;
import org.elasticsearch.index.engine.EngineFactory;
import org.elasticsearch.index.mapper.MapperMetrics;
Expand Down Expand Up @@ -76,6 +77,20 @@ public class IndicesServiceBuilder {
CheckedBiConsumer<ShardSearchRequest, StreamOutput, IOException> requestCacheKeyDifferentiator;
MapperMetrics mapperMetrics;
List<SearchOperationListener> searchOperationListener = List.of();
SlowLogFieldProvider slowLogFieldProvider = new SlowLogFieldProvider() {
@Override
public void init(IndexSettings indexSettings) {}

@Override
public Map<String, String> indexSlowLogFields() {
return Map.of();
}

@Override
public Map<String, String> searchSlowLogFields() {
return Map.of();
}
};

public IndicesServiceBuilder settings(Settings settings) {
this.settings = settings;
Expand Down Expand Up @@ -188,6 +203,11 @@ public IndicesServiceBuilder searchOperationListeners(List<SearchOperationListen
return this;
}

public IndicesServiceBuilder slowLogFieldProvider(SlowLogFieldProvider slowLogFieldProvider) {
this.slowLogFieldProvider = slowLogFieldProvider;
return this;
}

public IndicesService build() {
Objects.requireNonNull(settings);
Objects.requireNonNull(pluginsService);
Expand All @@ -213,6 +233,7 @@ public IndicesService build() {
Objects.requireNonNull(snapshotCommitSuppliers);
Objects.requireNonNull(mapperMetrics);
Objects.requireNonNull(searchOperationListener);
Objects.requireNonNull(slowLogFieldProvider);

// collect engine factory providers from plugins
engineFactoryProviders = pluginsService.filterPlugins(EnginePlugin.class)
Expand Down
27 changes: 27 additions & 0 deletions server/src/main/java/org/elasticsearch/node/NodeConstruction.java
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,9 @@
import org.elasticsearch.index.IndexMode;
import org.elasticsearch.index.IndexSettingProvider;
import org.elasticsearch.index.IndexSettingProviders;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.IndexingPressure;
import org.elasticsearch.index.SlowLogFieldProvider;
import org.elasticsearch.index.analysis.AnalysisRegistry;
import org.elasticsearch.index.mapper.MapperMetrics;
import org.elasticsearch.index.mapper.SourceFieldMetrics;
Expand Down Expand Up @@ -800,6 +802,30 @@ private void construct(
new ShardSearchPhaseAPMMetrics(telemetryProvider.getMeterRegistry())
);

List<? extends SlowLogFieldProvider> slowLogFieldProviders = pluginsService.loadServiceProviders(SlowLogFieldProvider.class);
// NOTE: the response of index/search slow log fields below must be calculated dynamically on every call
// because the responses may change dynamically at runtime
SlowLogFieldProvider slowLogFieldProvider = new SlowLogFieldProvider() {
@Override
public void init(IndexSettings indexSettings) {
slowLogFieldProviders.forEach(provider -> provider.init(indexSettings));
}

@Override
public Map<String, String> indexSlowLogFields() {
return slowLogFieldProviders.stream()
.flatMap(provider -> provider.indexSlowLogFields().entrySet().stream())
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}

@Override
public Map<String, String> searchSlowLogFields() {
return slowLogFieldProviders.stream()
.flatMap(provider -> provider.searchSlowLogFields().entrySet().stream())
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}
};

IndicesService indicesService = new IndicesServiceBuilder().settings(settings)
.pluginsService(pluginsService)
.nodeEnvironment(nodeEnvironment)
Expand All @@ -821,6 +847,7 @@ private void construct(
.requestCacheKeyDifferentiator(searchModule.getRequestCacheKeyDifferentiator())
.mapperMetrics(mapperMetrics)
.searchOperationListeners(searchOperationListeners)
.slowLogFieldProvider(slowLogFieldProvider)
.build();

final var parameters = new IndexSettingProvider.Parameters(clusterService, indicesService::createIndexMapperServiceForValidation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,7 @@ public void testLoadSlowLogFieldProvider() {
TestAnotherSlowLogFieldProvider.setFields(Map.of("key2", "value2"));

var indicesService = getIndicesService();
SlowLogFieldProvider fieldProvider = indicesService.loadSlowLogFieldProvider();
SlowLogFieldProvider fieldProvider = indicesService.slowLogFieldProvider;

// The map of fields from the two providers are merged to a single map of fields
assertEquals(Map.of("key1", "value1", "key2", "value2"), fieldProvider.searchSlowLogFields());
Expand Down

0 comments on commit 90aca9a

Please sign in to comment.