logstash-plugins / logstash-input-gelf

Apache License 2.0
20 stars 39 forks source link

Refactor the shutdown #19

Closed ph closed 8 years ago

ph commented 9 years ago

This PR introduces the changes needed to follow the new shutdown semantic, it also remove the gelf as a runtime dependencies and move it as a development dependency since its only used in the tests.

Fixes #17

jsvd commented 9 years ago

there might be some race condition happening here, I get the following every 1 in 5 or 6

logstash-input-gelf (git)-[pr/19] % bundle exec rspec 
Using Accessor#strict_set for specs
Run options: exclude {:redis=>true, :socket=>true, :performance=>true, :couchdb=>true, :elasticsearch=>true, :elasticsearch_secure=>true, :export_cypher=>true, :integration=>true, :windows=>true}
.F

Failures:

  1) LogStash::Inputs::Gelf when interrupting the plugin behaves like an interruptible input plugin #stop returns from run
     Failure/Error: subject.stop
     IOError:
       No such file or directory
     Shared Example Group: "an interruptible input plugin" called from ./spec/inputs/gelf_spec.rb:18
     # ./lib/logstash/inputs/gelf.rb:73:in `stop'
     # /Users/joaoduarte/projects/logstash-devutils/lib/logstash/devutils/rspec/shared_examples.rb:16:in `(root)'
     # /Users/joaoduarte/.rvm/gems/jruby-1.7.19/gems/rspec-wait-0.0.7/lib/rspec/wait.rb:46:in `(root)'

Finished in 1 second (files took 2.66 seconds to load)
2 examples, 1 failure

Failed examples:

rspec /Users/joaoduarte/projects/logstash-devutils/lib/logstash/devutils/rspec/shared_examples.rb:9 # LogStash::Inputs::Gelf when interrupting the plugin behaves like an interruptible input plugin #stop returns from run

Randomized with seed 22953
ph commented 9 years ago

@jsvd looking at it, I guess we have it for a while. ;)

ph commented 9 years ago

I can reproduce it with your seed. <3

ph commented 9 years ago

@jsvd it was in a middle of a crash, I have changed the code to use stoppable_sleep and added a rescue IOError when trying to close the socket, because its totally possible to try to close a socket which is in an inconsistent state.

ph commented 8 years ago

Weird, non deterministic errors are back, investigating.

ph commented 8 years ago

ready for another round of review @jsvd

jsvd commented 8 years ago

Minor comment, otherwise LGTM. squash and merge :sheep: it

elasticsearch-bot commented 8 years ago

Merged sucessfully into master!