Remove legacy output concurrency

[andrewvc]

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]

🤖 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]

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

[yaauie]

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

[andrewvc]

Nice catch!

[andrewvc]

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

[kaisecheng]

/run exhaustive tests

[kaisecheng]

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.

[elasticmachine]

💔 Build Failed

• Buildkite Build
• Commit: 2dab9bd

Failed CI Steps

• 🥼 Integration Tests - FIPS mode / part 5-of-6
• 🥼 Integration Tests - FIPS mode / part 5-of-6
• 🥼 Integration Tests - FIPS mode / part 5-of-6
• 🥼 Integration Tests - FIPS mode / part 5-of-6
• 🥼 Integration Tests - FIPS mode / part 6-of-6
• 🥼 Integration Tests - FIPS mode / part 6-of-6
• 🥼 Integration Tests - FIPS mode / part 6-of-6
• 🥼 Integration Tests - FIPS mode / part 6-of-6

History

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