Refactored the EngineConfigFactory / IndexShard instantiation of the …

…CodecService Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
opensearch-project · Mar 8, 2022 · 5379899 · 5379899
1 parent eeeab29
commit 5379899
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 65 deletions.
diff --git a/server/src/main/java/org/opensearch/index/codec/CodecServiceConfig.java b/server/src/main/java/org/opensearch/index/codec/CodecServiceConfig.java
@@ -23,7 +23,7 @@ public final class CodecServiceConfig {
     private final MapperService mapperService;
     private final Logger logger;
 
-    public CodecServiceConfig(IndexSettings indexSettings, @Nullable MapperService mapperService, Logger logger) {
+    public CodecServiceConfig(IndexSettings indexSettings, @Nullable MapperService mapperService, @Nullable Logger logger) {
         this.indexSettings = Objects.requireNonNull(indexSettings);
         this.mapperService = mapperService;
         this.logger = logger;
@@ -38,6 +38,7 @@ public MapperService getMapperService() {
         return mapperService;
     }
 
+    @Nullable
     public Logger getLogger() {
         return logger;
     }

diff --git a/server/src/main/java/org/opensearch/index/engine/EngineConfigFactory.java b/server/src/main/java/org/opensearch/index/engine/EngineConfigFactory.java
@@ -16,6 +16,7 @@
 import org.apache.lucene.search.ReferenceManager;
 import org.apache.lucene.search.Sort;
 import org.apache.lucene.search.similarities.Similarity;
+import org.opensearch.common.Nullable;
 import org.opensearch.common.unit.TimeValue;
 import org.opensearch.index.IndexSettings;
 import org.opensearch.index.codec.CodecService;
@@ -120,9 +121,7 @@ public EngineConfigFactory(PluginsService pluginsService, IndexSettings idxSetti
 
     /**
      * 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,
@@ -147,61 +146,10 @@ 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, /* mapperService */
-            null /* logger */
-        );
-    }
-
-    /** 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<ReferenceManager.RefreshListener> externalRefreshListener,
-        List<ReferenceManager.RefreshListener> internalRefreshListener,
-        Sort indexSort,
-        CircuitBreakerService circuitBreakerService,
-        LongSupplier globalCheckpointSupplier,
-        Supplier<RetentionLeases> retentionLeasesSupplier,
-        LongSupplier primaryTermSupplier,
-        EngineConfig.TombstoneDocSupplier tombstoneDocSupplier,
-        MapperService mapperService,
-        Logger logger
-    ) {
+        CodecService codecServiceToUse = codecService;
+        if (codecService == null && this.codecServiceFactory != null) {
+            codecServiceToUse = newCodecServiceOrDefault(indexSettings, null, null, null);
+        }
 
         return new EngineConfig(
             shardId,
@@ -212,9 +160,7 @@ public EngineConfig newEngineConfig(
             mergePolicy,
             analyzer,
             similarity,
-            this.codecServiceFactory != null
-                ? this.codecServiceFactory.createCodecService(new CodecServiceConfig(indexSettings, mapperService, logger))
-                : codecService,
+            codecServiceToUse,
             eventListener,
             queryCache,
             queryCachingPolicy,
@@ -231,4 +177,15 @@ public EngineConfig newEngineConfig(
             tombstoneDocSupplier
         );
     }
+
+    public CodecService newCodecServiceOrDefault(
+        IndexSettings indexSettings,
+        @Nullable MapperService mapperService,
+        Logger logger,
+        CodecService defaultCodecService
+    ) {
+        return this.codecServiceFactory != null
+            ? this.codecServiceFactory.createCodecService(new CodecServiceConfig(indexSettings, mapperService, logger))
+            : defaultCodecService;
+    }
 }
diff --git a/server/src/main/java/org/opensearch/index/shard/IndexShard.java b/server/src/main/java/org/opensearch/index/shard/IndexShard.java
@@ -3155,6 +3155,7 @@ private EngineConfig newEngineConfig(LongSupplier globalCheckpointSupplier) {
                 this.warmer.warm(reader);
             }
         };
+
         return this.engineConfigFactory.newEngineConfig(
             shardId,
             threadPool,
@@ -3164,7 +3165,7 @@ private EngineConfig newEngineConfig(LongSupplier globalCheckpointSupplier) {
             indexSettings.getMergePolicy(),
             mapperService != null ? mapperService.indexAnalyzer() : null,
             similarityService.similarity(mapperService),
-            codecService,
+            engineConfigFactory.newCodecServiceOrDefault(indexSettings, mapperService, logger, codecService),
             shardEventListener,
             indexCache != null ? indexCache.query() : null,
             cachingPolicy,
@@ -3177,9 +3178,7 @@ private EngineConfig newEngineConfig(LongSupplier globalCheckpointSupplier) {
             globalCheckpointSupplier,
             replicationTracker::getRetentionLeases,
             () -> getOperationPrimaryTerm(),
-            tombstoneDocSupplier(),
-            mapperService,
-            logger
+            tombstoneDocSupplier()
         );
     }