Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Choose postings format from FieldMapper instead of MappedFieldType #77234

Merged
merged 4 commits into from
Sep 3, 2021
Merged

Choose postings format from FieldMapper instead of MappedFieldType #77234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2c95b63
Per-field postings formats based on FieldMapper
romseygeek Sep 3, 2021
c3a1981
Merge branch 'master' into mapper/postings-format
elasticmachine Sep 3, 2021
dafb3f0
postings format shouldn't be pluggable on fieldmapper
romseygeek Sep 3, 2021
e4988e1
Merge remote-tracking branch 'romseygeek/mapper/postings-format' into…
romseygeek Sep 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions ...er/src/internalClusterTest/java/org/elasticsearch/indices/IndexingMemoryControllerIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
*/
package org.elasticsearch.indices;

import org.apache.logging.log4j.LogManager;
import org.elasticsearch.action.admin.indices.forcemerge.ForceMergeResponse;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.CollectionUtils;
Expand Down Expand Up @@ -52,7 +51,7 @@ EngineConfig engineConfigWithLargerIndexingMemory(EngineConfig config) {
IndexSettings indexSettings = new IndexSettings(config.getIndexSettings().getIndexMetadata(), settings);
return new EngineConfig(config.getShardId(), config.getThreadPool(),
indexSettings, config.getWarmer(), config.getStore(), config.getMergePolicy(), config.getAnalyzer(),
config.getSimilarity(), new CodecService(null, LogManager.getLogger(IndexingMemoryControllerIT.class)),
config.getSimilarity(), new CodecService(null),
config.getEventListener(), config.getQueryCache(),
config.getQueryCachingPolicy(), config.getTranslogConfig(), config.getFlushMergesAfter(),
config.getExternalRefreshListener(), config.getInternalRefreshListener(), config.getIndexSort(),
Expand Down
7 changes: 3 additions & 4 deletions server/src/main/java/org/elasticsearch/index/codec/CodecService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

package org.elasticsearch.index.codec;

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

public CodecService(@Nullable MapperService mapperService, Logger logger) {
public CodecService(@Nullable MapperService mapperService) {
final var codecs = new HashMap<String, Codec>();
if (mapperService == null) {
codecs.put(DEFAULT_CODEC, new Lucene87Codec());
codecs.put(BEST_COMPRESSION_CODEC, new Lucene87Codec(Mode.BEST_COMPRESSION));
} else {
codecs.put(DEFAULT_CODEC,
new PerFieldMappingPostingFormatCodec(Mode.BEST_SPEED, mapperService, logger));
new PerFieldMappingPostingFormatCodec(Mode.BEST_SPEED, mapperService));
codecs.put(BEST_COMPRESSION_CODEC,
new PerFieldMappingPostingFormatCodec(Mode.BEST_COMPRESSION, mapperService, logger));
new PerFieldMappingPostingFormatCodec(Mode.BEST_COMPRESSION, mapperService));
}
codecs.put(LUCENE_DEFAULT_CODEC, Codec.getDefault());
for (String codec : Codec.availableCodecs()) {
Expand Down
18 changes: 6 additions & 12 deletions server/src/main/java/org/elasticsearch/index/codec/PerFieldMappingPostingFormatCodec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,12 @@

package org.elasticsearch.index.codec;

import org.apache.logging.log4j.Logger;
import org.apache.lucene.codecs.Codec;
import org.apache.lucene.codecs.DocValuesFormat;
import org.apache.lucene.codecs.PostingsFormat;
import org.apache.lucene.codecs.lucene80.Lucene80DocValuesFormat;
import org.apache.lucene.codecs.lucene87.Lucene87Codec;
import org.elasticsearch.common.lucene.Lucene;
import org.elasticsearch.index.mapper.CompletionFieldMapper;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.MapperService;

/**
Expand All @@ -28,7 +25,7 @@
* configured for a specific field the default postings format is used.
*/
public class PerFieldMappingPostingFormatCodec extends Lucene87Codec {
private final Logger logger;

private final MapperService mapperService;
// Always enable compression on binary doc values
private final DocValuesFormat docValuesFormat = new Lucene80DocValuesFormat(Lucene80DocValuesFormat.Mode.BEST_COMPRESSION);
Expand All @@ -38,21 +35,18 @@ public class PerFieldMappingPostingFormatCodec extends Lucene87Codec {
"PerFieldMappingPostingFormatCodec must subclass the latest " + "lucene codec: " + Lucene.LATEST_CODEC;
}

public PerFieldMappingPostingFormatCodec(Mode compressionMode, MapperService mapperService, Logger logger) {
public PerFieldMappingPostingFormatCodec(Mode compressionMode, MapperService mapperService) {
super(compressionMode);
this.mapperService = mapperService;
this.logger = logger;
}

@Override
public PostingsFormat getPostingsFormatForField(String field) {
final MappedFieldType fieldType = mapperService.fieldType(field);
if (fieldType == null) {
logger.warn("no index mapper found for field: [{}] returning default postings format", field);
} else if (fieldType instanceof CompletionFieldMapper.CompletionFieldType) {
return CompletionFieldMapper.CompletionFieldType.postingsFormat();
PostingsFormat format = mapperService.mappingLookup().getPostingsFormat(field);
if (format == null) {
return super.getPostingsFormatForField(field);
}
return super.getPostingsFormatForField(field);
return format;
}

@Override
Expand Down
17 changes: 4 additions & 13 deletions server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import org.apache.lucene.document.FieldType;
import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.suggest.document.Completion84PostingsFormat;
import org.apache.lucene.search.suggest.document.CompletionAnalyzer;
import org.apache.lucene.search.suggest.document.CompletionQuery;
import org.apache.lucene.search.suggest.document.FuzzyCompletionQuery;
Expand Down Expand Up @@ -209,8 +208,6 @@ private void checkCompletionContextsLimit() {

public static final class CompletionFieldType extends TermBasedFieldType {

private static PostingsFormat postingsFormat;

private ContextMappings contextMappings = null;

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

/**
* @return postings format to use for this field-type
*/
public static synchronized PostingsFormat postingsFormat() {
if (postingsFormat == null) {
postingsFormat = new Completion84PostingsFormat();
}
return postingsFormat;
}

/**
* Completion prefix query
*/
Expand Down Expand Up @@ -313,6 +300,10 @@ public CompletionFieldType fieldType() {
return (CompletionFieldType) super.fieldType();
}

static PostingsFormat postingsFormat() {
return PostingsFormat.forName("Completion84");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: PostingsFormat.forName() does the same lazy-instantiation that the previous impl did

}

@Override
public boolean parsesArrayValue() {
return true;
Expand Down
14 changes: 14 additions & 0 deletions server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

package org.elasticsearch.index.mapper;

import org.apache.lucene.codecs.PostingsFormat;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.analysis.IndexAnalyzers;
import org.elasticsearch.index.analysis.NamedAnalyzer;
Expand Down Expand Up @@ -53,6 +54,7 @@ private CacheKey() {}
private final List<FieldMapper> indexTimeScriptMappers = new ArrayList<>();
private final Mapping mapping;
private final Set<String> shadowedFields;
private final Set<String> completionFields = new HashSet<>();

/**
* Creates a new {@link MappingLookup} instance by parsing the provided mapping and extracting its field definitions.
Expand Down Expand Up @@ -148,6 +150,9 @@ private MappingLookup(Mapping mapping,
if (mapper.hasScript()) {
indexTimeScriptMappers.add(mapper);
}
if (mapper instanceof CompletionFieldMapper) {
completionFields.add(mapper.name());
}
}

for (FieldAliasMapper aliasMapper : aliasMappers) {
Expand Down Expand Up @@ -213,6 +218,15 @@ public boolean isShadowed(String field) {
return shadowedFields.contains(field);
}

/**
* Gets the postings format for a particular field
* @param field the field to retrieve a postings format for
* @return the postings format for the field, or {@code null} if the default format should be used
*/
public PostingsFormat getPostingsFormat(String field) {
return completionFields.contains(field) ? CompletionFieldMapper.postingsFormat() : null;
}

void checkLimits(IndexSettings settings) {
checkFieldLimit(settings.getMappingTotalFieldsLimit());
checkObjectDepthLimit(settings.getMappingDepthLimit());
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/index/shard/IndexShard.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ public IndexShard(
assert shardRouting.initializing();
this.shardRouting = shardRouting;
final Settings settings = indexSettings.getSettings();
this.codecService = new CodecService(mapperService, logger);
this.codecService = new CodecService(mapperService);
this.warmer = warmer;
this.similarityService = similarityService;
Objects.requireNonNull(store, "Store must be provided to the index shard");
Expand Down
3 changes: 1 addition & 2 deletions server/src/test/java/org/elasticsearch/index/codec/CodecTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

package org.elasticsearch.index.codec;

import org.apache.logging.log4j.LogManager;
import org.apache.lucene.codecs.Codec;
import org.apache.lucene.codecs.lucene80.Lucene80DocValuesFormat;
import org.apache.lucene.codecs.lucene87.Lucene87Codec;
Expand Down Expand Up @@ -110,7 +109,7 @@ private CodecService createCodecService() throws IOException {
MapperPlugin.NOOP_FIELD_FILTER);
MapperService service = new MapperService(settings, indexAnalyzers, xContentRegistry(), similarityService, mapperRegistry,
() -> null, () -> false, ScriptCompiler.NONE);
return new CodecService(service, LogManager.getLogger("test"));
return new CodecService(service);
}

}
6 changes: 3 additions & 3 deletions server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2603,7 +2603,7 @@ public void testFailStart() throws IOException {
}

public void testSettings() {
CodecService codecService = new CodecService(null, logger);
CodecService codecService = new CodecService(null);
LiveIndexWriterConfig currentIndexWriterConfig = engine.getCurrentIndexWriterConfig();

assertEquals(engine.config().getCodec().getName(), codecService.codec(codecName).getName());
Expand Down Expand Up @@ -2935,7 +2935,7 @@ public void testRecoverFromForeignTranslog() throws IOException {
newMergePolicy(),
config.getAnalyzer(),
config.getSimilarity(),
new CodecService(null, logger),
new CodecService(null),
config.getEventListener(),
IndexSearcher.getDefaultQueryCache(),
IndexSearcher.getDefaultQueryCachingPolicy(),
Expand Down Expand Up @@ -6017,7 +6017,7 @@ public void testNotWarmUpSearcherInEngineCtor() throws Exception {
createTempDir(), config.getTranslogConfig().getIndexSettings(), config.getTranslogConfig().getBigArrays());
EngineConfig configWithWarmer = new EngineConfig(config.getShardId(), config.getThreadPool(),
config.getIndexSettings(), warmer, store, config.getMergePolicy(), config.getAnalyzer(),
config.getSimilarity(), new CodecService(null, logger), config.getEventListener(), config.getQueryCache(),
config.getSimilarity(), new CodecService(null), config.getEventListener(), config.getQueryCache(),
config.getQueryCachingPolicy(), translogConfig, config.getFlushMergesAfter(),
config.getExternalRefreshListener(), config.getInternalRefreshListener(), config.getIndexSort(),
config.getCircuitBreakerService(), config.getGlobalCheckpointSupplier(), config.retentionLeasesSupplier(),
Expand Down
13 changes: 13 additions & 0 deletions server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@
import org.apache.lucene.analysis.TokenStream;
import org.apache.lucene.analysis.core.SimpleAnalyzer;
import org.apache.lucene.analysis.standard.StandardAnalyzer;
import org.apache.lucene.codecs.Codec;
import org.apache.lucene.document.SortedSetDocValuesField;
import org.apache.lucene.index.IndexableField;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.suggest.document.Completion84PostingsFormat;
import org.apache.lucene.search.suggest.document.CompletionAnalyzer;
import org.apache.lucene.search.suggest.document.ContextSuggestField;
import org.apache.lucene.search.suggest.document.FuzzyCompletionQuery;
Expand All @@ -35,6 +37,8 @@
import org.elasticsearch.index.analysis.AnalyzerScope;
import org.elasticsearch.index.analysis.IndexAnalyzers;
import org.elasticsearch.index.analysis.NamedAnalyzer;
import org.elasticsearch.index.codec.CodecService;
import org.elasticsearch.index.codec.PerFieldMappingPostingFormatCodec;
import org.hamcrest.FeatureMatcher;
import org.hamcrest.Matcher;
import org.hamcrest.Matchers;
Expand Down Expand Up @@ -114,6 +118,15 @@ protected IndexAnalyzers createIndexAnalyzers(IndexSettings indexSettings) {
);
}

public void testPostingsFormat() throws IOException {
MapperService mapperService = createMapperService(fieldMapping(this::minimalMapping));
CodecService codecService = new CodecService(mapperService);
Codec codec = codecService.codec("default");
assertThat(codec, instanceOf(PerFieldMappingPostingFormatCodec.class));
PerFieldMappingPostingFormatCodec perFieldCodec = (PerFieldMappingPostingFormatCodec) codec;
assertThat(perFieldCodec.getPostingsFormatForField("field"), instanceOf(Completion84PostingsFormat.class));
}

public void testDefaultConfiguration() throws IOException {

DocumentMapper defaultMapper = createDocumentMapper(fieldMapping(this::minimalMapping));
Expand Down
2 changes: 1 addition & 1 deletion server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3924,7 +3924,7 @@ public void testCloseShardWhileEngineIsWarming() throws Exception {
};
EngineConfig configWithWarmer = new EngineConfig(config.getShardId(), config.getThreadPool(),
config.getIndexSettings(), warmer, config.getStore(), config.getMergePolicy(), config.getAnalyzer(),
config.getSimilarity(), new CodecService(null, logger), config.getEventListener(), config.getQueryCache(),
config.getSimilarity(), new CodecService(null), config.getEventListener(), config.getQueryCache(),
config.getQueryCachingPolicy(), config.getTranslogConfig(), config.getFlushMergesAfter(),
config.getExternalRefreshListener(), config.getInternalRefreshListener(), config.getIndexSort(),
config.getCircuitBreakerService(), config.getGlobalCheckpointSupplier(), config.retentionLeasesSupplier(),
Expand Down
2 changes: 1 addition & 1 deletion server/src/test/java/org/elasticsearch/index/shard/RefreshListenersTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public void onFailedEngine(String reason, @Nullable Exception e) {
newMergePolicy(),
iwc.getAnalyzer(),
iwc.getSimilarity(),
new CodecService(null, logger),
new CodecService(null),
eventListener,
IndexSearcher.getDefaultQueryCache(),
IndexSearcher.getDefaultQueryCachingPolicy(),
Expand Down
2 changes: 1 addition & 1 deletion server/src/test/java/org/elasticsearch/indices/IndexingMemoryControllerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ EngineConfig configWithRefreshListener(EngineConfig config, ReferenceManager.Ref
internalRefreshListener.add(listener);
return new EngineConfig(config.getShardId(), config.getThreadPool(),
config.getIndexSettings(), config.getWarmer(), config.getStore(), config.getMergePolicy(), config.getAnalyzer(),
config.getSimilarity(), new CodecService(null, logger), config.getEventListener(), config.getQueryCache(),
config.getSimilarity(), new CodecService(null), config.getEventListener(), config.getQueryCache(),
config.getQueryCachingPolicy(), config.getTranslogConfig(), config.getFlushMergesAfter(),
config.getExternalRefreshListener(), internalRefreshListener, config.getIndexSort(),
config.getCircuitBreakerService(), config.getGlobalCheckpointSupplier(), config.retentionLeasesSupplier(),
Expand Down
12 changes: 6 additions & 6 deletions test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ protected Settings indexSettings() {
public void setUp() throws Exception {
super.setUp();
primaryTerm.set(randomLongBetween(1, Long.MAX_VALUE));
CodecService codecService = new CodecService(null, logger);
CodecService codecService = new CodecService(null);
String name = Codec.getDefault().getName();
if (Arrays.asList(codecService.availableCodecs()).contains(name)) {
// some codecs are read only so we only take the ones that we have in the service and randomly
Expand Down Expand Up @@ -234,7 +234,7 @@ public void setUp() throws Exception {
public EngineConfig copy(EngineConfig config, LongSupplier globalCheckpointSupplier) {
return new EngineConfig(config.getShardId(), config.getThreadPool(), config.getIndexSettings(),
config.getWarmer(), config.getStore(), config.getMergePolicy(), config.getAnalyzer(), config.getSimilarity(),
new CodecService(null, logger), config.getEventListener(), config.getQueryCache(), config.getQueryCachingPolicy(),
new CodecService(null), config.getEventListener(), config.getQueryCache(), config.getQueryCachingPolicy(),
config.getTranslogConfig(), config.getFlushMergesAfter(),
config.getExternalRefreshListener(), Collections.emptyList(), config.getIndexSort(),
config.getCircuitBreakerService(), globalCheckpointSupplier, config.retentionLeasesSupplier(),
Expand All @@ -244,7 +244,7 @@ public EngineConfig copy(EngineConfig config, LongSupplier globalCheckpointSuppl
public EngineConfig copy(EngineConfig config, Analyzer analyzer) {
return new EngineConfig(config.getShardId(), config.getThreadPool(), config.getIndexSettings(),
config.getWarmer(), config.getStore(), config.getMergePolicy(), analyzer, config.getSimilarity(),
new CodecService(null, logger), config.getEventListener(), config.getQueryCache(), config.getQueryCachingPolicy(),
new CodecService(null), config.getEventListener(), config.getQueryCache(), config.getQueryCachingPolicy(),
config.getTranslogConfig(), config.getFlushMergesAfter(),
config.getExternalRefreshListener(), Collections.emptyList(), config.getIndexSort(),
config.getCircuitBreakerService(), config.getGlobalCheckpointSupplier(), config.retentionLeasesSupplier(),
Expand All @@ -254,7 +254,7 @@ public EngineConfig copy(EngineConfig config, Analyzer analyzer) {
public EngineConfig copy(EngineConfig config, MergePolicy mergePolicy) {
return new EngineConfig(config.getShardId(), config.getThreadPool(), config.getIndexSettings(),
config.getWarmer(), config.getStore(), mergePolicy, config.getAnalyzer(), config.getSimilarity(),
new CodecService(null, logger), config.getEventListener(), config.getQueryCache(), config.getQueryCachingPolicy(),
new CodecService(null), config.getEventListener(), config.getQueryCache(), config.getQueryCachingPolicy(),
config.getTranslogConfig(), config.getFlushMergesAfter(),
config.getExternalRefreshListener(), Collections.emptyList(), config.getIndexSort(),
config.getCircuitBreakerService(), config.getGlobalCheckpointSupplier(), config.retentionLeasesSupplier(),
Expand Down Expand Up @@ -656,7 +656,7 @@ public EngineConfig config(
mergePolicy,
iwc.getAnalyzer(),
iwc.getSimilarity(),
new CodecService(null, logger),
new CodecService(null),
eventListener,
IndexSearcher.getDefaultQueryCache(),
IndexSearcher.getDefaultQueryCachingPolicy(),
Expand All @@ -679,7 +679,7 @@ protected EngineConfig config(EngineConfig config, Store store, Path translogPat
TranslogConfig translogConfig = new TranslogConfig(shardId, translogPath, indexSettings, BigArrays.NON_RECYCLING_INSTANCE);
return new EngineConfig(config.getShardId(), config.getThreadPool(),
indexSettings, config.getWarmer(), store, config.getMergePolicy(), config.getAnalyzer(), config.getSimilarity(),
new CodecService(null, logger), config.getEventListener(), config.getQueryCache(), config.getQueryCachingPolicy(),
new CodecService(null), config.getEventListener(), config.getQueryCache(), config.getQueryCachingPolicy(),
translogConfig, config.getFlushMergesAfter(), config.getExternalRefreshListener(),
config.getInternalRefreshListener(), config.getIndexSort(), config.getCircuitBreakerService(),
config.getGlobalCheckpointSupplier(), config.retentionLeasesSupplier(),
Expand Down
Loading