Alternative approach

elastic · Mar 29, 2024 · 38b307f · 38b307f
1 parent 1d6baa4
commit 38b307f
Show file tree

Hide file tree

Showing 37 changed files with 192 additions and 310 deletions.
diff --git a/benchmarks/src/main/java/org/elasticsearch/benchmark/index/mapper/MapperServiceFactory.java b/benchmarks/src/main/java/org/elasticsearch/benchmark/index/mapper/MapperServiceFactory.java
@@ -26,7 +26,6 @@
 import org.elasticsearch.index.mapper.ProvidedIdFieldMapper;
 import org.elasticsearch.index.similarity.SimilarityService;
 import org.elasticsearch.indices.IndicesModule;
-import org.elasticsearch.indices.MapperMetrics;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptCompiler;
 import org.elasticsearch.script.ScriptContext;
@@ -72,8 +71,7 @@ public static MapperService create(String mappings) {
                 public <T> T compile(Script script, ScriptContext<T> scriptContext) {
                     throw new UnsupportedOperationException();
                 }
-            },
-            MapperMetrics.NOOP
+            }
         );
 
         try {

diff --git a/benchmarks/src/main/java/org/elasticsearch/benchmark/search/QueryParserHelperBenchmark.java b/benchmarks/src/main/java/org/elasticsearch/benchmark/search/QueryParserHelperBenchmark.java
@@ -39,7 +39,6 @@
 import org.elasticsearch.index.shard.IndexShard;
 import org.elasticsearch.index.similarity.SimilarityService;
 import org.elasticsearch.indices.IndicesModule;
-import org.elasticsearch.indices.MapperMetrics;
 import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptCompiler;
@@ -187,8 +186,7 @@ protected final MapperService createMapperService(String mappings) {
                 public <T> T compile(Script script, ScriptContext<T> scriptContext) {
                     throw new UnsupportedOperationException();
                 }
-            },
-            MapperMetrics.NOOP
+            }
         );
 
         try {

diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadataVerifier.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadataVerifier.java
@@ -26,7 +26,6 @@
 import org.elasticsearch.index.mapper.MapperRegistry;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.similarity.SimilarityService;
-import org.elasticsearch.indices.MapperMetrics;
 import org.elasticsearch.script.ScriptCompiler;
 import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.xcontent.NamedXContentRegistry;
@@ -59,16 +58,14 @@ public class IndexMetadataVerifier {
     private final MapperRegistry mapperRegistry;
     private final IndexScopedSettings indexScopedSettings;
     private final ScriptCompiler scriptService;
-    private final MapperMetrics mapperMetrics;
 
     public IndexMetadataVerifier(
         Settings settings,
         ClusterService clusterService,
         NamedXContentRegistry xContentRegistry,
         MapperRegistry mapperRegistry,
         IndexScopedSettings indexScopedSettings,
-        ScriptCompiler scriptCompiler,
-        MapperMetrics mapperMetrics
+        ScriptCompiler scriptCompiler
     ) {
         this.settings = settings;
         this.clusterService = clusterService;
@@ -77,7 +74,6 @@ public IndexMetadataVerifier(
         this.mapperRegistry = mapperRegistry;
         this.indexScopedSettings = indexScopedSettings;
         this.scriptService = scriptCompiler;
-        this.mapperMetrics = mapperMetrics;
     }
 
     /**
@@ -186,8 +182,7 @@ protected TokenStreamComponents createComponents(String fieldName) {
                     mapperRegistry,
                     () -> null,
                     indexSettings.getMode().idFieldMapperWithoutFieldData(),
-                    scriptService,
-                    mapperMetrics
+                    scriptService
                 )
             ) {
                 mapperService.merge(indexMetadata, MapperService.MergeReason.MAPPING_RECOVERY);

diff --git a/server/src/main/java/org/elasticsearch/common/metrics/MetricAccessor.java b/server/src/main/java/org/elasticsearch/common/metrics/MetricAccessor.java
@@ -0,0 +1,54 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.common.metrics;
+
+import java.util.Objects;
+
+/**
+ * Provides easy access to metrics (or anything really)
+ * without the need to pass metrics object explicitly.
+ * Intended to use as a singleton in consumers but still allows injecting test values
+ * by first checking thread local value before falling back to set value.
+ */
+public class MetricAccessor<T> {
+    // Intentionally not static - we expect users to only create instance
+    // per each "global" resource.
+    private ThreadLocal<T> threadLocal = ThreadLocal.withInitial(() -> null);
+    private T value;
+
+    public MetricAccessor(T value) {
+        this.value = Objects.requireNonNull(value);
+    }
+
+    public AutoCloseable initForTest(T value) {
+        threadLocal.set(value);
+
+        return new Reset(threadLocal);
+    }
+
+    /**
+     * returns stored value or thread local value if set
+     * @return T
+     */
+    public T get() {
+        var local = threadLocal.get();
+        if (local != null) {
+            return local;
+        }
+
+        return value;
+    }
+
+    private record Reset(ThreadLocal<?> threadLocal) implements AutoCloseable {
+        @Override
+        public void close() throws Exception {
+            threadLocal.remove();
+        }
+    }
+}
diff --git a/server/src/main/java/org/elasticsearch/index/IndexModule.java b/server/src/main/java/org/elasticsearch/index/IndexModule.java
@@ -53,7 +53,6 @@
 import org.elasticsearch.index.similarity.SimilarityService;
 import org.elasticsearch.index.store.FsDirectoryFactory;
 import org.elasticsearch.indices.IndicesQueryCache;
-import org.elasticsearch.indices.MapperMetrics;
 import org.elasticsearch.indices.breaker.CircuitBreakerService;
 import org.elasticsearch.indices.fielddata.cache.IndicesFieldDataCache;
 import org.elasticsearch.indices.recovery.RecoveryState;
@@ -476,8 +475,7 @@ public IndexService newIndexService(
         IdFieldMapper idFieldMapper,
         ValuesSourceRegistry valuesSourceRegistry,
         IndexStorePlugin.IndexFoldersDeletionListener indexFoldersDeletionListener,
-        Map<String, IndexStorePlugin.SnapshotCommitSupplier> snapshotCommitSuppliers,
-        MapperMetrics metrics
+        Map<String, IndexStorePlugin.SnapshotCommitSupplier> snapshotCommitSuppliers
     ) throws IOException {
         final IndexEventListener eventListener = freeze();
         Function<IndexService, CheckedFunction<DirectoryReader, DirectoryReader, IOException>> readerWrapperFactory = indexReaderWrapper
@@ -538,8 +536,7 @@ public IndexService newIndexService(
                 recoveryStateFactory,
                 indexFoldersDeletionListener,
                 snapshotCommitSupplier,
-                indexCommitListener.get(),
-                metrics
+                indexCommitListener.get()
             );
             success = true;
             return indexService;
@@ -636,8 +633,7 @@ public MapperService newIndexMapperService(
         ClusterService clusterService,
         XContentParserConfiguration parserConfiguration,
         MapperRegistry mapperRegistry,
-        ScriptService scriptService,
-        MapperMetrics mapperMetrics
+        ScriptService scriptService
     ) throws IOException {
         return new MapperService(
             clusterService,
@@ -650,8 +646,7 @@ public MapperService newIndexMapperService(
                 throw new UnsupportedOperationException("no index query shard context available");
             },
             indexSettings.getMode().idFieldMapperWithoutFieldData(),
-            scriptService,
-            mapperMetrics
+            scriptService
         );
     }
 

diff --git a/server/src/main/java/org/elasticsearch/index/IndexService.java b/server/src/main/java/org/elasticsearch/index/IndexService.java
@@ -77,7 +77,6 @@
 import org.elasticsearch.index.similarity.SimilarityService;
 import org.elasticsearch.index.store.Store;
 import org.elasticsearch.index.translog.Translog;
-import org.elasticsearch.indices.MapperMetrics;
 import org.elasticsearch.indices.breaker.CircuitBreakerService;
 import org.elasticsearch.indices.cluster.IndicesClusterStateService;
 import org.elasticsearch.indices.fielddata.cache.IndicesFieldDataCache;
@@ -192,8 +191,7 @@ public IndexService(
         IndexStorePlugin.RecoveryStateFactory recoveryStateFactory,
         IndexStorePlugin.IndexFoldersDeletionListener indexFoldersDeletionListener,
         IndexStorePlugin.SnapshotCommitSupplier snapshotCommitSupplier,
-        Engine.IndexCommitListener indexCommitListener,
-        MapperMetrics mapperMetrics
+        Engine.IndexCommitListener indexCommitListener
     ) {
         super(indexSettings);
         assert indexCreationContext != IndexCreationContext.RELOAD_ANALYZERS
@@ -220,8 +218,7 @@ public IndexService(
                 // we parse all percolator queries as they would be parsed on shard 0
                 () -> newSearchExecutionContext(0, 0, null, System::currentTimeMillis, null, emptyMap()),
                 idFieldMapper,
-                scriptService,
-                mapperMetrics
+                scriptService
             );
             this.indexFieldData = new IndexFieldDataService(indexSettings, indicesFieldDataCache, circuitBreakerService);
             if (indexSettings.getIndexSortConfig().hasIndexSort()) {

diff --git a/server/src/main/java/org/elasticsearch/index/get/ShardGetService.java b/server/src/main/java/org/elasticsearch/index/get/ShardGetService.java
@@ -296,7 +296,9 @@ private GetResult innerGetFetch(
         Map<String, DocumentField> documentFields = null;
         Map<String, DocumentField> metadataFields = null;
         DocIdAndVersion docIdAndVersion = get.docIdAndVersion();
-        SourceLoader loader = mapperService.getSourceLoader(mappingLookup, forceSyntheticSource);
+        SourceLoader loader = forceSyntheticSource
+            ? new SourceLoader.Synthetic(mappingLookup.getMapping())
+            : mappingLookup.newSourceLoader();
         StoredFieldLoader storedFieldLoader = buildStoredFieldLoader(storedFields, fetchSourceContext, loader);
         LeafStoredFieldLoader leafStoredFieldLoader = storedFieldLoader.getLoader(docIdAndVersion.reader.getContext(), null);
         try {

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java
@@ -28,7 +28,6 @@
 import org.elasticsearch.index.query.SearchExecutionContext;
 import org.elasticsearch.index.similarity.SimilarityService;
 import org.elasticsearch.indices.IndicesModule;
-import org.elasticsearch.indices.MapperMetrics;
 import org.elasticsearch.script.ScriptCompiler;
 import org.elasticsearch.xcontent.NamedXContentRegistry;
 import org.elasticsearch.xcontent.ToXContent;
@@ -153,7 +152,6 @@ public boolean isAutoUpdate() {
     private final IndexVersion indexVersionCreated;
     private final MapperRegistry mapperRegistry;
     private final Supplier<MappingParserContext> mappingParserContextSupplier;
-    private final MapperMetrics mapperMetrics;
 
     private volatile DocumentMapper mapper;
     private volatile long mappingVersion;
@@ -167,8 +165,7 @@ public MapperService(
         MapperRegistry mapperRegistry,
         Supplier<SearchExecutionContext> searchExecutionContextSupplier,
         IdFieldMapper idFieldMapper,
-        ScriptCompiler scriptCompiler,
-        MapperMetrics metrics
+        ScriptCompiler scriptCompiler
     ) {
         this(
             () -> clusterService.state().getMinTransportVersion(),
@@ -179,8 +176,7 @@ public MapperService(
             mapperRegistry,
             searchExecutionContextSupplier,
             idFieldMapper,
-            scriptCompiler,
-            metrics
+            scriptCompiler
         );
     }
 
@@ -194,8 +190,7 @@ public MapperService(
         MapperRegistry mapperRegistry,
         Supplier<SearchExecutionContext> searchExecutionContextSupplier,
         IdFieldMapper idFieldMapper,
-        ScriptCompiler scriptCompiler,
-        MapperMetrics mapperMetrics
+        ScriptCompiler scriptCompiler
     ) {
         super(indexSettings);
         this.indexVersionCreated = indexSettings.getIndexVersionCreated();
@@ -211,8 +206,7 @@ public MapperService(
             scriptCompiler,
             indexAnalyzers,
             indexSettings,
-            idFieldMapper,
-            mapperMetrics
+            idFieldMapper
         );
         this.documentParser = new DocumentParser(parserConfiguration, this.mappingParserContextSupplier.get());
         Map<String, MetadataFieldMapper.TypeParser> metadataMapperParsers = mapperRegistry.getMetadataMapperParsers(
@@ -224,7 +218,6 @@ public MapperService(
             this::getMetadataMappers,
             this::resolveDocumentType
         );
-        this.mapperMetrics = mapperMetrics;
     }
 
     public boolean hasNested() {
@@ -786,12 +779,4 @@ public DynamicTemplate[] getAllDynamicTemplates() {
     public MapperRegistry getMapperRegistry() {
         return mapperRegistry;
     }
-
-    public SourceLoader getSourceLoader(MappingLookup mappingLookup, boolean forceSyntheticSource) {
-        if (forceSyntheticSource) {
-            return new SourceLoader.Synthetic(mappingLookup.getMapping(), mapperMetrics.getSyntheticSourceMetrics());
-        }
-
-        return mappingLookup.newSourceLoader();
-    }
 }
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MappingParserContext.java b/server/src/main/java/org/elasticsearch/index/mapper/MappingParserContext.java
@@ -16,7 +16,6 @@
 import org.elasticsearch.index.analysis.IndexAnalyzers;
 import org.elasticsearch.index.query.SearchExecutionContext;
 import org.elasticsearch.index.similarity.SimilarityProvider;
-import org.elasticsearch.indices.MapperMetrics;
 import org.elasticsearch.script.ScriptCompiler;
 
 import java.util.function.Function;
@@ -39,7 +38,6 @@ public class MappingParserContext {
     private final IndexSettings indexSettings;
     private final IdFieldMapper idFieldMapper;
     private final long mappingObjectDepthLimit;
-    private final MapperMetrics mapperMetrics;
     private long mappingObjectDepth = 0;
 
     public MappingParserContext(
@@ -52,8 +50,7 @@ public MappingParserContext(
         ScriptCompiler scriptCompiler,
         IndexAnalyzers indexAnalyzers,
         IndexSettings indexSettings,
-        IdFieldMapper idFieldMapper,
-        MapperMetrics mapperMetrics
+        IdFieldMapper idFieldMapper
     ) {
         this.similarityLookupService = similarityLookupService;
         this.typeParsers = typeParsers;
@@ -66,7 +63,6 @@ public MappingParserContext(
         this.indexSettings = indexSettings;
         this.idFieldMapper = idFieldMapper;
         this.mappingObjectDepthLimit = indexSettings.getMappingDepthLimit();
-        this.mapperMetrics = mapperMetrics;
     }
 
     public IndexAnalyzers getIndexAnalyzers() {
@@ -136,10 +132,6 @@ public ScriptCompiler scriptCompiler() {
         return scriptCompiler;
     }
 
-    public MapperMetrics getIndicesMetrics() {
-        return mapperMetrics;
-    }
-
     void incrementMappingObjectDepth() throws MapperParsingException {
         mappingObjectDepth++;
         if (mappingObjectDepth > mappingObjectDepthLimit) {
@@ -167,8 +159,7 @@ private static class MultiFieldParserContext extends MappingParserContext {
                 in.scriptCompiler,
                 in.indexAnalyzers,
                 in.indexSettings,
-                in.idFieldMapper,
-                in.mapperMetrics
+                in.idFieldMapper
             );
         }
 
@@ -197,8 +188,7 @@ private static class DynamicTemplateParserContext extends MappingParserContext {
                 in.scriptCompiler,
                 in.indexAnalyzers,
                 in.indexSettings,
-                in.idFieldMapper,
-                in.mapperMetrics
+                in.idFieldMapper
             );
             this.dateFormatter = dateFormatter;
         }