-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
deprecate pipeline bus v1 #16643
base: 8.x
Are you sure you want to change the base?
deprecate pipeline bus v1 #16643
Conversation
Quality Gate passedIssues Measures |
💚 Build Succeeded
|
The addition of the logger commit makes sense. Im a bit confused about the retrofit test commit though c51a7d5 is that refactor a general update that is extending a pattern being applied elsewhere you could point me to? After looking closer: I see organizational improvements and some more formal coverage of the |
|
||
// This should unblock the listener thread | ||
bus.unregisterSender(output, addresses); | ||
TimeUnit.SECONDS.toMillis(30); |
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.
I think this was just moved around, not introduced but I think its unused.
Yeah, the first commit just retrofits tests onto the bit that selects which implementation to use, and needed to wrap the existing As you saw, the star of the PR is the second commit, which adds the deprecation log and verifies that it is emitted in the right conditions. In retrospect, the other two tests should have a negative assertion. I'll add those and bounce it back. |
Release notes
What does this PR do?
Adds a deprecation warning if Logstash is configured to use the legacy v1 pipeline bus. The legacy implementation was replaced with a more-performant one in 8.15.0, and v1 was only left in place as an "escape hatch".
The ability to select v1 implementation is set to be removed on
main
in advance of 9.0 via #16644Why is it important/What is the impact to the user?
It should have no user impact; the escape hatch was never documented, and is not expected to be used in the real world.
Checklist
[ ] I have made corresponding changes to the documentation[ ] I have made corresponding change to the default configuration files (and/or docker env variables)How to test this PR locally
-Dlogstash.pipelinebus.implementation=v1
toconfig/jvm.options