elastic / elasticsearch

Free and Open Source, Distributed, RESTful Search Engine
https://www.elastic.co/products/elasticsearch
Other
887 stars 24.81k forks source link

Avoid OOM on poison documents #73460

Open markharwood opened 3 years ago

markharwood commented 3 years ago

In examining an OOM heap dump from a 7.12 user it was apparent that the culprit was a rogue document that introduced 360k(!) new fields and had a bunch of TextFieldMapper objects held in o.e.index.mapper.ParseContext.InternalParseContext.dynamicMappers that together consumed 1.5GB of RAM of 2GB heap.

I know we have circuit breakers for numbers of fields in mappers but perhaps these objects are allocated during parsing before we test that condition?

Either way seems like we need some added robustness here.

elasticmachine commented 3 years ago

Pinging @elastic/es-distributed (Team:Distributed)

elasticmachine commented 3 years ago

Pinging @elastic/es-core-infra (Team:Core/Infra)

williamrandolph commented 3 years ago

The core-infra team discussed this in our sync today. We don't believe resiliency is the primary problem here.

If we have a configured limit on the number of fields in a mapping, the code for the dynamic mappers should use that limit to avoid allocating too many objects whether or not the memory usage threatens OOM. If the only way to catch this issue were by using a memory circuit breaker, we'd consider that a resilience issue, but there is likely a better way to catch this by improving dynamic mappers.

We think the search team is probably better equipped to handle this issue.

elasticmachine commented 3 years ago

Pinging @elastic/es-search (Team:Search)

ywelsch commented 3 years ago

73713 covers the scenario when a document has many different field names. There are scenarios which the PR does not cover, e.g. if the document has an array with a crazy number of subobjects that all have the same field name. We currently add a dynamic mapping update for all these subobjects fields, and only merge those later into one. This would only be solved by interleaving dynamic mapping creation with parsing, which isn’t easy to do. We could somewhat cover this scenario by also interleaving the index.mapping.nested_objects.limit + some other checks with parsing.

elasticsearchmachine commented 3 months ago

Pinging @elastic/es-search-foundations (Team:Search Foundations)