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

Index settings object is parsed on every document update #57395

Closed
itiyamas opened this issue May 31, 2020 · 5 comments · Fixed by #57492
Closed

Index settings object is parsed on every document update #57395

itiyamas opened this issue May 31, 2020 · 5 comments · Fixed by #57492
Labels
>enhancement :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch

Comments

@itiyamas
Copy link

itiyamas commented May 31, 2020

Parsing index settings for every doc update is inefficient and redundant. Instead of parsing the settings object to get the maxAllowedNumNestedDocs, we could just get the cached value and change it via settings applier. Else, we could just get this once per shard bulk request.
I saw close to 2% improvement in indexing throughput of nyc_taxis just by changing this on ES version 7.4.

@itiyamas itiyamas added >enhancement needs:triage Requires assignment of a team area label labels May 31, 2020
@dnhatn dnhatn added the :Search Foundations/Mapping Index mappings, including merging and defining field types label May 31, 2020
@elasticmachine
Copy link
Collaborator

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

@elasticmachine elasticmachine added the Team:Search Meta label for search team label May 31, 2020
@dnhatn dnhatn removed Team:Search Meta label for search team needs:triage Requires assignment of a team area label labels May 31, 2020
@jasontedor
Copy link
Member

It wouldn't feel great to me to treat this setting special. I'd prefer that we do something more general like parse once and cache the parsed and validated value internal to the Settings object, so that the next time we get the setting value we can use the internal cache and skip parsing and validation there. That will involve some boxing for settings of primitive type, but I suspect that will be okay, and there's ways around it if it proves to be a problem. What do you think of this idea @rjernst?

@rjernst
Copy link
Member

rjernst commented Jun 1, 2020

I wonder why this isn't parsed in the IndexSettings ctor like other index settings? It seems like this should be a member there, then no parsing is necessary here.

@itiyamas
Copy link
Author

itiyamas commented Jun 1, 2020

Agreed, a new member should be added that will be updated via settings consumer.

rjernst added a commit to rjernst/elasticsearch that referenced this issue Jun 1, 2020
There are several mapping settings that are currently re-parsed every
time they are read. This can be quite frequent, for example within every
document ingestion. This commit moves the parsed versions of these
mapping settings to be stored in IndexSettings, just as other index settings
are already.

closes elastic#57395
rjernst added a commit that referenced this issue Jun 1, 2020
There are several mapping settings that are currently re-parsed every
time they are read. This can be quite frequent, for example within every
document ingestion. This commit moves the parsed versions of these
mapping settings to be stored in IndexSettings, just as other index settings
are already.

closes #57395
rjernst added a commit that referenced this issue Jun 1, 2020
There are several mapping settings that are currently re-parsed every
time they are read. This can be quite frequent, for example within every
document ingestion. This commit moves the parsed versions of these
mapping settings to be stored in IndexSettings, just as other index settings
are already.

closes #57395
@jasontedor
Copy link
Member

The problem that I have with this approach is that it doesn’t work for index settings registered in X-Pack.

@javanna javanna added the Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch label Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :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

Successfully merging a pull request may close this issue.

6 participants