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

Improve performance of extracting warning value #24114

Merged
merged 1 commit into from
Apr 14, 2017

Conversation

jasontedor
Copy link
Member

When building headers for a REST response, we de-duplicate the warning headers based on the actual warning value. The current implementation of this uses a capturing regular expression that is prone to excessive backtracking. In cases a request involves a large number of warnings, this extraction can be a severe performance penalty. An example where this can arise is a bulk indexing request that utilizes a deprecated feature (e.g., using deprecated forms of boolean values). This commit is an attempt to address this performance regression. We already know the format of the warning header, so we do not need to use a regular expression to parse it but rather can parse it by hand to extract the warning value. This gains back the vast majority of the performance lost due to the usage of a deprecated feature. There is still a performance loss due to logging the deprecation message but we do not address that concern in this commit.

Closes #24018

When building headers for a REST response, we de-duplicate the warning
headers based on the actual warning value. The current implementation of
this uses a capturing regular expression that is prone to excessive
backtracking. In cases a request involves a large number of warnings,
this extraction can be a severe performance penalty. An example where
this can arise is a bulk indexing request that utilizes a deprecated
feature (e.g., using deprecated forms of boolean values). This commit is
an attempt to address this performance regression. We already know the
format of the warning header, so we do not need to use a regular
expression to parse it but rather can parse it by hand to extract the
warning value. This gains back the vast majority of the performance lost
due to the usage of a deprecated feature. There is still a performance
loss due to logging the deprecation message but we do not address that
concern in this commit.
@jasontedor jasontedor merged commit 09efdc3 into elastic:master Apr 14, 2017
jasontedor added a commit that referenced this pull request Apr 14, 2017
When building headers for a REST response, we de-duplicate the warning
headers based on the actual warning value. The current implementation of
this uses a capturing regular expression that is prone to excessive
backtracking. In cases a request involves a large number of warnings,
this extraction can be a severe performance penalty. An example where
this can arise is a bulk indexing request that utilizes a deprecated
feature (e.g., using deprecated forms of boolean values). This commit is
an attempt to address this performance regression. We already know the
format of the warning header, so we do not need to use a regular
expression to parse it but rather can parse it by hand to extract the
warning value. This gains back the vast majority of the performance lost
due to the usage of a deprecated feature. There is still a performance
loss due to logging the deprecation message but we do not address that
concern in this commit.

Relates #24114
jasontedor added a commit that referenced this pull request Apr 14, 2017
When building headers for a REST response, we de-duplicate the warning
headers based on the actual warning value. The current implementation of
this uses a capturing regular expression that is prone to excessive
backtracking. In cases a request involves a large number of warnings,
this extraction can be a severe performance penalty. An example where
this can arise is a bulk indexing request that utilizes a deprecated
feature (e.g., using deprecated forms of boolean values). This commit is
an attempt to address this performance regression. We already know the
format of the warning header, so we do not need to use a regular
expression to parse it but rather can parse it by hand to extract the
warning value. This gains back the vast majority of the performance lost
due to the usage of a deprecated feature. There is still a performance
loss due to logging the deprecation message but we do not address that
concern in this commit.

Relates #24114
jasontedor added a commit that referenced this pull request Apr 14, 2017
When building headers for a REST response, we de-duplicate the warning
headers based on the actual warning value. The current implementation of
this uses a capturing regular expression that is prone to excessive
backtracking. In cases a request involves a large number of warnings,
this extraction can be a severe performance penalty. An example where
this can arise is a bulk indexing request that utilizes a deprecated
feature (e.g., using deprecated forms of boolean values). This commit is
an attempt to address this performance regression. We already know the
format of the warning header, so we do not need to use a regular
expression to parse it but rather can parse it by hand to extract the
warning value. This gains back the vast majority of the performance lost
due to the usage of a deprecated feature. There is still a performance
loss due to logging the deprecation message but we do not address that
concern in this commit.

Relates #24114
@jasontedor
Copy link
Member Author

Thank you @jpountz.

@jasontedor jasontedor deleted the warning-value-performance branch April 14, 2017 16:21
jasontedor pushed a commit that referenced this pull request Jan 13, 2020
This commit improves the performance of warning value extraction in the
low-level REST client, and is similar to the approach taken in
#24114. There are some differences since the low-level REST client might
be connected to Elasticsearch through a proxy that injects its own
warnings.
jasontedor pushed a commit that referenced this pull request Jan 13, 2020
This commit improves the performance of warning value extraction in the
low-level REST client, and is similar to the approach taken in
#24114. There are some differences since the low-level REST client might
be connected to Elasticsearch through a proxy that injects its own
warnings.
jasontedor pushed a commit that referenced this pull request Jan 13, 2020
This commit improves the performance of warning value extraction in the
low-level REST client, and is similar to the approach taken in
#24114. There are some differences since the low-level REST client might
be connected to Elasticsearch through a proxy that injects its own
warnings.
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
This commit improves the performance of warning value extraction in the
low-level REST client, and is similar to the approach taken in
elastic#24114. There are some differences since the low-level REST client might
be connected to Elasticsearch through a proxy that injects its own
warnings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants