Skip to content

Cleanup obsolete hand-coded XContent duplicate checks #22253

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.

Sign up for GitHub

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

Jump to bottom
Closed
danielmitterdorfer opened this issue Dec 19, 2016 · 0 comments
Closed

Cleanup obsolete hand-coded XContent duplicate checks #22253

danielmitterdorfer opened this issue Dec 19, 2016 · 0 comments
Labels
blocker :Core/Infra/REST API REST infrastructure and utilities >enhancement v7.0.0-beta1

Comments

@danielmitterdorfer
Copy link
Member

With #22073 and #22225 we enabled strict duplicate checks for XContent. Although our benchmarks only showed a negligible performance drop of 1 - 2% (see #19614), Elasticsearch 6.0 still allows to disable the strict duplicate checks with an undocumented flag. Hence, we also needed to keep the existing hand-coded duplicate checks around in case Elasticsearch is started with disabled XContent parser duplicate checks.

In Elasticsearch 7.0, we should remove the undocumented fallback to disable strict duplicate checks and also remove our hand-coded duplicate checks.
@jpountz jpountz added help wanted adoptme and removed help wanted adoptme labels Dec 19, 2016
@nik9000 nik9000 added the help wanted adoptme label Mar 12, 2018
danielmitterdorfer added a commit to danielmitterdorfer/elasticsearch that referenced this issue Oct 18, 2018
@danielmitterdorfer
Remove hand-coded XContent duplicate checks
8892675 
With this commit we cleanup hand-coded duplicate checks in XContent
parsing. They were necessary previously but since we reconfigured the
underlying parser in elastic#22073 and elastic#22225, these checks are obsolete and
were also ineffective unless an undocumented system property has been
set. As we also remove this escape hatch, we can remove the additional
checks as well.

Closes elastic#22253
@danielmitterdorfer danielmitterdorfer mentioned this issue Oct 18, 2018
Merged
danielmitterdorfer added a commit that referenced this issue Oct 19, 2018
@danielmitterdorfer
Remove hand-coded XContent duplicate checks
dbb6fe5 
With this commit we cleanup hand-coded duplicate checks in XContent
parsing. They were necessary previously but since we reconfigured the
underlying parser in #22073 and #22225, these checks are obsolete and
were also ineffective unless an undocumented system property has been
set. As we also remove this escape hatch, we can remove the additional
checks as well.

Closes #22253
Relates #34588
kcm pushed a commit that referenced this issue Oct 30, 2018
@danielmitterdorfer @kcm
Remove hand-coded XContent duplicate checks
f2f08c0 
With this commit we cleanup hand-coded duplicate checks in XContent
parsing. They were necessary previously but since we reconfigured the
underlying parser in #22073 and #22225, these checks are obsolete and
were also ineffective unless an undocumented system property has been
set. As we also remove this escape hatch, we can remove the additional
checks as well.

Closes #22253
Relates #34588
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
blocker :Core/Infra/REST API REST infrastructure and utilities >enhancement v7.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@clintongormley @nik9000 @jpountz @danielmitterdorfer @jimczi