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

use plugin.stop to return from run #7

Closed
wants to merge 1 commit into from

Conversation

jsvd
Copy link
Member

@jsvd jsvd commented Aug 31, 2015

Implements stop to return from run according to elastic/logstash#3210

Depends on elastic/logstash-devutils#32 and elastic/logstash#3812

This implementation still has a problem that, during the hearbeat sleep, there is no way to abort the plugin run method. Therefore a very big interval_time can make this plugin hang the shutdown sequence
Maybe an option is to decide to do a Thread.kill if the thread is sleeping?

resolves #8

@purbon
Copy link

purbon commented Sep 15, 2015

It would also be helpful to handle this stop when Stud.interval get in a proper shutdown mechanism with wake up, isn't?

@purbon purbon self-assigned this Sep 15, 2015
@jsvd
Copy link
Member Author

jsvd commented Sep 18, 2015

@purbon plugin now properly aborts the stud interval 👍

@purbon
Copy link

purbon commented Sep 18, 2015

LGTM

@@ -67,4 +68,8 @@ def generate_message(sequence)
end
end

def stop
super
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for super anymore, right?

@jsvd jsvd force-pushed the shutdown_using_stop branch from 6216ef4 to 955fef5 Compare September 21, 2015 10:02
@jsvd
Copy link
Member Author

jsvd commented Sep 21, 2015

another review is needed for this, removed super

@untergeek
Copy link
Contributor

This is much prettier! LGTM

@elasticsearch-bot
Copy link

Merged sucessfully into master!

@jsvd jsvd closed this in 33cb2e2 Sep 21, 2015
@jsvd jsvd deleted the shutdown_using_stop branch September 22, 2015 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor shutdown sequence to use plugin.stop
5 participants