Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor @run_mutex for the new shutdown logic #3886

Closed
colinsurprenant opened this issue Sep 8, 2015 · 3 comments
Closed

refactor @run_mutex for the new shutdown logic #3886

colinsurprenant opened this issue Sep 8, 2015 · 3 comments

Comments

@colinsurprenant
Copy link
Contributor

@run_mutex specific refactor discussion moved here from #3812. Copied comment

after re-analyzing the @run_mutex related code it is useful for 2 things:

  • consistent view of the @ready and @started properties which need to be consistent across threads since the related ready? and started? methods will be typically called from another thread that the one executing the run method
  • to avoid calling the shutdown code before the startup is completed

I believe the ready? and started? methods could be implemented using atomic booleans and for the prevention startup and shutdown race condition it should be refactored in a better way. It really isn't clear now.

@jsvd
Copy link
Member

jsvd commented Sep 9, 2015

ready? and started? are only used for plugin specs, similar to what we're discussing in #3885

And looking at the existing plugins we have:

~/all_logstash_plugins % grep -e started? -e ready? . -r
./logstash-input-relp/spec/spec_helper.rb:    sleep 0.1 while !pipeline.ready?
./logstash-output-s3/spec/outputs/s3_spec.rb:        sleep 0.1 while !pipeline.ready?
~/projects/logstash % grep -e started? -e ready? . -r
./lib/logstash/pipeline.rb:  def ready?
./lib/logstash/pipeline.rb:  def started?

Regarding the second point "to avoid calling the shutdown code before the startup is completed", do you mind doing a thought experiment and answer.."why is that a requirement?"

@colinsurprenant
Copy link
Contributor Author

commits for this pushed in #3895

@suyograo
Copy link
Contributor

Addressed in #3895

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants