Skip to content

Commit 385b97f

Browse files
romseygeekromseygeek
authored
Choose postings format from FieldMapper instead of MappedFieldType (#77234)
Currently we configure per-field postings formats by asking the MapperService for the MappedFieldType of the field in question, and then checking to see if it is a completion field. If no MappedFieldType is available, we emit a warning. However, MappedFieldTypes are for search fields only, and so we end up emitting warnings for hidden sub-fields that have no corresponding field type, such as prefix or phrase accelerator fields on text mappers. This commit reworks things so that the MappingLookup is responsible for defining per-field postings formats, and will detect CompletionFieldMappers at build time. All fields that are not mapped to a completion field will just get the default postings format. This also means that we no longer need a logger instance on CodecService. Fixes #77183
1 parent f18b9d5 commit 385b97f

File tree

14 files changed

+67
-62
lines changed

14 files changed

+67
-62
lines changed

‎server/src/internalClusterTest/java/org/elasticsearch/indices/IndexingMemoryControllerIT.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
*/
88
package org.elasticsearch.indices;
99

10-
import org.apache.logging.log4j.LogManager;
1110
import org.elasticsearch.action.admin.indices.forcemerge.ForceMergeResponse;
1211
import org.elasticsearch.common.settings.Settings;
1312
import org.elasticsearch.common.util.CollectionUtils;
@@ -52,7 +51,7 @@ EngineConfig engineConfigWithLargerIndexingMemory(EngineConfig config) {
5251
IndexSettings indexSettings = new IndexSettings(config.getIndexSettings().getIndexMetadata(), settings);
5352
return new EngineConfig(config.getShardId(), config.getThreadPool(),
5453
indexSettings, config.getWarmer(), config.getStore(), config.getMergePolicy(), config.getAnalyzer(),
55-
config.getSimilarity(), new CodecService(null, LogManager.getLogger(IndexingMemoryControllerIT.class)),
54+
config.getSimilarity(), new CodecService(null),
5655
config.getEventListener(), config.getQueryCache(),
5756
config.getQueryCachingPolicy(), config.getTranslogConfig(), config.getFlushMergesAfter(),
5857
config.getExternalRefreshListener(), config.getInternalRefreshListener(), config.getIndexSort(),

‎server/src/main/java/org/elasticsearch/index/codec/CodecService.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
package org.elasticsearch.index.codec;
1010

11-
import org.apache.logging.log4j.Logger;
1211
import org.apache.lucene.codecs.Codec;
1312
import org.apache.lucene.codecs.lucene87.Lucene87Codec;
1413
import org.apache.lucene.codecs.lucene87.Lucene87Codec.Mode;
@@ -33,16 +32,16 @@ public class CodecService {
3332
/** the raw unfiltered lucene default. useful for testing */
3433
public static final String LUCENE_DEFAULT_CODEC = "lucene_default";
3534

36-
public CodecService(@Nullable MapperService mapperService, Logger logger) {
35+
public CodecService(@Nullable MapperService mapperService) {
3736
final var codecs = new HashMap<String, Codec>();
3837
if (mapperService == null) {
3938
codecs.put(DEFAULT_CODEC, new Lucene87Codec());
4039
codecs.put(BEST_COMPRESSION_CODEC, new Lucene87Codec(Mode.BEST_COMPRESSION));
4140
} else {
4241
codecs.put(DEFAULT_CODEC,
43-
new PerFieldMappingPostingFormatCodec(Mode.BEST_SPEED, mapperService, logger));
42+
new PerFieldMappingPostingFormatCodec(Mode.BEST_SPEED, mapperService));
4443
codecs.put(BEST_COMPRESSION_CODEC,
45-
new PerFieldMappingPostingFormatCodec(Mode.BEST_COMPRESSION, mapperService, logger));
44+
new PerFieldMappingPostingFormatCodec(Mode.BEST_COMPRESSION, mapperService));
4645
}
4746
codecs.put(LUCENE_DEFAULT_CODEC, Codec.getDefault());
4847
for (String codec : Codec.availableCodecs()) {

‎server/src/main/java/org/elasticsearch/index/codec/PerFieldMappingPostingFormatCodec.java

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,12 @@
88

99
package org.elasticsearch.index.codec;
1010

11-
import org.apache.logging.log4j.Logger;
1211
import org.apache.lucene.codecs.Codec;
1312
import org.apache.lucene.codecs.DocValuesFormat;
1413
import org.apache.lucene.codecs.PostingsFormat;
1514
import org.apache.lucene.codecs.lucene80.Lucene80DocValuesFormat;
1615
import org.apache.lucene.codecs.lucene87.Lucene87Codec;
1716
import org.elasticsearch.common.lucene.Lucene;
18-
import org.elasticsearch.index.mapper.CompletionFieldMapper;
19-
import org.elasticsearch.index.mapper.MappedFieldType;
2017
import org.elasticsearch.index.mapper.MapperService;
2118

2219
/**
@@ -28,7 +25,7 @@
2825
* configured for a specific field the default postings format is used.
2926
*/
3027
public class PerFieldMappingPostingFormatCodec extends Lucene87Codec {
31-
private final Logger logger;
28+
3229
private final MapperService mapperService;
3330
// Always enable compression on binary doc values
3431
private final DocValuesFormat docValuesFormat = new Lucene80DocValuesFormat(Lucene80DocValuesFormat.Mode.BEST_COMPRESSION);
@@ -38,21 +35,18 @@ public class PerFieldMappingPostingFormatCodec extends Lucene87Codec {
3835
"PerFieldMappingPostingFormatCodec must subclass the latest " + "lucene codec: " + Lucene.LATEST_CODEC;
3936
}
4037

41-
public PerFieldMappingPostingFormatCodec(Mode compressionMode, MapperService mapperService, Logger logger) {
38+
public PerFieldMappingPostingFormatCodec(Mode compressionMode, MapperService mapperService) {
4239
super(compressionMode);
4340
this.mapperService = mapperService;
44-
this.logger = logger;
4541
}
4642

4743
@Override
4844
public PostingsFormat getPostingsFormatForField(String field) {
49-
final MappedFieldType fieldType = mapperService.fieldType(field);
50-
if (fieldType == null) {
51-
logger.warn("no index mapper found for field: [{}] returning default postings format", field);
52-
} else if (fieldType instanceof CompletionFieldMapper.CompletionFieldType) {
53-
return CompletionFieldMapper.CompletionFieldType.postingsFormat();
45+
PostingsFormat format = mapperService.mappingLookup().getPostingsFormat(field);
46+
if (format == null) {
47+
return super.getPostingsFormatForField(field);
5448
}
55-
return super.getPostingsFormatForField(field);
49+
return format;
5650
}
5751

5852
@Override

‎server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import org.apache.lucene.document.FieldType;
1212
import org.apache.lucene.index.IndexOptions;
1313
import org.apache.lucene.index.Term;
14-
import org.apache.lucene.search.suggest.document.Completion84PostingsFormat;
1514
import org.apache.lucene.search.suggest.document.CompletionAnalyzer;
1615
import org.apache.lucene.search.suggest.document.CompletionQuery;
1716
import org.apache.lucene.search.suggest.document.FuzzyCompletionQuery;
@@ -209,8 +208,6 @@ private void checkCompletionContextsLimit() {
209208

210209
public static final class CompletionFieldType extends TermBasedFieldType {
211210

212-
private static PostingsFormat postingsFormat;
213-
214211
private ContextMappings contextMappings = null;
215212

216213
public CompletionFieldType(String name, NamedAnalyzer searchAnalyzer, Map<String, String> meta) {
@@ -236,16 +233,6 @@ public ContextMappings getContextMappings() {
236233
return contextMappings;
237234
}
238235

239-
/**
240-
* @return postings format to use for this field-type
241-
*/
242-
public static synchronized PostingsFormat postingsFormat() {
243-
if (postingsFormat == null) {
244-
postingsFormat = new Completion84PostingsFormat();
245-
}
246-
return postingsFormat;
247-
}
248-
249236
/**
250237
* Completion prefix query
251238
*/
@@ -313,6 +300,10 @@ public CompletionFieldType fieldType() {
313300
return (CompletionFieldType) super.fieldType();
314301
}
315302

303+
static PostingsFormat postingsFormat() {
304+
return PostingsFormat.forName("Completion84");
305+
}
306+
316307
@Override
317308
public boolean parsesArrayValue() {
318309
return true;

‎server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
package org.elasticsearch.index.mapper;
1010

11+
import org.apache.lucene.codecs.PostingsFormat;
1112
import org.elasticsearch.index.IndexSettings;
1213
import org.elasticsearch.index.analysis.IndexAnalyzers;
1314
import org.elasticsearch.index.analysis.NamedAnalyzer;
@@ -53,6 +54,7 @@ private CacheKey() {}
5354
private final List<FieldMapper> indexTimeScriptMappers = new ArrayList<>();
5455
private final Mapping mapping;
5556
private final Set<String> shadowedFields;
57+
private final Set<String> completionFields = new HashSet<>();
5658

5759
/**
5860
* Creates a new {@link MappingLookup} instance by parsing the provided mapping and extracting its field definitions.
@@ -148,6 +150,9 @@ private MappingLookup(Mapping mapping,
148150
if (mapper.hasScript()) {
149151
indexTimeScriptMappers.add(mapper);
150152
}
153+
if (mapper instanceof CompletionFieldMapper) {
154+
completionFields.add(mapper.name());
155+
}
151156
}
152157

153158
for (FieldAliasMapper aliasMapper : aliasMappers) {
@@ -213,6 +218,15 @@ public boolean isShadowed(String field) {
213218
return shadowedFields.contains(field);
214219
}
215220

221+
/**
222+
* Gets the postings format for a particular field
223+
* @param field the field to retrieve a postings format for
224+
* @return the postings format for the field, or {@code null} if the default format should be used
225+
*/
226+
public PostingsFormat getPostingsFormat(String field) {
227+
return completionFields.contains(field) ? CompletionFieldMapper.postingsFormat() : null;
228+
}
229+
216230
void checkLimits(IndexSettings settings) {
217231
checkFieldLimit(settings.getMappingTotalFieldsLimit());
218232
checkObjectDepthLimit(settings.getMappingDepthLimit());

‎server/src/main/java/org/elasticsearch/index/shard/IndexShard.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ public IndexShard(
312312
assert shardRouting.initializing();
313313
this.shardRouting = shardRouting;
314314
final Settings settings = indexSettings.getSettings();
315-
this.codecService = new CodecService(mapperService, logger);
315+
this.codecService = new CodecService(mapperService);
316316
this.warmer = warmer;
317317
this.similarityService = similarityService;
318318
Objects.requireNonNull(store, "Store must be provided to the index shard");

‎server/src/test/java/org/elasticsearch/index/codec/CodecTests.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
package org.elasticsearch.index.codec;
1010

11-
import org.apache.logging.log4j.LogManager;
1211
import org.apache.lucene.codecs.Codec;
1312
import org.apache.lucene.codecs.lucene80.Lucene80DocValuesFormat;
1413
import org.apache.lucene.codecs.lucene87.Lucene87Codec;
@@ -110,7 +109,7 @@ private CodecService createCodecService() throws IOException {
110109
MapperPlugin.NOOP_FIELD_FILTER);
111110
MapperService service = new MapperService(settings, indexAnalyzers, xContentRegistry(), similarityService, mapperRegistry,
112111
() -> null, () -> false, ScriptCompiler.NONE);
113-
return new CodecService(service, LogManager.getLogger("test"));
112+
return new CodecService(service);
114113
}
115114

116115
}

‎server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2603,7 +2603,7 @@ public void testFailStart() throws IOException {
26032603
}
26042604

26052605
public void testSettings() {
2606-
CodecService codecService = new CodecService(null, logger);
2606+
CodecService codecService = new CodecService(null);
26072607
LiveIndexWriterConfig currentIndexWriterConfig = engine.getCurrentIndexWriterConfig();
26082608

26092609
assertEquals(engine.config().getCodec().getName(), codecService.codec(codecName).getName());
@@ -2935,7 +2935,7 @@ public void testRecoverFromForeignTranslog() throws IOException {
29352935
newMergePolicy(),
29362936
config.getAnalyzer(),
29372937
config.getSimilarity(),
2938-
new CodecService(null, logger),
2938+
new CodecService(null),
29392939
config.getEventListener(),
29402940
IndexSearcher.getDefaultQueryCache(),
29412941
IndexSearcher.getDefaultQueryCachingPolicy(),
@@ -6018,7 +6018,7 @@ public void testNotWarmUpSearcherInEngineCtor() throws Exception {
60186018
createTempDir(), config.getTranslogConfig().getIndexSettings(), config.getTranslogConfig().getBigArrays());
60196019
EngineConfig configWithWarmer = new EngineConfig(config.getShardId(), config.getThreadPool(),
60206020
config.getIndexSettings(), warmer, store, config.getMergePolicy(), config.getAnalyzer(),
6021-
config.getSimilarity(), new CodecService(null, logger), config.getEventListener(), config.getQueryCache(),
6021+
config.getSimilarity(), new CodecService(null), config.getEventListener(), config.getQueryCache(),
60226022
config.getQueryCachingPolicy(), translogConfig, config.getFlushMergesAfter(),
60236023
config.getExternalRefreshListener(), config.getInternalRefreshListener(), config.getIndexSort(),
60246024
config.getCircuitBreakerService(), config.getGlobalCheckpointSupplier(), config.retentionLeasesSupplier(),

‎server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,11 @@
1010
import org.apache.lucene.analysis.TokenStream;
1111
import org.apache.lucene.analysis.core.SimpleAnalyzer;
1212
import org.apache.lucene.analysis.standard.StandardAnalyzer;
13+
import org.apache.lucene.codecs.Codec;
1314
import org.apache.lucene.document.SortedSetDocValuesField;
1415
import org.apache.lucene.index.IndexableField;
1516
import org.apache.lucene.search.Query;
17+
import org.apache.lucene.search.suggest.document.Completion84PostingsFormat;
1618
import org.apache.lucene.search.suggest.document.CompletionAnalyzer;
1719
import org.apache.lucene.search.suggest.document.ContextSuggestField;
1820
import org.apache.lucene.search.suggest.document.FuzzyCompletionQuery;
@@ -35,6 +37,8 @@
3537
import org.elasticsearch.index.analysis.AnalyzerScope;
3638
import org.elasticsearch.index.analysis.IndexAnalyzers;
3739
import org.elasticsearch.index.analysis.NamedAnalyzer;
40+
import org.elasticsearch.index.codec.CodecService;
41+
import org.elasticsearch.index.codec.PerFieldMappingPostingFormatCodec;
3842
import org.hamcrest.FeatureMatcher;
3943
import org.hamcrest.Matcher;
4044
import org.hamcrest.Matchers;
@@ -114,6 +118,15 @@ protected IndexAnalyzers createIndexAnalyzers(IndexSettings indexSettings) {
114118
);
115119
}
116120

121+
public void testPostingsFormat() throws IOException {
122+
MapperService mapperService = createMapperService(fieldMapping(this::minimalMapping));
123+
CodecService codecService = new CodecService(mapperService);
124+
Codec codec = codecService.codec("default");
125+
assertThat(codec, instanceOf(PerFieldMappingPostingFormatCodec.class));
126+
PerFieldMappingPostingFormatCodec perFieldCodec = (PerFieldMappingPostingFormatCodec) codec;
127+
assertThat(perFieldCodec.getPostingsFormatForField("field"), instanceOf(Completion84PostingsFormat.class));
128+
}
129+
117130
public void testDefaultConfiguration() throws IOException {
118131

119132
DocumentMapper defaultMapper = createDocumentMapper(fieldMapping(this::minimalMapping));

‎server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3924,7 +3924,7 @@ public void testCloseShardWhileEngineIsWarming() throws Exception {
39243924
};
39253925
EngineConfig configWithWarmer = new EngineConfig(config.getShardId(), config.getThreadPool(),
39263926
config.getIndexSettings(), warmer, config.getStore(), config.getMergePolicy(), config.getAnalyzer(),
3927-
config.getSimilarity(), new CodecService(null, logger), config.getEventListener(), config.getQueryCache(),
3927+
config.getSimilarity(), new CodecService(null), config.getEventListener(), config.getQueryCache(),
39283928
config.getQueryCachingPolicy(), config.getTranslogConfig(), config.getFlushMergesAfter(),
39293929
config.getExternalRefreshListener(), config.getInternalRefreshListener(), config.getIndexSort(),
39303930
config.getCircuitBreakerService(), config.getGlobalCheckpointSupplier(), config.retentionLeasesSupplier(),

‎server/src/test/java/org/elasticsearch/index/shard/RefreshListenersTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ public void onFailedEngine(String reason, @Nullable Exception e) {
126126
newMergePolicy(),
127127
iwc.getAnalyzer(),
128128
iwc.getSimilarity(),
129-
new CodecService(null, logger),
129+
new CodecService(null),
130130
eventListener,
131131
IndexSearcher.getDefaultQueryCache(),
132132
IndexSearcher.getDefaultQueryCachingPolicy(),

‎server/src/test/java/org/elasticsearch/indices/IndexingMemoryControllerTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ EngineConfig configWithRefreshListener(EngineConfig config, ReferenceManager.Ref
367367
internalRefreshListener.add(listener);
368368
return new EngineConfig(config.getShardId(), config.getThreadPool(),
369369
config.getIndexSettings(), config.getWarmer(), config.getStore(), config.getMergePolicy(), config.getAnalyzer(),
370-
config.getSimilarity(), new CodecService(null, logger), config.getEventListener(), config.getQueryCache(),
370+
config.getSimilarity(), new CodecService(null), config.getEventListener(), config.getQueryCache(),
371371
config.getQueryCachingPolicy(), config.getTranslogConfig(), config.getFlushMergesAfter(),
372372
config.getExternalRefreshListener(), internalRefreshListener, config.getIndexSort(),
373373
config.getCircuitBreakerService(), config.getGlobalCheckpointSupplier(), config.retentionLeasesSupplier(),

‎test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ protected Settings indexSettings() {
195195
public void setUp() throws Exception {
196196
super.setUp();
197197
primaryTerm.set(randomLongBetween(1, Long.MAX_VALUE));
198-
CodecService codecService = new CodecService(null, logger);
198+
CodecService codecService = new CodecService(null);
199199
String name = Codec.getDefault().getName();
200200
if (Arrays.asList(codecService.availableCodecs()).contains(name)) {
201201
// some codecs are read only so we only take the ones that we have in the service and randomly
@@ -234,7 +234,7 @@ public void setUp() throws Exception {
234234
public EngineConfig copy(EngineConfig config, LongSupplier globalCheckpointSupplier) {
235235
return new EngineConfig(config.getShardId(), config.getThreadPool(), config.getIndexSettings(),
236236
config.getWarmer(), config.getStore(), config.getMergePolicy(), config.getAnalyzer(), config.getSimilarity(),
237-
new CodecService(null, logger), config.getEventListener(), config.getQueryCache(), config.getQueryCachingPolicy(),
237+
new CodecService(null), config.getEventListener(), config.getQueryCache(), config.getQueryCachingPolicy(),
238238
config.getTranslogConfig(), config.getFlushMergesAfter(),
239239
config.getExternalRefreshListener(), Collections.emptyList(), config.getIndexSort(),
240240
config.getCircuitBreakerService(), globalCheckpointSupplier, config.retentionLeasesSupplier(),
@@ -244,7 +244,7 @@ public EngineConfig copy(EngineConfig config, LongSupplier globalCheckpointSuppl
244244
public EngineConfig copy(EngineConfig config, Analyzer analyzer) {
245245
return new EngineConfig(config.getShardId(), config.getThreadPool(), config.getIndexSettings(),
246246
config.getWarmer(), config.getStore(), config.getMergePolicy(), analyzer, config.getSimilarity(),
247-
new CodecService(null, logger), config.getEventListener(), config.getQueryCache(), config.getQueryCachingPolicy(),
247+
new CodecService(null), config.getEventListener(), config.getQueryCache(), config.getQueryCachingPolicy(),
248248
config.getTranslogConfig(), config.getFlushMergesAfter(),
249249
config.getExternalRefreshListener(), Collections.emptyList(), config.getIndexSort(),
250250
config.getCircuitBreakerService(), config.getGlobalCheckpointSupplier(), config.retentionLeasesSupplier(),
@@ -254,7 +254,7 @@ public EngineConfig copy(EngineConfig config, Analyzer analyzer) {
254254
public EngineConfig copy(EngineConfig config, MergePolicy mergePolicy) {
255255
return new EngineConfig(config.getShardId(), config.getThreadPool(), config.getIndexSettings(),
256256
config.getWarmer(), config.getStore(), mergePolicy, config.getAnalyzer(), config.getSimilarity(),
257-
new CodecService(null, logger), config.getEventListener(), config.getQueryCache(), config.getQueryCachingPolicy(),
257+
new CodecService(null), config.getEventListener(), config.getQueryCache(), config.getQueryCachingPolicy(),
258258
config.getTranslogConfig(), config.getFlushMergesAfter(),
259259
config.getExternalRefreshListener(), Collections.emptyList(), config.getIndexSort(),
260260
config.getCircuitBreakerService(), config.getGlobalCheckpointSupplier(), config.retentionLeasesSupplier(),
@@ -656,7 +656,7 @@ public EngineConfig config(
656656
mergePolicy,
657657
iwc.getAnalyzer(),
658658
iwc.getSimilarity(),
659-
new CodecService(null, logger),
659+
new CodecService(null),
660660
eventListener,
661661
IndexSearcher.getDefaultQueryCache(),
662662
IndexSearcher.getDefaultQueryCachingPolicy(),
@@ -680,7 +680,7 @@ protected EngineConfig config(EngineConfig config, Store store, Path translogPat
680680
TranslogConfig translogConfig = new TranslogConfig(shardId, translogPath, indexSettings, BigArrays.NON_RECYCLING_INSTANCE);
681681
return new EngineConfig(config.getShardId(), config.getThreadPool(),
682682
indexSettings, config.getWarmer(), store, config.getMergePolicy(), config.getAnalyzer(), config.getSimilarity(),
683-
new CodecService(null, logger), config.getEventListener(), config.getQueryCache(), config.getQueryCachingPolicy(),
683+
new CodecService(null), config.getEventListener(), config.getQueryCache(), config.getQueryCachingPolicy(),
684684
translogConfig, config.getFlushMergesAfter(), config.getExternalRefreshListener(),
685685
config.getInternalRefreshListener(), config.getIndexSort(), config.getCircuitBreakerService(),
686686
config.getGlobalCheckpointSupplier(), config.retentionLeasesSupplier(),

0 commit comments

Comments
 (0)