Reset default worker to 1 if using non thread-safe filter

[suyograo]

See background here: https://discuss.elastic.co/t/multiline-issue/33349

LS will fail to startup in 2.0 when using ML filter. It should instead revert to old behavior of setting workers to 1 and start the pipeline.

Using the config specified in the above discuss link

suyog@machine:~/bin/releases/2.0.0/logstash-2.0.0$ bin/logstash -f discuss.conf
Error: Cannot use more than 1 filter worker because the following plugins don't work with more than one worker: multiline, multiline
You may be interested in the '--configtest' flag which you can
use to validate logstash's configuration before you choose
to restart a running system.

[ph]

We are going away from the multiline filter with @guyboertje's changes to the multiline codec, is this really necessary? We also did a major version bump from 1.5 to 2.0.

Maybe improving the wording of the message and offer more guidance explaining the changes in the message would be sufficient.

[colinsurprenant]

I am not sure we should necessarily "go away" - there certainly are and will be valid use cases for multiline as a filter? In any case we should definitely unify the code bases with the multiline codec or see if we can use @jsvd work on codec filters?

[ph]

I am +1 for merging the code base.

Let assume we have valid use cases for it. But I am not sure if we should automatically uses -w 1 if the multiline filter is used. There is a lot of gain of having multiples workers threads, so this is why I am proposing to improving the message to educate users.

[colinsurprenant]

As a short term fix and for the "least surprise" principle I am +1 on having automatic fallback to 1 worker if a non thread-safe filter is used (and logging a warning that this is probably not the most efficient configuration).

[jordansissel]

The problem with multi line filter and workers is once you have more than
one worker, you lose ordering/sequence of events. Without order, the multi
line filter cannot function correctly - it's not about performance, it's
about correctness. I can diagram this if I am explaining poorly; let me
know.

On Monday, November 2, 2015, Pier-Hugues Pellerin notifications@github.com
wrote:

I am +1 for merging the code base.

Let assume we have valid use cases for it. But I am not sure if we should
automatically uses -w 1 if the multiline filter is used. There is a lot
of gain of having multiples workers threads, so this is why I am proposing
to improving the message to educate users.

—
Reply to this email directly or view it on GitHub
#4130 (comment).

[colinsurprenant]

@jordansissel I think everyone understands the problem - the issue is more about automatically falling back to single worker or not by default if/when the multiline filter is in use (or any other non thread-safe filter for that matter).

[suyograo]

IMO, starting with 1 worker is better than not starting at all and then forcing the user to supply -w 1. Sure, ML filter should not work with multiple workers, but the default should be chosen wisely -- in this case 1. Also, +1 on logging about why this default was chosen will be helpful.

[jordansissel]

+1

On Monday, November 2, 2015, Suyog Rao notifications@github.com wrote:

IMO, starting with 1 worker is better than not starting at all and then
forcing the user to supply -w 1. Sure, ML filter should not work with
multiple workers, but the default should be chosen wisely -- in this case

1. Also, +1 on logging about why this default was chosen will be helpful.

—
Reply to this email directly or view it on GitHub
#4130 (comment).

[ph]

@suyograo @colinsurprenant I am +1 with your proposition.

[colinsurprenant]

CONSENSUS! :D

just to make sure:

• if a non thread-safe filter is used we change the default number of workers to 1 and log a warning for why.
• if a non thread-safe filter is used AND the number of workers is set to > 1 we log a warning that this may cause problems but continue the startup.

[jordansissel]

@colinsurprenant agreed

[guyboertje]

This will require a small refactor

[guyboertje]

Submitting PR today.

[EamonZhang]

+1

[guyboertje]

PR #4156