Skip to content
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.

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

Move SlowLogFieldProvider instantiation to node construction #117949

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions docs/changelog/117949.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 117949
summary: Move `SlowLogFieldProvider` instantiation to node construction
area: Infra/Logging
type: bug
issues: []
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
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