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.

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

Add index level setting to unmap fields beyond total fields limit #14939

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Add fingerprint ingest processor ([#13724](https://github.com/opensearch-project/OpenSearch/pull/13724))
- [Remote Store] Rate limiter for remote store low priority uploads ([#14374](https://github.com/opensearch-project/OpenSearch/pull/14374/))
- Apply the date histogram rewrite optimization to range aggregation ([#13865](https://github.com/opensearch-project/OpenSearch/pull/13865))
- Add index level setting `index.mapping.total_fields.unmap_fields_beyond_limit` to unmap fields beyond the total fields limit ([#14939](https://github.com/opensearch-project/OpenSearch/pull/14939))
- [Writable Warm] Add composite directory implementation and integrate it with FileCache ([12782](https://github.com/opensearch-project/OpenSearch/pull/12782))
- [Workload Management] Add QueryGroup schema ([13669](https://github.com/opensearch-project/OpenSearch/pull/13669))
- Add batching supported processor base type AbstractBatchingProcessor ([#14554](https://github.com/opensearch-project/OpenSearch/pull/14554))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
---
"Test unmap the object type fields beyond the total fields limit":
- skip:
version: " - 2.99.99"
reason: "introduced in 3.0.0"
- do:
indices.create:
index: test_index_1
body:
settings:
index.mapping.total_fields.limit: 3
index.mapping.total_fields.unmap_fields_beyond_limit: true

- do:
index:
index: test_index_1
id: 1
body: {
field1: "field1",
field2: [1,2,3],
unmapField: "foo"
}
- do:
get:
index: test_index_1
id: 1
- match: { _source: { field1: "field1", field2: [1,2,3], unmapField: "foo" }}

- do:
indices.get_mapping:
index: test_index_1
- match: {test_index_1.mappings.properties.field1.type: text}
- match: {test_index_1.mappings.properties.field1.fields.keyword.type: keyword}
- match: {test_index_1.mappings.properties.field2.type: long}
- match: {test_index_1.mappings.properties.unmapField: null}

- do:
index:
index: test_index_1
id: 2
body: {
field3: {
"field4": "field4"
},
"field5": 100
}
- do:
get:
index: test_index_1
id: 2
- match: { _source: { field3: { field4: "field4" }, field5: 100 }}

- do:
indices.get_mapping:
index: test_index_1
- match: {test_index_1.mappings.properties.field1.type: text}
- match: {test_index_1.mappings.properties.field1.fields.keyword.type: keyword}
- match: {test_index_1.mappings.properties.field2.type: long}
- match: {test_index_1.mappings.properties.field3: null}
- match: {test_index_1.mappings.properties.field5: null}

- do:
indices.put_settings:
index: test_index_1
body:
index.mapping.total_fields.limit: 100

- do:
indices.get_settings:
index: test_index_1
- match: {test_index_1.settings.index.mapping.total_fields.limit: "100"}

- do:
index:
index: test_index_1
id: 2
body: {
field1: "field1",
field2: [1,2,3],
field3: {
"field4": "field4"
},
"field5": 100
}
- do:
get:
index: test_index_1
id: 2
- match: { _source: { field1: "field1", field2: [1,2,3], field3: { field4: "field4" }, field5: 100 }}

- do:
indices.get_mapping:
index: test_index_1
- match: {test_index_1.mappings.properties.field1.type: text}
- match: {test_index_1.mappings.properties.field1.fields.keyword.type: keyword}
- match: {test_index_1.mappings.properties.field2.type: long}
- match: {test_index_1.mappings.properties.field3.properties.field4.type: text}
- match: {test_index_1.mappings.properties.field3.properties.field4.fields.keyword.type: keyword}
- match: {test_index_1.mappings.properties.field5.type: long}
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
MapperService.INDEX_MAPPING_NESTED_FIELDS_LIMIT_SETTING,
MapperService.INDEX_MAPPING_NESTED_DOCS_LIMIT_SETTING,
MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING,
MapperService.INDEX_MAPPING_TOTAL_FIELDS_UNMAP_FIELDS_BEYONGD_LIMIT_SETTING,
MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING,
MapperService.INDEX_MAPPING_FIELD_NAME_LENGTH_LIMIT_SETTING,
BitsetFilterCache.INDEX_LOAD_RANDOM_ACCESS_FILTERS_EAGERLY_SETTING,
Expand Down
15 changes: 15 additions & 0 deletions server/src/main/java/org/opensearch/index/IndexSettings.java
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_NESTED_DOCS_LIMIT_SETTING;
import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_NESTED_FIELDS_LIMIT_SETTING;
import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING;
import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_TOTAL_FIELDS_UNMAP_FIELDS_BEYONGD_LIMIT_SETTING;
import static org.opensearch.index.store.remote.directory.RemoteSnapshotDirectory.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY_MINIMUM_VERSION;

/**
Expand Down Expand Up @@ -809,6 +810,7 @@
private volatile long mappingNestedFieldsLimit;
private volatile long mappingNestedDocsLimit;
private volatile long mappingTotalFieldsLimit;
private volatile boolean unmapFieldsBeyondTotalFieldsLimit;
private volatile long mappingDepthLimit;
private volatile long mappingFieldNameLengthLimit;

Expand Down Expand Up @@ -997,6 +999,7 @@
mappingNestedFieldsLimit = scopedSettings.get(INDEX_MAPPING_NESTED_FIELDS_LIMIT_SETTING);
mappingNestedDocsLimit = scopedSettings.get(INDEX_MAPPING_NESTED_DOCS_LIMIT_SETTING);
mappingTotalFieldsLimit = scopedSettings.get(INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING);
unmapFieldsBeyondTotalFieldsLimit = scopedSettings.get(INDEX_MAPPING_TOTAL_FIELDS_UNMAP_FIELDS_BEYONGD_LIMIT_SETTING);
mappingDepthLimit = scopedSettings.get(INDEX_MAPPING_DEPTH_LIMIT_SETTING);
mappingFieldNameLengthLimit = scopedSettings.get(INDEX_MAPPING_FIELD_NAME_LENGTH_LIMIT_SETTING);
maxFullFlushMergeWaitTime = scopedSettings.get(INDEX_MERGE_ON_FLUSH_MAX_FULL_FLUSH_MERGE_WAIT_TIME);
Expand Down Expand Up @@ -1115,6 +1118,10 @@
scopedSettings.addSettingsUpdateConsumer(INDEX_MAPPING_NESTED_FIELDS_LIMIT_SETTING, this::setMappingNestedFieldsLimit);
scopedSettings.addSettingsUpdateConsumer(INDEX_MAPPING_NESTED_DOCS_LIMIT_SETTING, this::setMappingNestedDocsLimit);
scopedSettings.addSettingsUpdateConsumer(INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING, this::setMappingTotalFieldsLimit);
scopedSettings.addSettingsUpdateConsumer(
INDEX_MAPPING_TOTAL_FIELDS_UNMAP_FIELDS_BEYONGD_LIMIT_SETTING,
this::setMappingUnmapFieldsBeyondTotalFieldsLimit
);
scopedSettings.addSettingsUpdateConsumer(INDEX_MAPPING_DEPTH_LIMIT_SETTING, this::setMappingDepthLimit);
scopedSettings.addSettingsUpdateConsumer(INDEX_MAPPING_FIELD_NAME_LENGTH_LIMIT_SETTING, this::setMappingFieldNameLengthLimit);
scopedSettings.addSettingsUpdateConsumer(INDEX_MERGE_ON_FLUSH_MAX_FULL_FLUSH_MERGE_WAIT_TIME, this::setMaxFullFlushMergeWaitTime);
Expand Down Expand Up @@ -1861,6 +1868,14 @@
this.mappingTotalFieldsLimit = value;
}

public boolean getUnmapFieldsBeyondTotalFieldsLimit() {
return unmapFieldsBeyondTotalFieldsLimit;
}

private void setMappingUnmapFieldsBeyondTotalFieldsLimit(boolean value) {
this.unmapFieldsBeyondTotalFieldsLimit = value;
}

Check warning on line 1877 in server/src/main/java/org/opensearch/index/IndexSettings.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/IndexSettings.java#L1876-L1877

Added lines #L1876 - L1877 were not covered by tests

public long getMappingDepthLimit() {
return mappingDepthLimit;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,12 @@
throw new StrictDynamicMappingException(dynamic.name().toLowerCase(Locale.ROOT), mapper.fullPath(), currentFieldName);
case TRUE:
case STRICT_ALLOW_TEMPLATES:
// if dynamic is true or strict_allow_templates, we check if we need to unmap the fields beyond the total fields limit
if (checkIfUnmapFieldsBeyondTotalFieldsLimit(context)) {
context.parser().skipChildren();
break;
}

Mapper.Builder builder = findTemplateBuilder(
context,
currentFieldName,
Expand All @@ -573,12 +579,27 @@
// not dynamic, read everything up to end object
context.parser().skipChildren();
}

for (int i = 0; i < parentMapperTuple.v1(); i++) {
context.path().remove();
}
}
}

private static boolean checkIfUnmapFieldsBeyondTotalFieldsLimit(ParseContext context) {
return context.indexSettings().getUnmapFieldsBeyondTotalFieldsLimit()
&& context.docMapper()
.mappers()
.exceedTotalFieldsLimit(context.indexSettings().getMappingTotalFieldsLimit(), context.getDynamicMappers());
}

private static boolean checkUnmapFieldsOrNotIfAddNewField(ParseContext context, Mapper mapper) {
return context.indexSettings().getUnmapFieldsBeyondTotalFieldsLimit()
&& context.docMapper()
.mappers()

Check warning on line 599 in server/src/main/java/org/opensearch/index/mapper/DocumentParser.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/mapper/DocumentParser.java#L598-L599

Added lines #L598 - L599 were not covered by tests
.exceedTotalFieldsLimitIfAddNewField(context.indexSettings().getMappingTotalFieldsLimit(), mapper);
}

private static void parseArray(ParseContext context, ObjectMapper parentMapper, String lastFieldName, String[] paths)
throws IOException {
try {
Expand Down Expand Up @@ -861,12 +882,20 @@
if (dynamic == ObjectMapper.Dynamic.STRICT) {
throw new StrictDynamicMappingException(dynamic.name().toLowerCase(Locale.ROOT), parentMapper.fullPath(), currentFieldName);
}
if (dynamic == ObjectMapper.Dynamic.FALSE) {
// if dynamic is true or strict_allow_templates, and index.mapping.total_fields.unmap_fields_beyond_limit is true,
// then we check if we need to unmap the fields beyond the total fields limit
if (dynamic == ObjectMapper.Dynamic.FALSE || checkIfUnmapFieldsBeyondTotalFieldsLimit(context)) {
return;
}
final Mapper.BuilderContext builderContext = new Mapper.BuilderContext(context.indexSettings().getSettings(), context.path());
final Mapper.Builder<?> builder = createBuilderFromDynamicValue(context, token, currentFieldName, dynamic, parentMapper.fullPath());
Mapper mapper = builder.build(builderContext);

// edge case, if index.mapping.total_fields.unmap_fields_beyond_limit is true,
// then we check if adding a new field will cause the field count to exceed the total fields limit, if so we don't add it
if (checkUnmapFieldsOrNotIfAddNewField(context, mapper)) {
return;

Check warning on line 897 in server/src/main/java/org/opensearch/index/mapper/DocumentParser.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/mapper/DocumentParser.java#L897

Added line #L897 was not covered by tests
}
context.addDynamicMapper(mapper);

parseObjectOrField(context, mapper);
Expand Down Expand Up @@ -956,6 +985,11 @@
throw new StrictDynamicMappingException(dynamic.name().toLowerCase(Locale.ROOT), parent.fullPath(), paths[i]);
case STRICT_ALLOW_TEMPLATES:
case TRUE:
// if dynamic is true or strict_allow_templates, we check if we need to unmap the fields beyond the total fields
// limit
if (checkIfUnmapFieldsBeyondTotalFieldsLimit(context)) {
return new Tuple<>(pathsAdded, parent);

Check warning on line 991 in server/src/main/java/org/opensearch/index/mapper/DocumentParser.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/mapper/DocumentParser.java#L991

Added line #L991 was not covered by tests
}
Mapper.Builder builder = findTemplateBuilder(
context,
paths[i],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,14 @@ public enum MergeReason {
Property.Dynamic,
Property.IndexScope
);
// if set to true, the new detected fields from dynamic mapping which beyond the total fields limit will be unmapped, i.e. will not
// be added to the mapping
public static final Setting<Boolean> INDEX_MAPPING_TOTAL_FIELDS_UNMAP_FIELDS_BEYONGD_LIMIT_SETTING = Setting.boolSetting(
"index.mapping.total_fields.unmap_fields_beyond_limit",
false,
Property.Dynamic,
Property.IndexScope
);
public static final Setting<Long> INDEX_MAPPING_DEPTH_LIMIT_SETTING = Setting.longSetting(
"index.mapping.depth.limit",
20L,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@
private final FieldTypeLookup fieldTypeLookup;
private final int metadataFieldCount;
private final FieldNameAnalyzer indexAnalyzer;
private final int nonMetadataFieldCount;
private int dynamicFieldCount;
private int dynamicObjectFieldCount;

private static void put(Map<String, Analyzer> analyzers, String key, Analyzer value, Analyzer defaultValue) {
if (value == null) {
Expand Down Expand Up @@ -154,6 +157,7 @@
this.fieldMappers = Collections.unmodifiableMap(fieldMappers);
this.indexAnalyzer = new FieldNameAnalyzer(indexAnalyzers);
this.objectMappers = Collections.unmodifiableMap(objects);
this.nonMetadataFieldCount = fieldMappers.size() + objectMappers.size() - metadataFieldCount;
}

/**
Expand Down Expand Up @@ -190,8 +194,49 @@
checkNestedLimit(settings.getMappingNestedFieldsLimit());
}

/**
*
* @param limit the value of the setting index.mapping.total_fields_limit
* @param dynamicMappers mappers of the new fields detected by dynamic mapping
* @return true if adding new fields will cause to exceed the total fields limit
*/
public boolean exceedTotalFieldsLimit(long limit, List<Mapper> dynamicMappers) {
if (nonMetadataFieldCount > limit) {
return true;

Check warning on line 205 in server/src/main/java/org/opensearch/index/mapper/MappingLookup.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/mapper/MappingLookup.java#L205

Added line #L205 was not covered by tests
}

List<ObjectMapper> newObjectMappers = new ArrayList<>();
gaobinlong marked this conversation as resolved.
Show resolved Hide resolved
List<FieldMapper> newFieldMappers = new ArrayList<>();
List<FieldAliasMapper> newFieldAliasMappers = new ArrayList<>();
dynamicMappers.forEach(dynamicMapper -> collect(dynamicMapper, newObjectMappers, newFieldMappers, newFieldAliasMappers));
this.dynamicFieldCount = newFieldMappers.size();
this.dynamicObjectFieldCount = newObjectMappers.size();

return nonMetadataFieldCount + dynamicFieldCount + dynamicObjectFieldCount >= limit;
}

/**
*
* @param limit the value of the setting index.mapping.total_fields_limit
* @param mapper mapper of the new field detected by dynamic mapping
* @return true if adding a new field will cause to exceed the total fields limit
*/
public boolean exceedTotalFieldsLimitIfAddNewField(long limit, Mapper mapper) {
if (nonMetadataFieldCount > limit) {
return true;

Check warning on line 226 in server/src/main/java/org/opensearch/index/mapper/MappingLookup.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/mapper/MappingLookup.java#L226

Added line #L226 was not covered by tests
}

List<ObjectMapper> newObjectMappers = new ArrayList<>();
List<FieldMapper> newFieldMappers = new ArrayList<>();
List<FieldAliasMapper> newFieldAliasMappers = new ArrayList<>();
collect(mapper, newObjectMappers, newFieldMappers, newFieldAliasMappers);

Check warning on line 232 in server/src/main/java/org/opensearch/index/mapper/MappingLookup.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/mapper/MappingLookup.java#L229-L232

Added lines #L229 - L232 were not covered by tests

return nonMetadataFieldCount + dynamicFieldCount + dynamicObjectFieldCount + newObjectMappers.size() + newFieldMappers

Check warning on line 234 in server/src/main/java/org/opensearch/index/mapper/MappingLookup.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/mapper/MappingLookup.java#L234

Added line #L234 was not covered by tests
.size() > limit;
}

private void checkFieldLimit(long limit) {
if (fieldMappers.size() + objectMappers.size() - metadataFieldCount > limit) {
if (nonMetadataFieldCount > limit) {
throw new IllegalArgumentException("Limit of total fields [" + limit + "] has been exceeded");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Function;

import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_TOTAL_FIELDS_UNMAP_FIELDS_BEYONGD_LIMIT_SETTING;
import static org.opensearch.index.store.remote.directory.RemoteSnapshotDirectory.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY_MINIMUM_VERSION;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.core.StringContains.containsString;
Expand Down Expand Up @@ -787,6 +788,25 @@ public void testRemoteStoreExplicitSetting() {
assertTrue(settings.isRemoteStoreEnabled());
}

public void testUnmapFieldsBeyondTotalFieldsLimitSetting() {
IndexMetadata metadata = newIndexMeta(
"index",
Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT).build()
);
IndexSettings settings = new IndexSettings(metadata, Settings.EMPTY);
assertFalse(settings.getUnmapFieldsBeyondTotalFieldsLimit());

metadata = newIndexMeta(
"index",
Settings.builder()
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
.put(INDEX_MAPPING_TOTAL_FIELDS_UNMAP_FIELDS_BEYONGD_LIMIT_SETTING.getKey(), true)
.build()
);
settings = new IndexSettings(metadata, Settings.EMPTY);
assertTrue(settings.getUnmapFieldsBeyondTotalFieldsLimit());
}

public void testRemoteTranslogStoreDefaultSetting() {
IndexMetadata metadata = newIndexMeta(
"index",
Expand Down
Loading
Loading