Add a soft limit for the number of requested doc-value fields #26574
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Author
cbuescher
added
:Search/Search
Member
Seems fine to me, especially for a soft limit. |
cbuescher
force-pushed
the
fix-softLimitDocValueFields
branch
from
697f56d to
e1fb5cd
Compare
jpountz
approved these changes
Sep 11, 2017
Contributor
jpountz
left a comment
jpountz
left a comment
There was a problem hiding this comment.
LGTM. 100 sounds like a good start to me.
Requesting to many docvalue_fields in a search request can potentially be costly because it might incur a per-field per-document seek. This change introduces a soft limit on the number of fields that can be retrieved. The setting can be changed per index using the `index.max_docvalue_fields_search` setting. Relates to elastic#26390
cbuescher
force-pushed
the
fix-softLimitDocValueFields
branch
from
e1fb5cd to
3ad15f1
Compare
Member
Author
nik9000
reviewed
Sep 11, 2017
Member
nik9000
left a comment
nik9000
left a comment
There was a problem hiding this comment.
I left one thing for discussion. Otherwise it LGTM.
| * per search request. The default maximum of 100 is defensive for the reason that retrieving | ||
| * doc values might incur a per-field per-document seek. | ||
| */ | ||
| public static final Setting<Integer> MAX_DOCVALUE_FIELDS_SEARCH_SETTING = |
Member
There was a problem hiding this comment.
I think 0 is a good minimum value.
| int maxAllowedDocvalueFields = context.mapperService().getIndexSettings().getMaxDocvalueFields(); | ||
| if (source.docValueFields().size() > maxAllowedDocvalueFields) { | ||
| throw new IllegalArgumentException( | ||
| "Trying to retrieve too many docvalue_fields. " + "Must be less than or equal to: [" + maxAllowedDocvalueFields |
cbuescher
added a commit
that referenced
this pull request
Sep 13, 2017
Requesting to many docvalue_fields in a search request can potentially be costly because it might incur a per-field per-document seek. This change introduces a soft limit on the number of fields that can be retrieved. The setting can be changed per index using the `index.max_docvalue_fields_search` setting. Relates to #26390
cbuescher
added a commit
that referenced
this pull request
Sep 13, 2017
Requesting to many docvalue_fields in a search request can potentially be costly because it might incur a per-field per-document seek. This change introduces a soft limit on the number of fields that can be retrieved. The setting can be changed per index using the `index.max_docvalue_fields_search` setting. Relates to #26390
cbuescher
added a commit
that referenced
this pull request
Sep 13, 2017
After backporting the related change to the 6.x branches, this test can now also be run in a mixed cluster. Relates to #26574
jasontedor
added a commit
to jpountz/elasticsearch
that referenced
this pull request
Sep 13, 2017
* master: (21 commits) Ensure module is bundled before installing in tests Add boolean similarity to built in similarity types (elastic#26613) [Tests] Remove skip tests in search/30_limits.yml Let search phases override max concurrent requests Add a soft limit for the number of requested doc-value fields (elastic#26574) Support for accessing Azure repositories through a proxy (elastic#23518) Add beta tag to MSI Windows Installer (elastic#26616) Fix Lucene version of 5.6.1. Remove azure deprecated settings (elastic#26099) Handle the 5.6.0 release Allow plugins to validate cluster-state on join (elastic#26595) Remove index mapper dynamic settings (elastic#25734) update AWS SDK for ECS Task IAM support in discovery-ec2 (elastic#26479) Azure repository: Accelerate the listing of files (used in delete snapshot) (elastic#25710) Build: Remove norelease from forbidden patterns (elastic#26592) Fix reference to painless inside expression engine (elastic#26528) Build: Move javadoc linking to root build.gradle (elastic#26529) Test: Remove leftover static bwc test case (elastic#26584) Docs: Remove remaining references to file and native scripts (elastic#26580) Snapshot fallback should consider build.snapshot ...
jasontedor
added a commit
to synhershko/elasticsearch
that referenced
this pull request
Sep 14, 2017
* master: (39 commits) [Docs] Correct typo in removal_of_types.asciidoc (elastic#26646) [Docs] "The the" is a great band, but ... (elastic#26644) Move all repository-azure classes under one single package (elastic#26624) [Docs] Update link in removal_of_types.asciidoc (elastic#26614) Fix percolator highlight sub fetch phase to not highlight query twice (elastic#26622) Refactor bootstrap check results and error messages Add BootstrapContext to expose settings and recovered state to bootstrap checks (elastic#26628) [Tests] Removing skipping tests in search rest tests Initialize checkpoint tracker with allocation ID Move non-core mappers to a module. (elastic#26549) [Docs] Clarify size parameter in Completion Suggester doc (elastic#26617) Add soft limit on allowed number of script fields in request (elastic#26598) Remove MapperService#dynamic. (elastic#26603) Fix incomplete sentences in parent-join docs (elastic#26623) More efficient encoding of range fields. (elastic#26470) Ensure module is bundled before installing in tests Add boolean similarity to built in similarity types (elastic#26613) [Tests] Remove skip tests in search/30_limits.yml Let search phases override max concurrent requests Add a soft limit for the number of requested doc-value fields (elastic#26574) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requesting to many docvalue_fields in a search request can potentially be costly
because it might incur a per-field per-document seek. This change introduces a
soft limit on the number of fields that can be retrieved. The setting can be
changed per index using the
index.max_docvalue_fields_searchsetting.Relates to #26390