jordansissel
commented
8 years ago I haven't reviewed the code yet partly because there's no CLA signed. Can you do https://github.com/elastic/logstash/blob/master/CONTRIBUTING.md#contribution-steps ?
I'll do a review after CLA so I can look at the code :)
One comment on your description:
I would also be argue that since the logstash metrics are not time stamped when sent to graphite
I dont' think this is true. Logstash does use the timestamp from the event when sending it to graphite:
- The text sent over tcp includes a timestamp: https://github.com/logstash-plugins/logstash-output-graphite/blob/master/lib/logstash/outputs/graphite.rb#L187
- And that timestamp is unix epoch in seconds: https://github.com/logstash-plugins/logstash-output-graphite/blob/master/lib/logstash/outputs/graphite.rb#L173
smford
elasticsearch-release
RickardCardell
TommySedin
karmi
synFK
Preventing output blocking when the graphite server is offline by introducing a resend_attempts counter.
TLDR: This patch adds a resend_attempts option that ensures that metrics are discarded after a 3 failed attempts, or by whatever number is defined by the user in the logstash configuration.
There is is a problem with the previous logstash-output-graphite plugin that if the resend_on_failure was enabled and the graphite server went offline, logstash would infinitely attempt to resend metrics and end up blocking logstashs single output worker entirely (meaning logstash processed logs could not reach elasticsearch). Given that in some business environments graphites metrics are not considered vital, the option for discarding metrics after a few attempts is often considered acceptable, compared to lost or delayed logstash processed logs.
This could even result in logstash crashing as both logs and metrics get held in memory awaiting to be outputted by the now blocked single output worker. This issue would cause back pressure to the logstash-forwarder clients sending logs to logstash. This problem could compound with logstashs own logs becoming filled with millions of back pressure warnings until all local file storage is consumed.
I would also be argue that since the logstash metrics are not time stamped when sent to graphite, the discarding of stale metrics is far more favourable than having a large backlog of stale metrics being dumped in to graphite in a small window of time once the graphite server does come back online. Because graphite has a metric storage resolution/retention policy, a large surge/backlog of the same metric arriving from logstash within a window smaller than the resolution/retention policy, will simply mean the metrics get discarded anyway.
We have tested this extensively on logstash v1.5.4-1 and found logstash to be considerably more stable.
The resend_attempts option works well with a reduced reconnect_interval setting too, for example: reconnect_interval => 1 resend_attempts => 3 resend_on_failure => true
Note: I will create another PR in the future to deal with the other logstash-output-graphite problem where logstash cannot start without being able to connect to a working graphite server.