search

logstash-plugins / logstash-input-cloudwatch

A Logstash input to pull events from the Amazon Web Services CloudWatch API
Apache License 2.0
43 stars 27 forks source link

Fix plugin shutdown handling #43

Closed robbavey closed 6 years ago

robbavey commented 6 years ago

Makes call to Stud.stop! to shutdown the loop when the plugin is stopped

Fixes #41

colinsurprenant commented 6 years ago

@robbavey I would advise against that design which has proven to cause concurrency problems and is hard to reason about. See this PR were I fixed such a problem this week logstash-input-heartbeat#15

Would it be possible to use this pattern instead?

colinsurprenant commented 6 years ago

@robbavey great! thanks! code LGTM but could you also add the input shutdown spec similar to this also? https://github.com/logstash-plugins/logstash-input-heartbeat/blob/cc94ae29c1457d5f4e51a8c1bce3963ad7aebb07/spec/inputs/heartbeat_spec.rb#L6-L8

colinsurprenant commented 6 years ago

@robbavey I am also fine if you want to followup with another PR for the spec, your choice. LGTM

colinsurprenant commented 6 years ago

LGTM² thanks for the spec @robbavey

elasticsearch-bot commented 6 years ago

Rob Bavey merged this into the following branches!

Branch Commits
master 6b37bde82e45ce34913edc1461230727cdcf8e40, d68ab4fda97314db87f5a6cd146e9485aa9b5353, d7892cf36a36f7b3b42c5b02117454afb5dd6eda, bb9a266288b838f9a92b8f8d6effa4627bf15811
robbavey commented 6 years ago

Thanks @colinsurprenant!