Skip to content

Commit 3ca46cc

Browse files
romseygeekromseygeek
committed
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 bab0387 commit 3ca46cc

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 MapBuilder<String, Codec> codecs = MapBuilder.<String, Codec>newMapBuilder();
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;
@@ -205,8 +204,6 @@ private void checkCompletionContextsLimit() {
205204

206205
public static final class CompletionFieldType extends TermBasedFieldType {
207206

208-
private static PostingsFormat postingsFormat;
209-
210207
private ContextMappings contextMappings = null;
211208

212209
public CompletionFieldType(String name, NamedAnalyzer searchAnalyzer, Map<String, String> meta) {
@@ -232,16 +229,6 @@ public ContextMappings getContextMappings() {
232229
return contextMappings;
233230
}
234231

235-
/**
236-
* @return postings format to use for this field-type
237-
*/
238-
public static synchronized PostingsFormat postingsFormat() {
239-
if (postingsFormat == null) {
240-
postingsFormat = new Completion84PostingsFormat();
241-
}
242-
return postingsFormat;
243-
}
244-
245232
/**
246233
* Completion prefix query
247234
*/
@@ -309,6 +296,10 @@ public CompletionFieldType fieldType() {
309296
return (CompletionFieldType) super.fieldType();
310297
}
311298

299+
static PostingsFormat postingsFormat() {
300+
return PostingsFormat.forName("Completion84");
301+
}
302+
312303
@Override
313304
public boolean parsesArrayValue() {
314305
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;
@@ -55,6 +56,7 @@ private CacheKey() {}
5556
private final List<FieldMapper> indexTimeScriptMappers = new ArrayList<>();
5657
private final Mapping mapping;
5758
private final Set<String> shadowedFields;
59+
private final Set<String> completionFields = new HashSet<>();
5860

5961
/**
6062
* Creates a new {@link MappingLookup} instance by parsing the provided mapping and extracting its field definitions.
@@ -150,6 +152,9 @@ private MappingLookup(Mapping mapping,
150152
if (mapper.hasScript()) {
151153
indexTimeScriptMappers.add(mapper);
152154
}
155+
if (mapper instanceof CompletionFieldMapper) {
156+
completionFields.add(mapper.name());
157+
}
153158
}
154159

155160
for (FieldAliasMapper aliasMapper : aliasMappers) {
@@ -215,6 +220,15 @@ public boolean isShadowed(String field) {
215220
return shadowedFields.contains(field);
216221
}
217222

223+
/**
224+
* Gets the postings format for a particular field
225+
* @param field the field to retrieve a postings format for
226+
* @return the postings format for the field, or {@code null} if the default format should be used
227+
*/
228+
public PostingsFormat getPostingsFormat(String field) {
229+
return completionFields.contains(field) ? CompletionFieldMapper.postingsFormat() : null;
230+
}
231+
218232
void checkLimits(IndexSettings settings) {
219233
checkFieldLimit(settings.getMappingTotalFieldsLimit());
220234
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
@@ -310,7 +310,7 @@ public IndexShard(
310310
assert shardRouting.initializing();
311311
this.shardRouting = shardRouting;
312312
final Settings settings = indexSettings.getSettings();
313-
this.codecService = new CodecService(mapperService, logger);
313+
this.codecService = new CodecService(mapperService);
314314
this.warmer = warmer;
315315
this.similarityService = similarityService;
316316
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
@@ -2942,7 +2942,7 @@ public void testFailStart() throws IOException {
29422942
}
29432943

29442944
public void testSettings() {
2945-
CodecService codecService = new CodecService(null, logger);
2945+
CodecService codecService = new CodecService(null);
29462946
LiveIndexWriterConfig currentIndexWriterConfig = engine.getCurrentIndexWriterConfig();
29472947

29482948
assertEquals(engine.config().getCodec().getName(), codecService.codec(codecName).getName());
@@ -3293,7 +3293,7 @@ public void testRecoverFromForeignTranslog() throws IOException {
32933293
newMergePolicy(),
32943294
config.getAnalyzer(),
32953295
config.getSimilarity(),
3296-
new CodecService(null, logger),
3296+
new CodecService(null),
32973297
config.getEventListener(),
32983298
IndexSearcher.getDefaultQueryCache(),
32993299
IndexSearcher.getDefaultQueryCachingPolicy(),
@@ -6396,7 +6396,7 @@ public void testNotWarmUpSearcherInEngineCtor() throws Exception {
63966396
createTempDir(), config.getTranslogConfig().getIndexSettings(), config.getTranslogConfig().getBigArrays());
63976397
EngineConfig configWithWarmer = new EngineConfig(config.getShardId(), config.getThreadPool(),
63986398
config.getIndexSettings(), warmer, store, config.getMergePolicy(), config.getAnalyzer(),
6399-
config.getSimilarity(), new CodecService(null, logger), config.getEventListener(), config.getQueryCache(),
6399+
config.getSimilarity(), new CodecService(null), config.getEventListener(), config.getQueryCache(),
64006400
config.getQueryCachingPolicy(), translogConfig, config.getFlushMergesAfter(),
64016401
config.getExternalRefreshListener(), config.getInternalRefreshListener(), config.getIndexSort(),
64026402
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;
@@ -34,6 +36,8 @@
3436
import org.elasticsearch.index.analysis.AnalyzerScope;
3537
import org.elasticsearch.index.analysis.IndexAnalyzers;
3638
import org.elasticsearch.index.analysis.NamedAnalyzer;
39+
import org.elasticsearch.index.codec.CodecService;
40+
import org.elasticsearch.index.codec.PerFieldMappingPostingFormatCodec;
3741
import org.hamcrest.FeatureMatcher;
3842
import org.hamcrest.Matcher;
3943
import org.hamcrest.Matchers;
@@ -113,6 +117,15 @@ protected IndexAnalyzers createIndexAnalyzers(IndexSettings indexSettings) {
113117
return new IndexAnalyzers(analyzers, Collections.emptyMap(), Collections.emptyMap());
114118
}
115119

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

118131
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
@@ -4200,7 +4200,7 @@ public void testCloseShardWhileEngineIsWarming() throws Exception {
42004200
};
42014201
EngineConfig configWithWarmer = new EngineConfig(config.getShardId(), config.getThreadPool(),
42024202
config.getIndexSettings(), warmer, config.getStore(), config.getMergePolicy(), config.getAnalyzer(),
4203-
config.getSimilarity(), new CodecService(null, logger), config.getEventListener(), config.getQueryCache(),
4203+
config.getSimilarity(), new CodecService(null), config.getEventListener(), config.getQueryCache(),
42044204
config.getQueryCachingPolicy(), config.getTranslogConfig(), config.getFlushMergesAfter(),
42054205
config.getExternalRefreshListener(), config.getInternalRefreshListener(), config.getIndexSort(),
42064206
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
@@ -125,7 +125,7 @@ public void onFailedEngine(String reason, @Nullable Exception e) {
125125
newMergePolicy(),
126126
iwc.getAnalyzer(),
127127
iwc.getSimilarity(),
128-
new CodecService(null, logger),
128+
new CodecService(null),
129129
eventListener,
130130
IndexSearcher.getDefaultQueryCache(),
131131
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(),

0 commit comments

Comments
 (0)