Remove legacy output concurrency by andrewvc · Pull Request #19003 · elastic/logstash

andrewvc · 2026-04-14T21:39:25Z

Release notes

The default legacy output strategy is now removed. This should cause no user-facing issues.

What does this PR do?

Removes the legacy output strategy, which used the ancient and long deprecated multi-worker concurrency queue with a per-output queue / threadpool. See #18971 for more detail / analysis.

There is a small non-breaking change in behavior, now when users declare workers => 2 or greater values instead of an exception being thrown they only get a warning. This should not be considered a breaking change since this is something no one could rightly be imagined to depend on.

Why is it important/What is the impact to the user?

Code simplification / cleanup.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files (and/or docker env variables)
I have added tests that prove my fix is effective or that my feature works

Author's Checklist

Make sure that running with the workers setting produces no effect
Make sure that outputs with undeclared concurrency still work

How to test this PR locally

Confirm the commands / output below

#Try this as-is, and with workers => 1 and workers => 2
bin/logstash -e 'input { generator { count => 10 } } output { pipe { command => "tee teeout" } }'
# View the output
cat teeout

Related issues

Fixes #18971

github-actions · 2026-04-14T21:39:37Z

🤖 GitHub comments

Just comment with:

run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)
run exhaustive tests : Run the exhaustive tests Buildkite pipeline.

mergify · 2026-04-14T22:35:10Z

This pull request does not have a backport label. Could you fix it @andrewvc? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit.
If no backport is necessary, please add the backport-skip label

…t-concurrency

andrewvc · 2026-04-16T01:57:06Z

+    # Outputs that never declared a concurrency strategy used the now-removed :legacy
+    # strategy, which accepted a `workers` setting. Strip it so existing configs with
+    # `workers => 1` don't blow up.
+    if !self.class.instance_variable_defined?(:@concurrency) && @params.delete("workers")


I was on the fence about adding a new @concurrency_explicit boolean to detect this, glad to switch to that approach if reviewers find this pragile.

I would find it simpler to just use the :deprecated => "Extra text for deprecation log" facility already supported by the config mixin.

yaauie · 2026-04-16T20:17:05Z

+    # Outputs that never declared a concurrency strategy used the now-removed :legacy
+    # strategy, which accepted a `workers` setting. Strip it so existing configs with
+    # `workers => 1` don't blow up.
+    if !self.class.instance_variable_defined?(:@concurrency) && @params.delete("workers")


I would find it simpler to just use the :deprecated => "Extra text for deprecation log" facility already supported by the config mixin.

yaauie · 2026-04-16T20:17:59Z

+    # strategy, which accepted a `workers` setting. Strip it so existing configs with
+    # `workers => 1` don't blow up.
+    if !self.class.instance_variable_defined?(:@concurrency) && @params.delete("workers")
+      self.logger.warn("Output plugin #{self.class.name}: the `workers` setting is no longer used and will be removed in a future release.")


if we do our own logging, we should use the deprecation_logger#deprecated() so that it goes to the deprecation log.

yaauie · 2026-04-16T20:19:08Z

    it "should instantiate cleanly" do
-      params = { "dummy_option" => "potatoes", "codec" => "json", "workers" => 2 }
-      worker_params = params.dup; worker_params["workers"] = 1
+      params = { "dummy_option" => "potatoes", "codec" => "json" }


here's a bug that pre-existed your change: the params local variable is assigned but not used.

Nice catch!

andrewvc · 2026-04-16T23:05:59Z

@yaauie thanks for the review, I've incorporated all your fixes, should be ready for review again.

kaisecheng · 2026-04-21T15:28:47Z

/run exhaustive tests

kaisecheng · 2026-04-21T15:30:50Z

run exhaustive tests

kaisecheng

LGTM. The red CI is unrelated to this change

yaauie

I think that the general approach makes sense. I have a couple comments on finding a path forward that gives users more-meaningful info if/when they hit the issue.

Add rye's suggestion for improved error message for legacy concurrency plugins Co-authored-by: Rye Biesemeyer <yaauie@users.noreply.github.com>

Use inspect in the corner case of missing `config_name` Co-authored-by: Rye Biesemeyer <yaauie@users.noreply.github.com>

elasticmachine · 2026-04-30T20:16:06Z

💔 Build Failed

Buildkite Build
Commit: 2dab9bd

Failed CI Steps

History

💛 Build #4704 was flaky b30ee46
💚 Build #4689 succeeded 1d56ce0
💔 Build #4688 failed a8d2940
💛 Build #4684 was flaky 3ecd923

andrewvc added 2 commits April 14, 2026 16:23

Remove legacy output concurrenncy

963183e

Fix failing tests

3ecd923

andrewvc added the bug label Apr 14, 2026

andrewvc added the backport-skip Skip automated backport with mergify label Apr 15, 2026

andrewvc requested a review from kaisecheng April 15, 2026 19:08

andrewvc marked this pull request as ready for review April 15, 2026 19:08

andrewvc added 4 commits April 15, 2026 14:14

Add safeguard for 'workers => 1'

6eb1e2d

Merge remote-tracking branch 'upstream/main' into remove-legacy-outpu…

bedd960

…t-concurrency

Add safeguard for 'workers => 1'

a8d2940

Fix failing test

1d56ce0

andrewvc commented Apr 16, 2026

View reviewed changes

yaauie reviewed Apr 16, 2026

View reviewed changes

Incorporate PR feedback, switch to parameter

b30ee46

andrewvc commented Apr 16, 2026

View reviewed changes

Comment thread logstash-core/lib/logstash/config/mixin.rb Outdated

kaisecheng approved these changes Apr 21, 2026

View reviewed changes

yaauie reviewed Apr 22, 2026

View reviewed changes

Comment thread logstash-core/lib/logstash/outputs/base.rb Outdated

Comment thread logstash-core/lib/logstash/config/mixin.rb Outdated

andrewvc and others added 2 commits April 30, 2026 14:06

Update logstash-core/lib/logstash/outputs/base.rb

5989c5c

Add rye's suggestion for improved error message for legacy concurrency plugins Co-authored-by: Rye Biesemeyer <yaauie@users.noreply.github.com>

Apply suggestions from code review

2dab9bd

Use inspect in the corner case of missing `config_name` Co-authored-by: Rye Biesemeyer <yaauie@users.noreply.github.com>

robbavey assigned andrewvc and yaauie May 1, 2026

Conversation

andrewvc commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Release notes

What does this PR do?

Why is it important/What is the impact to the user?

Checklist

Author's Checklist

How to test this PR locally

Related issues

Uh oh!

github-actions Bot commented Apr 14, 2026

🤖 GitHub comments

Uh oh!

mergify Bot commented Apr 14, 2026

Uh oh!

andrewvc Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

yaauie Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yaauie Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

yaauie Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

yaauie Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

andrewvc Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andrewvc commented Apr 16, 2026

Uh oh!

kaisecheng commented Apr 21, 2026

Uh oh!

kaisecheng commented Apr 21, 2026

Uh oh!

kaisecheng left a comment

Choose a reason for hiding this comment

Uh oh!

yaauie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

elasticmachine commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💔 Build Failed

Failed CI Steps

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

andrewvc commented Apr 14, 2026 •

edited

Loading

elasticmachine commented Apr 30, 2026 •

edited

Loading