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

interval: only sleep off remainder #100

Merged
merged 2 commits into from
Sep 15, 2021

Conversation

yaauie
Copy link
Contributor

@yaauie yaauie commented Sep 14, 2021

Attempt to adhere to the interval, regardless of how long execution takes.
After executing, sleep off only the remainder of the interval (if any), and
log a helpful warning when execution takes longer than the interval.

Release notes

Fixes issues related to fetching SNMP data at a periodic interval. The plugin now only sleeps off the remainder of the interval between runs, and logs a helpful warning when execution takes longer than the configured interval.

What does this PR do?

Introduces and tests a new StoppableIntervalRunner, which is used to run the crawler at a periodic interval. The implementation respects the stop-status of the plugin, and uses the plugin's logger to warn when execution takes longer than the configured interval.

Why is it important/What is the impact to the user?

Ensures that the interval option supports actually collecting SNMP data at that periodic interval. When this plugin is configured with a 5-minute interval (interval => 300), and a number of hosts that take anywhere from 3-seconds to 1-minute to fetch data from, this ensures that the start of each crawl is correctly 5-minutes apart.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • [ ] I have made corresponding change to the default configuration files (and/or docker env variables)
  • I have added tests that prove my fix is effective or that my feature works

Related issues

Resolves: #61

Logs

When the interval cannot be satisfied, a helpful log message is emitted:

[2021-09-14T17:54:05,585][WARN ][logstash.input.snmp    ][my-snmp-pipeline] polling hosts took longer than the configured interval {:interval_seconds=>30, :duration_seconds=>37.019}

Attempt to adhere to the interval, regardless of how long execution takes.
After executing, sleep off only the _remainder_ of the interval (if any), and
log a helpful warning when execution takes longer than the interval.

Resolves: logstash-plugins#61
@yaauie yaauie added the bug Something isn't working label Sep 14, 2021
@andsel andsel self-requested a review September 15, 2021 07:34
Copy link
Contributor

@andsel andsel left a comment

Choose a reason for hiding this comment

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

Nice PR

LGTM

@yaauie yaauie merged commit a435eb7 into logstash-plugins:master Sep 15, 2021
@yaauie yaauie deleted the interval-improvements branch September 15, 2021 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Interval weirdness
2 participants