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

Wrap ContentTooLongException into a BatchSizeTooLargeException #17449

Merged
merged 2 commits into from Nov 24, 2023

Conversation

mpfz0r
Copy link
Member

@mpfz0r mpfz0r commented Nov 23, 2023

The dynamic batch size of archiving depends on catching these exceptions
and then retrying the scroll query with a smaller batch size.

Fixes https://github.com/Graylog2/graylog-plugin-enterprise/issues/3318

Tested with ES/OS containers with http.max_content_length=10mb env.

ES:

docker run --name elasticsearch -e "http.host=0.0.0.0" -p 9200:9200 -e "xpack.security.enabled=false" -e "http.max_content_length=10mb" -e "discovery.type=single-node" -e "action.auto_create_index=.watches,.triggered_watches,.watcher-history-*"   docker.elastic.co/elasticsearch/elasticsearch:7.10.2

OS:

docker run -d --name opensearch -p 9200:9200 -p 9600:9600 -e "http.max_content_length=10mb" -e "discovery.type=single-node" -e "DISABLE_INSTALL_DEMO_CONFIG=true" -e "DISABLE_SECURITY_PLUGIN=true" -e "action.auto_create_index=false" opensearchproject/opensearch:2.6.0

Ideally that should have an integration test, but that's just so much work :-/

The dynamic batch size of archiving depends on catching these exceptions
and then retrying the scroll query with a smaller batch size.

Fixes Graylog2/graylog-plugin-enterprise#3318
@mpfz0r mpfz0r force-pushed the fix-dynamic-archiving-batch-size branch from a37f01f to 749ebd1 Compare November 23, 2023 15:27
@mpfz0r mpfz0r changed the title WIP Wrap ContentTooLongException into an BatchSizeTooLargeException Nov 23, 2023
@mpfz0r mpfz0r marked this pull request as ready for review November 23, 2023 15:41
@mpfz0r mpfz0r changed the title Wrap ContentTooLongException into an BatchSizeTooLargeException Wrap ContentTooLongException into a BatchSizeTooLargeException Nov 23, 2023
Copy link
Contributor

@patrickmann patrickmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - nice catch. Pretty elusive since it depends on document size.

On a side note: wondering why we don't have a base class for the search client classes. They have a lot of code in common.

@mpfz0r mpfz0r merged commit 32cf437 into master Nov 24, 2023
15 of 16 checks passed
@mpfz0r mpfz0r deleted the fix-dynamic-archiving-batch-size branch November 24, 2023 11:11
mpfz0r added a commit that referenced this pull request Nov 24, 2023
* Wrap ContentTooLongException into an BatchSizeTooLargeException

The dynamic batch size of archiving depends on catching these exceptions
and then retrying the scroll query with a smaller batch size.

Fixes Graylog2/graylog-plugin-enterprise#3318

* Add changelog

(cherry picked from commit 32cf437)
mpfz0r added a commit that referenced this pull request Nov 24, 2023
* Wrap ContentTooLongException into an BatchSizeTooLargeException

The dynamic batch size of archiving depends on catching these exceptions
and then retrying the scroll query with a smaller batch size.

Fixes Graylog2/graylog-plugin-enterprise#3318

* Add changelog

(cherry picked from commit 32cf437)
patrickmann pushed a commit that referenced this pull request Nov 24, 2023
… (#17459)

* Wrap ContentTooLongException into an BatchSizeTooLargeException

The dynamic batch size of archiving depends on catching these exceptions
and then retrying the scroll query with a smaller batch size.

Fixes Graylog2/graylog-plugin-enterprise#3318

* Add changelog

(cherry picked from commit 32cf437)
patrickmann pushed a commit that referenced this pull request Nov 24, 2023
… (#17458)

* Wrap ContentTooLongException into an BatchSizeTooLargeException

The dynamic batch size of archiving depends on catching these exceptions
and then retrying the scroll query with a smaller batch size.

Fixes Graylog2/graylog-plugin-enterprise#3318

* Add changelog

(cherry picked from commit 32cf437)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants