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

Include runtime fields in total fields count #89251

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 6 additions & 0 deletions docs/changelog/89251.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 89251
summary: Include runtime fields in total fields count
area: Mapping
type: bug
issues:
- 88265
3 changes: 2 additions & 1 deletion docs/reference/mapping/mapping-settings-limit.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ Use the following settings to limit the number of field mappings (created manual

`index.mapping.total_fields.limit`::
The maximum number of fields in an index. Field and object mappings, as well as
field aliases count towards this limit. The default value is `1000`.
field aliases count towards this limit. Mapped runtime fields count towards this
limit as well. The default value is `1000`.
+
[IMPORTANT]
====
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.elasticsearch.action.support.WriteRequest;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.ClusterStateUpdateTask;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.metadata.MappingMetadata;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Randomness;
Expand Down Expand Up @@ -213,6 +214,71 @@ public void onFailure(Exception e) {
}
}

public void testTotalFieldsLimitWithRuntimeFields() {
Settings indexSettings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(), 4)
.build();

String mapping = """
{
"dynamic":"runtime",
"runtime": {
"my_object.rfield1": {
"type": "keyword"
},
"rfield2": {
"type": "keyword"
}
},
"properties": {
"field3" : {
"type": "keyword"
}
}
}
""";

client().admin().indices().prepareCreate("index1").setSettings(indexSettings).setMapping(mapping).get();
ensureGreen("index1");

{
// introduction of a new object with 2 new sub-fields fails
final IndexRequestBuilder indexRequestBuilder = client().prepareIndex("index1")
.setId("1")
.setSource("field3", "value3", "my_object2", Map.of("new_field1", "value1", "new_field2", "value2"));
Exception exc = expectThrows(MapperParsingException.class, () -> indexRequestBuilder.get(TimeValue.timeValueSeconds(10)));
assertThat(exc.getMessage(), Matchers.containsString("failed to parse"));
assertThat(exc.getCause(), instanceOf(IllegalArgumentException.class));
assertThat(
exc.getCause().getMessage(),
Matchers.containsString("Limit of total fields [4] has been exceeded while adding new fields [2]")
);
}

{
// introduction of a new single field succeeds
client().prepareIndex("index1").setId("2").setSource("field3", "value3", "new_field4", 100).get();
}

{
// remove 2 runtime field mappings
assertAcked(client().admin().indices().preparePutMapping("index1").setSource("""
{
"runtime": {
"my_object.rfield1": null,
"rfield2" : null
}
}
""", XContentType.JSON));

// introduction of a new object with 2 new sub-fields succeeds
client().prepareIndex("index1")
.setId("1")
.setSource("field3", "value3", "my_object2", Map.of("new_field1", "value1", "new_field2", "value2"));
}
}

public void testMappingVersionAfterDynamicMappingUpdate() throws Exception {
createIndex("test");
final ClusterService clusterService = internalCluster().clusterService();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,14 @@ final ObjectMapper getDynamicObjectMapper(String name) {

/**
* Add a new runtime field dynamically created while parsing.
* We use the same set for both new indexed and new runtime fields,
* because for dynamic mappings, a new field can be either mapped
* as runtime or indexed, but never both.
*/
public final void addDynamicRuntimeField(RuntimeField runtimeField) {
final void addDynamicRuntimeField(RuntimeField runtimeField) {
if (newFieldsSeen.add(runtimeField.name())) {
Copy link
Member

Choose a reason for hiding this comment

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

A runtime field can have the same name as an indexed field, in which case it will shadow the indexed field at search time. I wondered if it's ok to have the same set for both runtime fields and indexed fields and it does look ok because when it comes to dynamic mappings, when we encounter a new field, either we map it as a runtime field or as an indexed field, and never both, so there are never going to be collisions. Shall we add a comment to clarify that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@javanna Good suggestion, addressed in dbf2022

Indeed for dynamic updates, a field can be mapped either as runtime field or as indexed field, so no problem here.

For PUT _mapping updates, a user can introduce a new runtime field with the same name as existing indexed field. In this case, this new runtime field will be added to the total count, and can cause an exception for total field counts.

mappingLookup.checkFieldLimit(indexSettings().getMappingTotalFieldsLimit(), newFieldsSeen.size());
}
dynamicRuntimeFields.add(runtimeField);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ private CacheKey() {}
/** Full field name to mapper */
private final Map<String, Mapper> fieldMappers;
private final Map<String, ObjectMapper> objectMappers;
private final int runtimeFieldMappersCount;
private final NestedLookup nestedLookup;
private final FieldTypeLookup fieldTypeLookup;
private final FieldTypeLookup indexTimeLookup; // for index-time scripts, a lookup that does not include runtime fields
Expand Down Expand Up @@ -180,6 +181,7 @@ private MappingLookup(
// make all fields into compact+fast immutable maps
this.fieldMappers = Map.copyOf(fieldMappers);
this.objectMappers = Map.copyOf(objects);
this.runtimeFieldMappersCount = runtimeFields.size();
this.indexAnalyzersMap = Map.copyOf(indexAnalyzersMap);
this.completionFields = Set.copyOf(completionFields);
this.indexTimeScriptMappers = List.copyOf(indexTimeScriptMappers);
Expand Down Expand Up @@ -262,7 +264,8 @@ private void checkFieldLimit(long limit) {
}

void checkFieldLimit(long limit, int additionalFieldsToAdd) {
if (fieldMappers.size() + objectMappers.size() + additionalFieldsToAdd - mapping.getSortedMetadataMappers().length > limit) {
if (fieldMappers.size() + objectMappers.size() + runtimeFieldMappersCount + additionalFieldsToAdd - mapping
.getSortedMetadataMappers().length > limit) {
throw new IllegalArgumentException(
"Limit of total fields ["
+ limit
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,13 @@ public void testTotalFieldsLimit() throws Throwable {
() -> merge(mapperService, mapping(b -> b.startObject("newfield").field("type", "long").endObject()))
);
assertTrue(e.getMessage(), e.getMessage().contains("Limit of total fields [" + totalFieldsLimit + "] has been exceeded"));

// adding one more runtime field should trigger exception
e = expectThrows(
IllegalArgumentException.class,
() -> merge(mapperService, runtimeMapping(b -> b.startObject("newfield").field("type", "long").endObject()))
);
assertTrue(e.getMessage(), e.getMessage().contains("Limit of total fields [" + totalFieldsLimit + "] has been exceeded"));
}

private void createMappingSpecifyingNumberOfFields(XContentBuilder b, int numberOfFields) throws IOException {
Expand Down