Skip to content

Avoid OOM on poison documents #73460

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

Open
markharwood opened this issue May 27, 2021 · 6 comments
Open

Avoid OOM on poison documents #73460

markharwood opened this issue May 27, 2021 · 6 comments
Labels
>bug priority:critical A label for assessing bug priority to be used by ES engineers :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch

Comments

@markharwood
Copy link
Contributor

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.

@markharwood markharwood added >bug :Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. :Core/Infra/Resiliency Keep running when everything is ok. Die quickly if things go horribly wrong. labels May 27, 2021
@elasticmachine elasticmachine added Team:Core/Infra Meta label for core/infra team Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. labels May 27, 2021
@elasticmachine
Copy link
Collaborator

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

@elasticmachine
Copy link
Collaborator

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

@williamrandolph
Copy link
Contributor

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.

@williamrandolph williamrandolph added :Search Foundations/Mapping Index mappings, including merging and defining field types and removed :Core/Infra/Resiliency Keep running when everything is ok. Die quickly if things go horribly wrong. :Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. Team:Core/Infra Meta label for core/infra team Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. labels Jun 2, 2021
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Jun 2, 2021
@elasticmachine
Copy link
Collaborator

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

ywelsch added a commit that referenced this issue Jun 7, 2021
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
@ywelsch
Copy link
Contributor

ywelsch commented Jun 7, 2021

#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.

ywelsch added a commit that referenced this issue Jun 7, 2021
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
@javanna javanna added priority:high A label for assessing bug priority to be used by ES engineers priority:critical A label for assessing bug priority to be used by ES engineers and removed priority:high A label for assessing bug priority to be used by ES engineers labels Jun 7, 2024
@javanna javanna added Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch and removed Team:Search Meta label for search team labels Jul 16, 2024
@elasticsearchmachine
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug priority:critical A label for assessing bug priority to be used by ES engineers :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch
Projects
None yet
Development

No branches or pull requests

7 participants