elastic / elastic/logstash

Shutdown Semantics: Exception handling in the pipeline

Open
#2,477 8 comments 0 reactions 1 assignee Claimed by @jsvd View on GitHub
code cleanup design discuss
Dominant language
Java
Stars
14.9k
Forks
3.5k
Avg merge
19h 14m
Merged PRs (30d)
63

Description

Related issues/prs:
https://github.com/elasticsearch/logstash/pull/1373
https://github.com/elasticsearch/logstash/issues/1250
https://github.com/elasticsearch/logstash/issues/2130
https://github.com/elasticsearch/logstash/issues/2152

The pipeline is composed of [{input,filter,output}workers](https://github.com/elasticsearch/logstash/blob/master/lib/logstash/pipeline.rb#L171-L241) that execute the plugins' code and take care of uncaught exceptions that may occur during their execution.

These workers should tolerate (hopefully) transient exceptions and not allow the pipeline to crash. Also, known fatal exceptions should abort the pipeline execution and terminate logstash. Therefore, workers should have the following behaviour:
##### Input workers have a long running method call `run`

``` ruby
begin
# call input_plugin.run (usually a loop)
rescue TransientException
@logger.warn event, exception
retry
rescue FatalException
@logger.error event, exception
rescue ShutdownSignal
@logger.info event
ensure # in both situations above we just want to log, call teardown, and exit
teardown
end
```
##### Filters and output workers continuously pop from a queue

``` ruby
begin
loop
event = queue.pop
if LogStash::Event
# call plugin code
elsif LogStash::ShutdownEvent
# if filter pass event along
break
end
end
rescue TransientException
@logger.warn event, exception
retry
rescue FatalException
@logger.error event, exception
ensure
@filters.each teardown
# or
@outputs.each teardown
end
```
#### What is a TransientException / FatalException?

We have no way of knowing what the plugin considers a transient/fatal exception. Two possible choices:

1) Treat all StandardError derived exceptions as transient, those derived from Exception as fatal
2) Provide LogStash::TransientException as a parent exception for transient exceptions, LogStash::FatalException for fatal ones. Plugins would only throw descendants of these 2 classes
#### Questions

Should a transient failure call some setup/teardown behavior on the plugin?
[Current code for the input plugin](https://github.com/elasticsearch/logstash/blob/master/lib/logstash/pipeline.rb#L171-L196) calls teardown and then retries, but at that point the plugin instance might no longer be usable.

For filters and outputs, the shutdown is done through a ShutdownEvent. However, if a plugin is very slow (e.g. executing sleep 100000), it will never pop the ShutdownEvent from the queue and execute teardown. Should there be a pipeline.shutdown(force=true) on some situations?
#### Thread naming problem

Related issues:
https://github.com/elasticsearch/logstash/issues/2462
https://github.com/elasticsearch/logstash/issues/2425
#### Best practices when developing plugins

Exception handling recommendations:
- which kind of exceptions should reach the worker?
- what happens if a plugin misbehaves (throws top level Exception, hangs)?

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.