Skip to content

Compression should be done on splitted batches #937

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

Closed
wants to merge 2 commits into from

Conversation

agix
Copy link

@agix agix commented Apr 20, 2020

Elasticsearch http.max_content_length limit (https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-http.html) is applied on the plain content wether it's compressed or not.

In this plugin when compression is enabled, compressed batches are splitted using arbitrary constant TARGET_BULK_BYTES of 20mb.

It's possible to find cases where uncompressed data are bigger than http.max_content_length (100 mb) but smaller than TARGET_BULK_BYTES if compressed resulting on 413 Request Entity Too Large. (related to #823)

(with elasticsearch http.max_content_length defined to 1mb):

agix@host:/tmp# ls -alh /tmp/aaaa*
-rw-r--r-- 1 agix agix 2.1M Apr 20 18:14 /tmp/aaaa
-rw-r--r-- 1 agix agix 2.1K Apr 20 18:14 /tmp/aaaa.gz
agix@host:/tmp# curl -i 127.0.0.1:9200 --data-binary @/tmp/aaaa -H 'Content-Type: application/json'
HTTP/1.1 413 Request Entity Too Large
content-length: 0

agix@host:/tmp# curl -i 127.0.0.1:9200 --data-binary @/tmp/aaaa.gz -H 'Content-Type: application/json' -H 'Content-Encoding: gzip'
HTTP/1.1 100 Continue

HTTP/1.1 413 Request Entity Too Large
content-length: 0
connection: close

Furthermore, as the elasticsearch http.max_content_length options exists, TARGET_BULK_BYTES should be configurable (default to the elasticsearch default of 100mb) so people can pick their own value.

I think multiple issues/PR are related to this (#785, #833, #786)

@kares
Copy link
Contributor

kares commented Nov 25, 2021

Thanks for the PR, believe similar functionality has been implemented #972 (in plugin version 10.8.6)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants