-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Turn off gzip for ElasticSearch in functional tests #9078
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
base: main
Are you sure you want to change the base?
Conversation
ebaed3f to
ff8e328
Compare
ff8e328 to
ba2abcf
Compare
| // httpClient is the awsHttpClient to be used for creating esClient. | ||
| httpClient *http.Client | ||
| // disableGzip disables gzip compression for Elasticsearch requests. | ||
| disableGzip bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intentionally not exported as it's for testing purposes only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tbh, I would even support making this public. Gzip is notoriously inefficient and is a terrible choice for protocol compression. (It's a terrible choice for any compression these days except for compatibility concerns.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems we enabled it in production by default on a hunch:
It might help reduce/improve network traffic.
cc @rodrigozhou
| // httpClient is the awsHttpClient to be used for creating esClient. | ||
| httpClient *http.Client | ||
| // disableGzip disables gzip compression for Elasticsearch requests. | ||
| disableGzip bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tbh, I would even support making this public. Gzip is notoriously inefficient and is a terrible choice for protocol compression. (It's a terrible choice for any compression these days except for compatibility concerns.)
What changed?
Turn off gzip for ElasticSearch in functional tests.
Why?
Looking at a Go heap from a functional test that uses the ElasticSearch client (link), we can see that significant allocations (and likely CPU usage) are spent on compressing/decompressing.
It turns out that this is only happening on setups with ElasticSearch. And since the tests run on a single machine, gzip has very little benefit. Turning it off reduces memory and CPU usage significantly.
The entries are completely removed from this PR's Go heap dump (example).
How did you test it?
Potential risks
gzipis enabled by default in test envs, staging and production; therefore still being exercised sufficiently.