Skip to content

Commit d81398a

Browse files
authored
Check total field limit at parse time (#73713)
If dynamic mapping updates are enabled, having a crazy number of fields in a document can generate a very large dynamic mapping update that in turn can make a node go OOM even before the mapping update is validated. This commit adds a pre-flight check on the total number of fields allowed in a mapping at document parse time, while the (potentially large) dynamic mapping update is being built. Relates #73460
1 parent 9146606 commit d81398a

File tree

4 files changed

+82
-6
lines changed

4 files changed

+82
-6
lines changed

server/src/internalClusterTest/java/org/elasticsearch/index/mapper/DynamicMappingIT.java

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import java.util.concurrent.CountDownLatch;
4141
import java.util.concurrent.atomic.AtomicReference;
4242

43+
import static org.elasticsearch.index.mapper.MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING;
4344
import static org.elasticsearch.index.mapper.MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING;
4445
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
4546
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits;
@@ -131,9 +132,11 @@ public void run() {
131132
}
132133

133134
public void testPreflightCheckAvoidsMaster() throws InterruptedException {
134-
createIndex("index", Settings.builder().put(INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(), 2).build());
135+
// can't use INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING as a check here, as that is already checked at parse time,
136+
// see testTotalFieldsLimitForDynamicMappingsUpdateCheckedAtDocumentParseTime
137+
createIndex("index", Settings.builder().put(INDEX_MAPPING_DEPTH_LIMIT_SETTING.getKey(), 2).build());
135138
ensureGreen("index");
136-
client().prepareIndex("index").setId("1").setSource("field1", "value1").get();
139+
client().prepareIndex("index").setId("1").setSource("field1", Map.of("field2", "value1")).get();
137140

138141
final CountDownLatch masterBlockedLatch = new CountDownLatch(1);
139142
final CountDownLatch indexingCompletedLatch = new CountDownLatch(1);
@@ -154,11 +157,49 @@ public void onFailure(String source, Exception e) {
154157
});
155158

156159
masterBlockedLatch.await();
157-
final IndexRequestBuilder indexRequestBuilder = client().prepareIndex("index").setId("2").setSource("field2", "value2");
160+
final IndexRequestBuilder indexRequestBuilder = client().prepareIndex("index").setId("2").setSource("field1",
161+
Map.of("field3", Map.of("field4", "value2")));
158162
try {
159163
assertThat(
160164
expectThrows(IllegalArgumentException.class, () -> indexRequestBuilder.get(TimeValue.timeValueSeconds(10))).getMessage(),
161-
Matchers.containsString("Limit of total fields [2] has been exceeded"));
165+
Matchers.containsString("Limit of mapping depth [2] has been exceeded due to object field [field1.field3]"));
166+
} finally {
167+
indexingCompletedLatch.countDown();
168+
}
169+
}
170+
171+
public void testTotalFieldsLimitForDynamicMappingsUpdateCheckedAtDocumentParseTime() throws InterruptedException {
172+
createIndex("index", Settings.builder().put(INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(), 2).build());
173+
ensureGreen("index");
174+
client().prepareIndex("index").setId("1").setSource("field1", "value1").get();
175+
176+
final CountDownLatch masterBlockedLatch = new CountDownLatch(1);
177+
final CountDownLatch indexingCompletedLatch = new CountDownLatch(1);
178+
179+
internalCluster().getInstance(ClusterService.class, internalCluster().getMasterName()).submitStateUpdateTask("block-state-updates",
180+
new ClusterStateUpdateTask() {
181+
@Override
182+
public ClusterState execute(ClusterState currentState) throws Exception {
183+
masterBlockedLatch.countDown();
184+
indexingCompletedLatch.await();
185+
return currentState;
186+
}
187+
188+
@Override
189+
public void onFailure(String source, Exception e) {
190+
throw new AssertionError("unexpected", e);
191+
}
192+
});
193+
194+
masterBlockedLatch.await();
195+
final IndexRequestBuilder indexRequestBuilder = client().prepareIndex("index").setId("2").setSource("field2", "value2");
196+
try {
197+
Exception e = expectThrows(MapperParsingException.class, () -> indexRequestBuilder.get(TimeValue.timeValueSeconds(10)));
198+
assertThat(e.getMessage(),
199+
Matchers.containsString("failed to parse"));
200+
assertThat(e.getCause(), instanceOf(IllegalArgumentException.class));
201+
assertThat(e.getCause().getMessage(),
202+
Matchers.containsString("Limit of total fields [2] has been exceeded while adding new fields [1]"));
162203
} finally {
163204
indexingCompletedLatch.countDown();
164205
}

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,13 @@ void checkLimits(IndexSettings settings) {
207207
}
208208

209209
private void checkFieldLimit(long limit) {
210-
if (fieldMappers.size() + objectMappers.size() - mapping.getSortedMetadataMappers().length > limit) {
211-
throw new IllegalArgumentException("Limit of total fields [" + limit + "] has been exceeded");
210+
checkFieldLimit(limit, 0);
211+
}
212+
213+
void checkFieldLimit(long limit, int additionalFieldsToAdd) {
214+
if (fieldMappers.size() + objectMappers.size() + additionalFieldsToAdd - mapping.getSortedMetadataMappers().length > limit) {
215+
throw new IllegalArgumentException("Limit of total fields [" + limit + "] has been exceeded" +
216+
(additionalFieldsToAdd > 0 ? " while adding new fields [" + additionalFieldsToAdd + "]" : ""));
212217
}
213218
}
214219

server/src/main/java/org/elasticsearch/index/mapper/ParseContext.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,7 @@ public static class InternalParseContext extends ParseContext {
306306
private final SourceToParse sourceToParse;
307307
private final long maxAllowedNumNestedDocs;
308308
private final List<Mapper> dynamicMappers = new ArrayList<>();
309+
private final Set<String> newFieldsSeen = new HashSet<>();
309310
private final Map<String, ObjectMapper> dynamicObjectMappers = new HashMap<>();
310311
private final List<RuntimeField> dynamicRuntimeFields = new ArrayList<>();
311312
private final Set<String> ignoredFields = new HashSet<>();
@@ -427,6 +428,14 @@ public void seqID(SeqNoFieldMapper.SequenceIDFields seqID) {
427428

428429
@Override
429430
public void addDynamicMapper(Mapper mapper) {
431+
// eagerly check field name limit here to avoid OOM errors
432+
// only check fields that are not already mapped or tracked in order to avoid hitting field limit too early via double-counting
433+
// note that existing fields can also receive dynamic mapping updates (e.g. constant_keyword to fix the value)
434+
if (mappingLookup.getMapper(mapper.name()) == null &&
435+
mappingLookup.objectMappers().containsKey(mapper.name()) == false &&
436+
newFieldsSeen.add(mapper.name())) {
437+
mappingLookup.checkFieldLimit(indexSettings.getMappingTotalFieldsLimit(), newFieldsSeen.size());
438+
}
430439
if (mapper instanceof ObjectMapper) {
431440
dynamicObjectMappers.put(mapper.name(), (ObjectMapper)mapper);
432441
}

x-pack/plugin/mapper-constant-keyword/src/internalClusterTest/java/org/elasticsearch/xpack/constantkeyword/mapper/ConstantKeywordFieldMapperTests.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.apache.lucene.search.Query;
1212
import org.elasticsearch.common.Strings;
1313
import org.elasticsearch.common.compress.CompressedXContent;
14+
import org.elasticsearch.common.settings.Settings;
1415
import org.elasticsearch.common.xcontent.XContentBuilder;
1516
import org.elasticsearch.index.mapper.DocumentMapper;
1617
import org.elasticsearch.index.mapper.MappedFieldType;
@@ -28,6 +29,7 @@
2829
import java.util.Collection;
2930
import java.util.List;
3031

32+
import static org.elasticsearch.index.mapper.MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING;
3133
import static org.hamcrest.Matchers.instanceOf;
3234

3335
public class ConstantKeywordFieldMapperTests extends MapperTestCase {
@@ -102,6 +104,25 @@ public void testDynamicValue() throws Exception {
102104
assertNull(doc.dynamicMappingsUpdate());
103105
}
104106

107+
public void testDynamicValueFieldLimit() throws Exception {
108+
MapperService mapperService = createMapperService(
109+
Settings.builder().put(INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(), 1).build(),
110+
fieldMapping(b -> b.field("type", "constant_keyword")));
111+
112+
ParsedDocument doc = mapperService.documentMapper().parse(source(b -> b.field("field", "foo")));
113+
assertNull(doc.rootDoc().getField("field"));
114+
assertNotNull(doc.dynamicMappingsUpdate());
115+
116+
CompressedXContent mappingUpdate = new CompressedXContent(Strings.toString(doc.dynamicMappingsUpdate()));
117+
DocumentMapper updatedMapper = mapperService.merge("_doc", mappingUpdate, MergeReason.MAPPING_UPDATE);
118+
String expectedMapping = Strings.toString(fieldMapping(b -> b.field("type", "constant_keyword").field("value", "foo")));
119+
assertEquals(expectedMapping, updatedMapper.mappingSource().toString());
120+
121+
doc = updatedMapper.parse(source(b -> b.field("field", "foo")));
122+
assertNull(doc.rootDoc().getField("field"));
123+
assertNull(doc.dynamicMappingsUpdate());
124+
}
125+
105126
public void testBadValues() {
106127
{
107128
MapperParsingException e = expectThrows(MapperParsingException.class, () -> createMapperService(fieldMapping(b -> {

0 commit comments

Comments
 (0)