-
Notifications
You must be signed in to change notification settings - Fork 3.5k
fix DLQ integration tests #12871
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
fix DLQ integration tests #12871
Conversation
9045fa1 to
c9f68fe
Compare
andsel
left a comment
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.
LGTM, only tiny doubt on the change of pipeline.batch.size
| "dead_letter_queue.enable" => true, | ||
| "pipeline.batch.size" => 1, | ||
| "config.string" => "input { generator { message => '#{message}' codec => \"json\" count => 1000 } } filter { mutate { add_field => { \"geoip\" => \"somewhere\" } } } output { elasticsearch {} }" | ||
| "pipeline.batch.size" => 100, |
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.
The downsize of the batch size is it functional to the test or to the fact that now it's single node cluster?
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.
don't mind my comment, I thought it passed from 100 to 1, but it's opposite and is reasonable to have a batch greater than the unit
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.
yeah having size 1 was making these tests unnecessarily slow
* master: fix DLQ integration tests (elastic#12871) Fix ES HOW integration tests on `master` (elastic#12872) Update logstash_releases.json Support for UTF-16 and other multi-byte-character logfiles (elastic#9702) change download path for geoip plugin (elastic#12863) Change Gradle's :logstash-integration-tests:integrationTests task to depends on copyES (elastic#12847) Doc: Keystore must be accessible to logstash user (elastic#12775) Remove json ~>1 pinning (elastic#12851) Adapted install/uninstall/list PluginManager's command to respect the alised plugins (elastic#12821) Load a plugin by alias name. (elastic#12796)
This PR changes the DLQ integration tests to not rely on default names of indices for writing data to ES.
Also ensures ES is started in a single node mode.
These changes remove some of the magic of the integration test, where we relied on a index having a template with a geoip field with geo data types already. In these commits we explicitly push a template and then use a message that clearly clashes with that template.