From bd7a46f9ae4b094c3516888dbe70df1243298e8d Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Mon, 18 Jun 2018 14:40:02 -0600 Subject: [PATCH 1/2] Lazily retrieve EngineFactory based on IndexSettings This commit changes the way that `EngineFactory`s are returned, instead of the EngineFactory being set at time of shard creation, this changes the pluggability so that an existing `IndexShard` can return a different engine factory depending on dynamic index settings. Relates to #31141 --- .../org/elasticsearch/index/IndexModule.java | 15 ++++---- .../org/elasticsearch/index/IndexService.java | 13 ++++--- .../elasticsearch/index/shard/IndexShard.java | 13 ++++--- .../elasticsearch/indices/IndicesService.java | 7 ++-- .../elasticsearch/index/IndexModuleTests.java | 38 ++++++++++++------- .../IndexLevelReplicationTests.java | 5 ++- .../RecoveryDuringReplicationTests.java | 29 ++++++++------ .../index/shard/IndexShardIT.java | 2 +- .../index/shard/IndexShardTests.java | 12 +++--- .../indices/IndicesServiceTests.java | 20 ++++++++-- .../BlobStoreRepositoryRestoreTests.java | 2 +- .../ESIndexLevelReplicationTestCase.java | 5 ++- .../index/shard/IndexShardTestCase.java | 15 ++++---- 13 files changed, 105 insertions(+), 71 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/IndexModule.java b/server/src/main/java/org/elasticsearch/index/IndexModule.java index 9e859a16956c8..7cbfcdaf118a6 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexModule.java +++ b/server/src/main/java/org/elasticsearch/index/IndexModule.java @@ -105,7 +105,7 @@ public final class IndexModule { private final IndexSettings indexSettings; private final AnalysisRegistry analysisRegistry; - private final EngineFactory engineFactory; + private final Function engineFactoryFn; private SetOnce indexSearcherWrapper = new SetOnce<>(); private final Set indexEventListeners = new HashSet<>(); private final Map> similarities = new HashMap<>(); @@ -121,12 +121,13 @@ public final class IndexModule { * * @param indexSettings the index settings * @param analysisRegistry the analysis registry - * @param engineFactory the engine factory + * @param engineFactoryFn a function from IndexSettings to the engine factory to use */ - public IndexModule(final IndexSettings indexSettings, final AnalysisRegistry analysisRegistry, final EngineFactory engineFactory) { + public IndexModule(final IndexSettings indexSettings, final AnalysisRegistry analysisRegistry, + final Function engineFactoryFn) { this.indexSettings = indexSettings; this.analysisRegistry = analysisRegistry; - this.engineFactory = Objects.requireNonNull(engineFactory); + this.engineFactoryFn = Objects.requireNonNull(engineFactoryFn); this.searchOperationListeners.add(new SearchSlowLog(indexSettings)); this.indexOperationListeners.add(new IndexingSlowLog(indexSettings)); } @@ -172,8 +173,8 @@ public Index getIndex() { * * @return the engine factory */ - EngineFactory getEngineFactory() { - return engineFactory; + Function getEngineFactoryFn() { + return engineFactoryFn; } /** @@ -382,7 +383,7 @@ public IndexService newIndexService( } return new IndexService(indexSettings, environment, xContentRegistry, new SimilarityService(indexSettings, scriptService, similarities), - shardStoreDeleter, analysisRegistry, engineFactory, circuitBreakerService, bigArrays, threadPool, scriptService, + shardStoreDeleter, analysisRegistry, engineFactoryFn, circuitBreakerService, bigArrays, threadPool, scriptService, client, queryCache, store, eventListener, searcherWrapperFactory, mapperRegistry, indicesFieldDataCache, searchOperationListeners, indexOperationListeners, namedWriteableRegistry); } diff --git a/server/src/main/java/org/elasticsearch/index/IndexService.java b/server/src/main/java/org/elasticsearch/index/IndexService.java index 5e9e811bc32ec..fe2d1c5828124 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexService.java +++ b/server/src/main/java/org/elasticsearch/index/IndexService.java @@ -88,6 +88,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; +import java.util.function.Function; import java.util.function.LongSupplier; import java.util.function.Supplier; @@ -109,7 +110,7 @@ public class IndexService extends AbstractIndexComponent implements IndicesClust private final NamedXContentRegistry xContentRegistry; private final NamedWriteableRegistry namedWriteableRegistry; private final SimilarityService similarityService; - private final EngineFactory engineFactory; + private final Function engineFactoryFn; private final IndexWarmer warmer; private volatile Map shards = emptyMap(); private final AtomicBoolean closed = new AtomicBoolean(false); @@ -139,7 +140,7 @@ public IndexService( SimilarityService similarityService, ShardStoreDeleter shardStoreDeleter, AnalysisRegistry registry, - EngineFactory engineFactory, + Function engineFactoryFn, CircuitBreakerService circuitBreakerService, BigArrays bigArrays, ThreadPool threadPool, @@ -188,7 +189,7 @@ public IndexService( this.warmer = new IndexWarmer(indexSettings.getSettings(), threadPool, indexFieldData, bitsetFilterCache.createListener(threadPool)); this.indexCache = new IndexCache(indexSettings, queryCache, bitsetFilterCache); - this.engineFactory = Objects.requireNonNull(engineFactory); + this.engineFactoryFn = Objects.requireNonNull(engineFactoryFn); // initialize this last -- otherwise if the wrapper requires any other member to be non-null we fail with an NPE this.searcherWrapper = wrapperFactory.newWrapper(this); this.searchOperationListeners = Collections.unmodifiableList(searchOperationListeners); @@ -380,7 +381,7 @@ public synchronized IndexShard createShard(ShardRouting routing, Consumer eventListener.onStoreClosed(shardId))); indexShard = new IndexShard(routing, this.indexSettings, path, store, indexSortSupplier, - indexCache, mapperService, similarityService, engineFactory, + indexCache, mapperService, similarityService, engineFactoryFn, eventListener, searcherWrapper, threadPool, bigArrays, engineWarmer, searchOperationListeners, indexingOperationListeners, () -> globalCheckpointSyncer.accept(shardId), circuitBreakerService); @@ -681,8 +682,8 @@ public interface ShardStoreDeleter { void addPendingDelete(ShardId shardId, IndexSettings indexSettings); } - public final EngineFactory getEngineFactory() { - return engineFactory; + public final Function getEngineFactoryFn() { + return engineFactoryFn; } final IndexSearcherWrapper getSearcherWrapper() { diff --git a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java index 34230be14cb7e..ef80e9bdbc58a 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java @@ -154,6 +154,7 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiConsumer; import java.util.function.Consumer; +import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.StreamSupport; @@ -193,7 +194,7 @@ public class IndexShard extends AbstractIndexShardComponent implements IndicesCl protected volatile IndexShardState state; protected volatile long primaryTerm; protected final AtomicReference currentEngineReference = new AtomicReference<>(); - final EngineFactory engineFactory; + final Function engineFactoryFn; private final IndexingOperationListener indexingOperationListeners; private final Runnable globalCheckpointSyncer; @@ -248,7 +249,7 @@ public IndexShard( IndexCache indexCache, MapperService mapperService, SimilarityService similarityService, - @Nullable EngineFactory engineFactory, + Function engineFactoryFn, IndexEventListener indexEventListener, IndexSearcherWrapper indexSearcherWrapper, ThreadPool threadPool, @@ -266,7 +267,7 @@ public IndexShard( this.warmer = warmer; this.similarityService = similarityService; Objects.requireNonNull(store, "Store must be provided to the index shard"); - this.engineFactory = Objects.requireNonNull(engineFactory); + this.engineFactoryFn = Objects.requireNonNull(engineFactoryFn); this.store = store; this.indexSortSupplier = indexSortSupplier; this.indexEventListener = indexEventListener; @@ -2115,7 +2116,7 @@ private Engine createNewEngine(EngineConfig config) { } protected Engine newEngine(EngineConfig config) { - return engineFactory.newReadWriteEngine(config); + return engineFactoryFn.apply(this.indexSettings).newReadWriteEngine(config); } private static void persistMetadata( @@ -2445,8 +2446,8 @@ public ShardFailure(ShardRouting routing, String reason, @Nullable Exception cau } } - EngineFactory getEngineFactory() { - return engineFactory; + Function getEngineFactoryFn() { + return engineFactoryFn; } // for tests diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesService.java b/server/src/main/java/org/elasticsearch/indices/IndicesService.java index 5141ca5a0c178..657a58725c3a5 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -451,7 +451,7 @@ private synchronized IndexService createIndexService(final String reason, idxSettings.getNumberOfReplicas(), reason); - final IndexModule indexModule = new IndexModule(idxSettings, analysisRegistry, getEngineFactory(idxSettings)); + final IndexModule indexModule = new IndexModule(idxSettings, analysisRegistry, this::getEngineFactory); for (IndexingOperationListener operationListener : indexingOperationListeners) { indexModule.addIndexOperationListener(operationListener); } @@ -475,7 +475,8 @@ private synchronized IndexService createIndexService(final String reason, ); } - private EngineFactory getEngineFactory(final IndexSettings idxSettings) { + // Visible for testing + EngineFactory getEngineFactory(final IndexSettings idxSettings) { final List> engineFactories = engineFactoryProviders .stream() @@ -511,7 +512,7 @@ private EngineFactory getEngineFactory(final IndexSettings idxSettings) { */ public synchronized MapperService createIndexMapperService(IndexMetaData indexMetaData) throws IOException { final IndexSettings idxSettings = new IndexSettings(indexMetaData, this.settings, indexScopedSettings); - final IndexModule indexModule = new IndexModule(idxSettings, analysisRegistry, getEngineFactory(idxSettings)); + final IndexModule indexModule = new IndexModule(idxSettings, analysisRegistry, this::getEngineFactory); pluginsService.onIndexModule(indexModule); return indexModule.newIndexMapperService(xContentRegistry, mapperRegistry, scriptService); } diff --git a/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java b/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java index 1d531bdeb902f..e8c68ea8e8e95 100644 --- a/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java +++ b/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java @@ -147,12 +147,13 @@ private IndexService newIndexService(IndexModule module) throws IOException { } public void testWrapperIsBound() throws IOException { - IndexModule module = new IndexModule(indexSettings, emptyAnalysisRegistry, new MockEngineFactory(AssertingDirectoryReader.class)); + IndexModule module = new IndexModule(indexSettings, emptyAnalysisRegistry, + s -> new MockEngineFactory(AssertingDirectoryReader.class)); module.setSearcherWrapper((s) -> new Wrapper()); IndexService indexService = newIndexService(module); assertTrue(indexService.getSearcherWrapper() instanceof Wrapper); - assertSame(indexService.getEngineFactory(), module.getEngineFactory()); + assertSame(indexService.getEngineFactoryFn(), module.getEngineFactoryFn()); indexService.close("simon says", false); } @@ -165,7 +166,7 @@ public void testRegisterIndexStore() throws IOException { .put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), "foo_store") .build(); IndexSettings indexSettings = IndexSettingsModule.newIndexSettings(index, settings); - IndexModule module = new IndexModule(indexSettings, emptyAnalysisRegistry, new InternalEngineFactory()); + IndexModule module = new IndexModule(indexSettings, emptyAnalysisRegistry, s -> new InternalEngineFactory()); module.addIndexStore("foo_store", FooStore::new); try { module.addIndexStore("foo_store", FooStore::new); @@ -189,7 +190,7 @@ public void beforeIndexRemoved(IndexService indexService, IndexRemovalReason rea } }; IndexSettings indexSettings = IndexSettingsModule.newIndexSettings(index, settings); - IndexModule module = new IndexModule(indexSettings, emptyAnalysisRegistry, new InternalEngineFactory()); + IndexModule module = new IndexModule(indexSettings, emptyAnalysisRegistry, s -> new InternalEngineFactory()); module.addIndexEventListener(eventListener); IndexService indexService = newIndexService(module); IndexSettings x = indexService.getIndexSettings(); @@ -204,7 +205,7 @@ public void beforeIndexRemoved(IndexService indexService, IndexRemovalReason rea public void testListener() throws IOException { Setting booleanSetting = Setting.boolSetting("index.foo.bar", false, Property.Dynamic, Property.IndexScope); final IndexSettings indexSettings = IndexSettingsModule.newIndexSettings(index, settings, booleanSetting); - IndexModule module = new IndexModule(indexSettings, emptyAnalysisRegistry, new InternalEngineFactory()); + IndexModule module = new IndexModule(indexSettings, emptyAnalysisRegistry, s -> new InternalEngineFactory()); Setting booleanSetting2 = Setting.boolSetting("index.foo.bar.baz", false, Property.Dynamic, Property.IndexScope); AtomicBoolean atomicBoolean = new AtomicBoolean(false); module.addSettingsUpdateConsumer(booleanSetting, atomicBoolean::set); @@ -224,7 +225,8 @@ public void testListener() throws IOException { public void testAddIndexOperationListener() throws IOException { IndexModule module = - new IndexModule(IndexSettingsModule.newIndexSettings(index, settings), emptyAnalysisRegistry, new InternalEngineFactory()); + new IndexModule(IndexSettingsModule.newIndexSettings(index, settings), + emptyAnalysisRegistry, s -> new InternalEngineFactory()); AtomicBoolean executed = new AtomicBoolean(false); IndexingOperationListener listener = new IndexingOperationListener() { @Override @@ -255,7 +257,8 @@ public Engine.Index preIndex(ShardId shardId, Engine.Index operation) { public void testAddSearchOperationListener() throws IOException { IndexModule module = - new IndexModule(IndexSettingsModule.newIndexSettings(index, settings), emptyAnalysisRegistry, new InternalEngineFactory()); + new IndexModule(IndexSettingsModule.newIndexSettings(index, settings), + emptyAnalysisRegistry, s -> new InternalEngineFactory()); AtomicBoolean executed = new AtomicBoolean(false); SearchOperationListener listener = new SearchOperationListener() { @@ -289,7 +292,8 @@ public void testAddSimilarity() throws IOException { .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()) .build(); IndexModule module = - new IndexModule(IndexSettingsModule.newIndexSettings("foo", settings), emptyAnalysisRegistry, new InternalEngineFactory()); + new IndexModule(IndexSettingsModule.newIndexSettings("foo", settings), + emptyAnalysisRegistry, s -> new InternalEngineFactory()); module.addSimilarity("test_similarity", (providerSettings, indexCreatedVersion, scriptService) -> new TestSimilarity(providerSettings.get("key"))); @@ -304,7 +308,8 @@ public void testAddSimilarity() throws IOException { public void testFrozen() { IndexModule module = - new IndexModule(IndexSettingsModule.newIndexSettings(index, settings), emptyAnalysisRegistry, new InternalEngineFactory()); + new IndexModule(IndexSettingsModule.newIndexSettings(index, settings), + emptyAnalysisRegistry, s -> new InternalEngineFactory()); module.freeze(); String msg = "Can't modify IndexModule once the index service has been created"; assertEquals(msg, expectThrows(IllegalStateException.class, () -> module.addSearchOperationListener(null)).getMessage()); @@ -323,7 +328,8 @@ public void testSetupUnknownSimilarity() throws IOException { .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()) .build(); IndexModule module = - new IndexModule(IndexSettingsModule.newIndexSettings("foo", settings), emptyAnalysisRegistry, new InternalEngineFactory()); + new IndexModule(IndexSettingsModule.newIndexSettings("foo", settings), + emptyAnalysisRegistry, s -> new InternalEngineFactory()); Exception ex = expectThrows(IllegalArgumentException.class, () -> newIndexService(module)); assertEquals("Unknown Similarity type [test_similarity] for [my_similarity]", ex.getMessage()); } @@ -335,7 +341,8 @@ public void testSetupWithoutType() throws IOException { .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT) .build(); IndexModule module = - new IndexModule(IndexSettingsModule.newIndexSettings("foo", settings), emptyAnalysisRegistry, new InternalEngineFactory()); + new IndexModule(IndexSettingsModule.newIndexSettings("foo", settings), + emptyAnalysisRegistry, s -> new InternalEngineFactory()); Exception ex = expectThrows(IllegalArgumentException.class, () -> newIndexService(module)); assertEquals("Similarity [my_similarity] must have an associated type", ex.getMessage()); } @@ -345,7 +352,8 @@ public void testForceCustomQueryCache() throws IOException { .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()) .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).build(); IndexModule module = - new IndexModule(IndexSettingsModule.newIndexSettings("foo", settings), emptyAnalysisRegistry, new InternalEngineFactory()); + new IndexModule(IndexSettingsModule.newIndexSettings("foo", settings), + emptyAnalysisRegistry, s -> new InternalEngineFactory()); module.forceQueryCacheProvider((a, b) -> new CustomQueryCache()); expectThrows(AlreadySetException.class, () -> module.forceQueryCacheProvider((a, b) -> new CustomQueryCache())); IndexService indexService = newIndexService(module); @@ -358,7 +366,8 @@ public void testDefaultQueryCacheImplIsSelected() throws IOException { .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()) .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).build(); IndexModule module = - new IndexModule(IndexSettingsModule.newIndexSettings("foo", settings), emptyAnalysisRegistry, new InternalEngineFactory()); + new IndexModule(IndexSettingsModule.newIndexSettings("foo", settings), + emptyAnalysisRegistry, s -> new InternalEngineFactory()); IndexService indexService = newIndexService(module); assertTrue(indexService.cache().query() instanceof IndexQueryCache); indexService.close("simon says", false); @@ -370,7 +379,8 @@ public void testDisableQueryCacheHasPrecedenceOverForceQueryCache() throws IOExc .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()) .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).build(); IndexModule module = - new IndexModule(IndexSettingsModule.newIndexSettings("foo", settings), emptyAnalysisRegistry, new InternalEngineFactory()); + new IndexModule(IndexSettingsModule.newIndexSettings("foo", settings), + emptyAnalysisRegistry, s -> new InternalEngineFactory()); module.forceQueryCacheProvider((a, b) -> new CustomQueryCache()); IndexService indexService = newIndexService(module); assertTrue(indexService.cache().query() instanceof DisabledQueryCache); diff --git a/server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java b/server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java index 018548be9629f..b23022f30e17c 100644 --- a/server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java +++ b/server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java @@ -61,6 +61,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.function.Function; import static org.elasticsearch.index.translog.SnapshotMatchers.containsOperationsInAnyOrder; import static org.hamcrest.Matchers.anyOf; @@ -244,8 +245,8 @@ public void testDocumentFailureReplication() throws Exception { new ThrowingDocumentFailureEngineFactory(failureMessage); try (ReplicationGroup shards = new ReplicationGroup(buildIndexMetaData(0)) { @Override - protected EngineFactory getEngineFactory(ShardRouting routing) { - return throwingDocumentFailureEngineFactory; + protected Function getEngineFactory(ShardRouting routing) { + return s -> throwingDocumentFailureEngineFactory; }}) { // test only primary diff --git a/server/src/test/java/org/elasticsearch/index/replication/RecoveryDuringReplicationTests.java b/server/src/test/java/org/elasticsearch/index/replication/RecoveryDuringReplicationTests.java index ee97ba14fe09e..3ea4f145d9331 100644 --- a/server/src/test/java/org/elasticsearch/index/replication/RecoveryDuringReplicationTests.java +++ b/server/src/test/java/org/elasticsearch/index/replication/RecoveryDuringReplicationTests.java @@ -63,6 +63,7 @@ import java.util.concurrent.Future; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.both; @@ -460,12 +461,14 @@ public void testWaitForPendingSeqNo() throws Exception { try (ReplicationGroup shards = new ReplicationGroup(metaData) { @Override - protected EngineFactory getEngineFactory(ShardRouting routing) { - if (routing.primary()) { - return primaryEngineFactory; - } else { - return new InternalEngineFactory(); - } + protected Function getEngineFactory(ShardRouting routing) { + return indexSettings -> { + if (routing.primary()) { + return primaryEngineFactory; + } else { + return new InternalEngineFactory(); + } + }; } }) { shards.startAll(); @@ -556,12 +559,14 @@ public void testCheckpointsAndMarkingInSync() throws Exception { try ( ReplicationGroup shards = new ReplicationGroup(metaData) { @Override - protected EngineFactory getEngineFactory(final ShardRouting routing) { - if (routing.primary()) { - return new InternalEngineFactory(); - } else { - return replicaEngineFactory; - } + protected Function getEngineFactory(final ShardRouting routing) { + return indexSettings -> { + if (routing.primary()) { + return new InternalEngineFactory(); + } else { + return replicaEngineFactory; + } + }; } }; AutoCloseable ignored = replicaEngineFactory // make sure we release indexers before closing diff --git a/server/src/test/java/org/elasticsearch/index/shard/IndexShardIT.java b/server/src/test/java/org/elasticsearch/index/shard/IndexShardIT.java index d6d50b24d1f68..b3d2f52506edf 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/IndexShardIT.java +++ b/server/src/test/java/org/elasticsearch/index/shard/IndexShardIT.java @@ -620,7 +620,7 @@ public static final IndexShard newIndexShard(IndexService indexService, IndexSha ShardRouting initializingShardRouting = getInitializingShardRouting(shard.routingEntry()); IndexShard newShard = new IndexShard(initializingShardRouting, indexService.getIndexSettings(), shard.shardPath(), shard.store(), indexService.getIndexSortSupplier(), indexService.cache(), indexService.mapperService(), indexService.similarityService(), - shard.getEngineFactory(), indexService.getIndexEventListener(), wrapper, + shard.getEngineFactoryFn(), indexService.getIndexEventListener(), wrapper, indexService.getThreadPool(), indexService.getBigArrays(), null, Collections.emptyList(), Arrays.asList(listeners), () -> {}, cbs); return newShard; } diff --git a/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java b/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java index 31afb5ed42fc0..780a26ce7a988 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java @@ -841,7 +841,7 @@ public void testGlobalCheckpointSync() throws IOException { final IndexMetaData.Builder indexMetadata = IndexMetaData.builder(shardRouting.getIndexName()).settings(settings).primaryTerm(0, 1); final AtomicBoolean synced = new AtomicBoolean(); final IndexShard primaryShard = - newShard(shardRouting, indexMetadata.build(), null, new InternalEngineFactory(), () -> synced.set(true)); + newShard(shardRouting, indexMetadata.build(), null, s -> new InternalEngineFactory(), () -> synced.set(true)); // add a replica recoverShardFromStore(primaryShard); final IndexShard replicaShard = newShard(shardId, false); @@ -1892,7 +1892,7 @@ public IndexSearcher wrap(IndexSearcher searcher) throws EngineException { shard.shardPath(), shard.indexSettings().getIndexMetaData(), wrapper, - new InternalEngineFactory(), + s -> new InternalEngineFactory(), () -> {}, EMPTY_EVENT_LISTENER); @@ -2044,7 +2044,7 @@ public IndexSearcher wrap(IndexSearcher searcher) throws EngineException { shard.shardPath(), shard.indexSettings().getIndexMetaData(), wrapper, - new InternalEngineFactory(), + s -> new InternalEngineFactory(), () -> {}, EMPTY_EVENT_LISTENER); @@ -2529,7 +2529,7 @@ public void testReadSnapshotAndCheckIndexConcurrently() throws Exception { .put(IndexSettings.INDEX_CHECK_ON_STARTUP.getKey(), randomFrom("false", "true", "checksum", "fix"))) .build(); final IndexShard newShard = newShard(shardRouting, indexShard.shardPath(), indexMetaData, - null, indexShard.engineFactory, indexShard.getGlobalCheckpointSyncer(), EMPTY_EVENT_LISTENER); + null, indexShard.engineFactoryFn, indexShard.getGlobalCheckpointSyncer(), EMPTY_EVENT_LISTENER); Store.MetadataSnapshot storeFileMetaDatas = newShard.snapshotStoreMetadata(); assertTrue("at least 2 files, commit and data: " + storeFileMetaDatas.toString(), storeFileMetaDatas.size() > 1); @@ -3029,8 +3029,8 @@ public void testFlushOnInactive() throws Exception { ShardPath shardPath = new ShardPath(false, nodePath.resolve(shardId), nodePath.resolve(shardId), shardId); AtomicBoolean markedInactive = new AtomicBoolean(); AtomicReference primaryRef = new AtomicReference<>(); - IndexShard primary = newShard(shardRouting, shardPath, metaData, null, new InternalEngineFactory(), () -> { - }, new IndexEventListener() { + IndexShard primary = newShard(shardRouting, shardPath, metaData, null, + s -> new InternalEngineFactory(), () -> { }, new IndexEventListener() { @Override public void onShardInactive(IndexShard indexShard) { markedInactive.set(true); diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java index 35416c617fdd0..30157623069a6 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java @@ -533,9 +533,11 @@ public void testGetEngineFactory() throws IOException { .build(); final IndexService indexService = indicesService.createIndex(indexMetaData, Collections.emptyList()); if (value != null && value) { - assertThat(indexService.getEngineFactory(), instanceOf(FooEnginePlugin.FooEngineFactory.class)); + assertThat(indexService.getEngineFactoryFn().apply(new IndexSettings(indexMetaData, builder.build())), + instanceOf(FooEnginePlugin.FooEngineFactory.class)); } else { - assertThat(indexService.getEngineFactory(), instanceOf(InternalEngineFactory.class)); + assertThat(indexService.getEngineFactoryFn().apply(new IndexSettings(indexMetaData, builder.build())), + instanceOf(InternalEngineFactory.class)); } } } @@ -556,11 +558,21 @@ public void testConflictingEngineFactories() throws IOException { .build(); final IndicesService indicesService = getIndicesService(); - final IllegalStateException e = - expectThrows(IllegalStateException.class, () -> indicesService.createIndex(indexMetaData, Collections.emptyList())); + final IndexSettings indexSettings = new IndexSettings(indexMetaData, settings); + final IllegalStateException e = expectThrows(IllegalStateException.class, () -> indicesService.getEngineFactory(indexSettings)); final String pattern = ".*multiple engine factories provided for \\[foobar/.*\\]: \\[.*FooEngineFactory\\],\\[.*BarEngineFactory\\].*"; assertThat(e, hasToString(new RegexMatcher(pattern))); + + final Settings newSettings = Settings.builder().put(settings).put(BarEnginePlugin.BAR_INDEX_SETTING.getKey(), false).build(); + IndexMetaData newIndexMetaData = new IndexMetaData.Builder(index.getName()) + .settings(newSettings) + .numberOfShards(1) + .numberOfReplicas(0) + .build(); + final IndexSettings newIndexSettings = new IndexSettings(newIndexMetaData, newSettings); + final IndexService indexService = indicesService.createIndex(newIndexMetaData, Collections.emptyList()); + assertThat(indexService.getEngineFactoryFn().apply(newIndexSettings), instanceOf(FooEnginePlugin.FooEngineFactory.class)); } } diff --git a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryRestoreTests.java b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryRestoreTests.java index 7a1d3a894204f..10f8a8fda10da 100644 --- a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryRestoreTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryRestoreTests.java @@ -105,7 +105,7 @@ public void testRestoreSnapshotWithExistingFiles() throws IOException { shard.shardPath(), shard.indexSettings().getIndexMetaData(), null, - new InternalEngineFactory(), + s -> new InternalEngineFactory(), () -> {}, EMPTY_EVENT_LISTENER); diff --git a/test/framework/src/main/java/org/elasticsearch/index/replication/ESIndexLevelReplicationTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/replication/ESIndexLevelReplicationTestCase.java index ab18e359458bd..a31641ac1fe23 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/replication/ESIndexLevelReplicationTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/replication/ESIndexLevelReplicationTestCase.java @@ -59,6 +59,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.Index; +import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.engine.EngineFactory; import org.elasticsearch.index.engine.InternalEngineFactory; import org.elasticsearch.index.seqno.GlobalCheckpointSyncAction; @@ -160,8 +161,8 @@ private ShardRouting createShardRouting(String nodeId, boolean primary) { primary ? RecoverySource.StoreRecoverySource.EMPTY_STORE_INSTANCE : RecoverySource.PeerRecoverySource.INSTANCE); } - protected EngineFactory getEngineFactory(ShardRouting routing) { - return new InternalEngineFactory(); + protected Function getEngineFactory(ShardRouting routing) { + return indexSettings -> new InternalEngineFactory(); } public int indexDocs(final int numOfDoc) throws Exception { diff --git a/test/framework/src/main/java/org/elasticsearch/index/shard/IndexShardTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/shard/IndexShardTestCase.java index 9b21af713701a..73f5f044c17b1 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/shard/IndexShardTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/shard/IndexShardTestCase.java @@ -94,6 +94,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; import java.util.function.BiFunction; +import java.util.function.Function; import static org.elasticsearch.cluster.routing.TestShardRouting.newShardRouting; import static org.hamcrest.Matchers.contains; @@ -228,7 +229,7 @@ protected IndexShard newShard(ShardId shardId, boolean primary, String nodeId, I @Nullable IndexSearcherWrapper searcherWrapper, Runnable globalCheckpointSyncer) throws IOException { ShardRouting shardRouting = TestShardRouting.newShardRouting(shardId, nodeId, primary, ShardRoutingState.INITIALIZING, primary ? RecoverySource.StoreRecoverySource.EMPTY_STORE_INSTANCE : RecoverySource.PeerRecoverySource.INSTANCE); - return newShard(shardRouting, indexMetaData, searcherWrapper, new InternalEngineFactory(), globalCheckpointSyncer); + return newShard(shardRouting, indexMetaData, searcherWrapper, s -> new InternalEngineFactory(), globalCheckpointSyncer); } @@ -242,7 +243,7 @@ protected IndexShard newShard(ShardId shardId, boolean primary, String nodeId, I */ protected IndexShard newShard(ShardRouting routing, IndexMetaData indexMetaData, IndexingOperationListener... listeners) throws IOException { - return newShard(routing, indexMetaData, null, new InternalEngineFactory(), () -> {}, listeners); + return newShard(routing, indexMetaData, null, s -> new InternalEngineFactory(), () -> {}, listeners); } /** @@ -256,7 +257,7 @@ protected IndexShard newShard(ShardRouting routing, IndexMetaData indexMetaData, */ protected IndexShard newShard(ShardRouting routing, IndexMetaData indexMetaData, @Nullable IndexSearcherWrapper indexSearcherWrapper, - @Nullable EngineFactory engineFactory, + Function engineFactoryFn, Runnable globalCheckpointSyncer, IndexingOperationListener... listeners) throws IOException { @@ -264,7 +265,7 @@ protected IndexShard newShard(ShardRouting routing, IndexMetaData indexMetaData, final ShardId shardId = routing.shardId(); final NodeEnvironment.NodePath nodePath = new NodeEnvironment.NodePath(createTempDir()); ShardPath shardPath = new ShardPath(false, nodePath.resolve(shardId), nodePath.resolve(shardId), shardId); - return newShard(routing, shardPath, indexMetaData, indexSearcherWrapper, engineFactory, globalCheckpointSyncer, + return newShard(routing, shardPath, indexMetaData, indexSearcherWrapper, engineFactoryFn, globalCheckpointSyncer, EMPTY_EVENT_LISTENER, listeners); } @@ -280,7 +281,7 @@ protected IndexShard newShard(ShardRouting routing, IndexMetaData indexMetaData, */ protected IndexShard newShard(ShardRouting routing, ShardPath shardPath, IndexMetaData indexMetaData, @Nullable IndexSearcherWrapper indexSearcherWrapper, - @Nullable EngineFactory engineFactory, + Function engineFactoryFn, Runnable globalCheckpointSyncer, IndexEventListener indexEventListener, IndexingOperationListener... listeners) throws IOException { final Settings nodeSettings = Settings.builder().put("node.name", routing.currentNodeId()).build(); @@ -299,7 +300,7 @@ protected IndexShard newShard(ShardRouting routing, ShardPath shardPath, IndexMe ClusterSettings clusterSettings = new ClusterSettings(nodeSettings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); CircuitBreakerService breakerService = new HierarchyCircuitBreakerService(nodeSettings, clusterSettings); indexShard = new IndexShard(routing, indexSettings, shardPath, store, () -> null, indexCache, mapperService, similarityService, - engineFactory, indexEventListener, indexSearcherWrapper, threadPool, + engineFactoryFn, indexEventListener, indexSearcherWrapper, threadPool, BigArrays.NON_RECYCLING_INSTANCE, warmer, Collections.emptyList(), Arrays.asList(listeners), globalCheckpointSyncer, breakerService); success = true; @@ -336,7 +337,7 @@ protected IndexShard reinitShard(IndexShard current, ShardRouting routing, Index current.shardPath(), current.indexSettings().getIndexMetaData(), null, - current.engineFactory, + current.engineFactoryFn, current.getGlobalCheckpointSyncer(), EMPTY_EVENT_LISTENER, listeners); } From e150fdb70de61ebb137566b3c089bd23e2f353c5 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Mon, 18 Jun 2018 16:52:15 -0600 Subject: [PATCH 2/2] Fix WatcherPluginTests --- .../org/elasticsearch/xpack/watcher/WatcherPluginTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java index abb981053e730..af66d026c17c9 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java @@ -69,7 +69,7 @@ public void testWatcherDisabledTests() throws Exception { IndexSettings indexSettings = IndexSettingsModule.newIndexSettings(Watch.INDEX, settings); AnalysisRegistry registry = new AnalysisRegistry(TestEnvironment.newEnvironment(settings), emptyMap(), emptyMap(), emptyMap(), emptyMap(), emptyMap(), emptyMap(), emptyMap(), emptyMap(), emptyMap()); - IndexModule indexModule = new IndexModule(indexSettings, registry, new InternalEngineFactory()); + IndexModule indexModule = new IndexModule(indexSettings, registry, s -> new InternalEngineFactory()); // this will trip an assertion if the watcher indexing operation listener is null (which it is) but we try to add it watcher.onIndexModule(indexModule);