From 5c136be13a945966adf70c36654e42300345278a Mon Sep 17 00:00:00 2001 From: Andriy Redko Date: Fri, 4 Mar 2022 16:19:05 -0500 Subject: [PATCH] Delayed CodecService instantiation up to the shard initialization Signed-off-by: Andriy Redko --- .../index/engine/EngineConfigFactory.java | 85 +++++++++++++++---- .../opensearch/index/shard/IndexShard.java | 3 +- .../opensearch/indices/IndicesService.java | 3 +- .../engine/EngineConfigFactoryTests.java | 10 +-- 4 files changed, 75 insertions(+), 26 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/engine/EngineConfigFactory.java b/server/src/main/java/org/opensearch/index/engine/EngineConfigFactory.java index 1eb70e21ea870..6d146d8223dec 100644 --- a/server/src/main/java/org/opensearch/index/engine/EngineConfigFactory.java +++ b/server/src/main/java/org/opensearch/index/engine/EngineConfigFactory.java @@ -42,30 +42,23 @@ * A factory to create an EngineConfig based on custom plugin overrides */ public class EngineConfigFactory { - private final CodecService codecService; + private final CodecServiceFactory codecServiceFactory; private final TranslogDeletionPolicyFactory translogDeletionPolicyFactory; /** default ctor primarily used for tests without plugins */ public EngineConfigFactory(IndexSettings idxSettings) { - this(Collections.emptyList(), idxSettings, null); + this(Collections.emptyList(), idxSettings); } /** * Construct a factory using the plugin service and provided index settings */ public EngineConfigFactory(PluginsService pluginsService, IndexSettings idxSettings) { - this(pluginsService.filterPlugins(EnginePlugin.class), idxSettings, null); - } - - /** - * Construct a factory using the plugin service, provided index settings and mapper service - */ - public EngineConfigFactory(PluginsService pluginsService, IndexSettings idxSettings, MapperService mapperService) { - this(pluginsService.filterPlugins(EnginePlugin.class), idxSettings, mapperService); + this(pluginsService.filterPlugins(EnginePlugin.class), idxSettings); } /* private constructor to construct the factory from specific EnginePlugins and IndexSettings */ - EngineConfigFactory(Collection enginePlugins, IndexSettings idxSettings, MapperService mapperService) { + EngineConfigFactory(Collection enginePlugins, IndexSettings idxSettings) { Optional codecService = Optional.empty(); String codecServiceOverridingPlugin = null; Optional codecServiceFactory = Optional.empty(); @@ -119,15 +112,16 @@ public EngineConfigFactory(PluginsService pluginsService, IndexSettings idxSetti ); } - this.codecService = codecService.isPresent() ? codecService.get() : codecServiceFactory.map(factory -> { - final CodecServiceConfig config = new CodecServiceConfig(idxSettings, mapperService); - return factory.createCodecService(config); - }).orElse(null); - + final CodecService instance = codecService.orElse(null); + this.codecServiceFactory = (instance != null) ? (config) -> instance : codecServiceFactory.orElse(null); this.translogDeletionPolicyFactory = translogDeletionPolicyFactory.orElse((idxs, rtls) -> null); } - /** Instantiates a new EngineConfig from the provided custom overrides */ + /** + * Instantiates a new EngineConfig from the provided custom overrides + * @deprecated please use overloaded {@code newEngineConfig} with {@link MapperService} + */ + @Deprecated public EngineConfig newEngineConfig( ShardId shardId, ThreadPool threadPool, @@ -152,6 +146,59 @@ public EngineConfig newEngineConfig( LongSupplier primaryTermSupplier, EngineConfig.TombstoneDocSupplier tombstoneDocSupplier ) { + return newEngineConfig( + shardId, + threadPool, + indexSettings, + warmer, + store, + mergePolicy, + analyzer, + similarity, + codecService, + eventListener, + queryCache, + queryCachingPolicy, + translogConfig, + flushMergesAfter, + externalRefreshListener, + internalRefreshListener, + indexSort, + circuitBreakerService, + globalCheckpointSupplier, + retentionLeasesSupplier, + primaryTermSupplier, + tombstoneDocSupplier, + null + ); + } + + /** Instantiates a new EngineConfig from the provided custom overrides */ + public EngineConfig newEngineConfig( + ShardId shardId, + ThreadPool threadPool, + IndexSettings indexSettings, + Engine.Warmer warmer, + Store store, + MergePolicy mergePolicy, + Analyzer analyzer, + Similarity similarity, + CodecService codecService, + Engine.EventListener eventListener, + QueryCache queryCache, + QueryCachingPolicy queryCachingPolicy, + TranslogConfig translogConfig, + TimeValue flushMergesAfter, + List externalRefreshListener, + List internalRefreshListener, + Sort indexSort, + CircuitBreakerService circuitBreakerService, + LongSupplier globalCheckpointSupplier, + Supplier retentionLeasesSupplier, + LongSupplier primaryTermSupplier, + EngineConfig.TombstoneDocSupplier tombstoneDocSupplier, + MapperService mapperService + ) { return new EngineConfig( shardId, @@ -162,7 +209,9 @@ public EngineConfig newEngineConfig( mergePolicy, analyzer, similarity, - this.codecService != null ? this.codecService : codecService, + this.codecServiceFactory != null + ? this.codecServiceFactory.createCodecService(new CodecServiceConfig(indexSettings, mapperService)) + : codecService, eventListener, queryCache, queryCachingPolicy, diff --git a/server/src/main/java/org/opensearch/index/shard/IndexShard.java b/server/src/main/java/org/opensearch/index/shard/IndexShard.java index b93fe71e3f70b..5792d6cf65c5a 100644 --- a/server/src/main/java/org/opensearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/opensearch/index/shard/IndexShard.java @@ -3177,7 +3177,8 @@ private EngineConfig newEngineConfig(LongSupplier globalCheckpointSupplier) { globalCheckpointSupplier, replicationTracker::getRetentionLeases, () -> getOperationPrimaryTerm(), - tombstoneDocSupplier() + tombstoneDocSupplier(), + mapperService ); } diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index 2b6fcf4d62b88..392bda21a0309 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -742,8 +742,7 @@ private synchronized IndexService createIndexService( } private EngineConfigFactory getEngineConfigFactory(final IndexSettings idxSettings) { - final IndexService indexService = indices.get(idxSettings.getUUID()); - return new EngineConfigFactory(this.pluginsService, idxSettings, (indexService != null) ? indexService.mapperService() : null); + return new EngineConfigFactory(this.pluginsService, idxSettings); } private EngineFactory getEngineFactory(final IndexSettings idxSettings) { diff --git a/server/src/test/java/org/opensearch/index/engine/EngineConfigFactoryTests.java b/server/src/test/java/org/opensearch/index/engine/EngineConfigFactoryTests.java index 1c0fca8df8e70..a6bc87d53c004 100644 --- a/server/src/test/java/org/opensearch/index/engine/EngineConfigFactoryTests.java +++ b/server/src/test/java/org/opensearch/index/engine/EngineConfigFactoryTests.java @@ -41,7 +41,7 @@ public void testCreateEngineConfigFromFactory() { .build(); List plugins = Collections.singletonList(new FooEnginePlugin()); IndexSettings indexSettings = IndexSettingsModule.newIndexSettings("test", meta.getSettings()); - EngineConfigFactory factory = new EngineConfigFactory(plugins, indexSettings, null); + EngineConfigFactory factory = new EngineConfigFactory(plugins, indexSettings); EngineConfig config = factory.newEngineConfig( null, @@ -82,7 +82,7 @@ public void testCreateEngineConfigFromFactoryMultipleCodecServiceIllegalStateExc List plugins = Arrays.asList(new FooEnginePlugin(), new BarEnginePlugin()); IndexSettings indexSettings = IndexSettingsModule.newIndexSettings("test", meta.getSettings()); - expectThrows(IllegalStateException.class, () -> new EngineConfigFactory(plugins, indexSettings, null)); + expectThrows(IllegalStateException.class, () -> new EngineConfigFactory(plugins, indexSettings)); } public void testCreateEngineConfigFromFactoryMultipleCodecServiceAndFactoryIllegalStateException() { @@ -94,7 +94,7 @@ public void testCreateEngineConfigFromFactoryMultipleCodecServiceAndFactoryIlleg List plugins = Arrays.asList(new FooEnginePlugin(), new BakEnginePlugin()); IndexSettings indexSettings = IndexSettingsModule.newIndexSettings("test", meta.getSettings()); - expectThrows(IllegalStateException.class, () -> new EngineConfigFactory(plugins, indexSettings, null)); + expectThrows(IllegalStateException.class, () -> new EngineConfigFactory(plugins, indexSettings)); } public void testCreateEngineConfigFromFactoryMultipleCustomTranslogDeletionPolicyFactoryIllegalStateException() { @@ -106,7 +106,7 @@ public void testCreateEngineConfigFromFactoryMultipleCustomTranslogDeletionPolic List plugins = Arrays.asList(new FooEnginePlugin(), new BazEnginePlugin()); IndexSettings indexSettings = IndexSettingsModule.newIndexSettings("test", meta.getSettings()); - expectThrows(IllegalStateException.class, () -> new EngineConfigFactory(plugins, indexSettings, null)); + expectThrows(IllegalStateException.class, () -> new EngineConfigFactory(plugins, indexSettings)); } public void testCreateCodecServiceFromFactory() { @@ -118,7 +118,7 @@ public void testCreateCodecServiceFromFactory() { List plugins = Arrays.asList(new BakEnginePlugin()); IndexSettings indexSettings = IndexSettingsModule.newIndexSettings("test", meta.getSettings()); - EngineConfigFactory factory = new EngineConfigFactory(plugins, indexSettings, null); + EngineConfigFactory factory = new EngineConfigFactory(plugins, indexSettings); EngineConfig config = factory.newEngineConfig( null, null,