Skip to content

Conversation

@kares
Copy link
Contributor

@kares kares commented Jun 14, 2022

Normally, with up-to-date rufus-scheduler, EoTime.now happens on Scheduler.new ... (from start).

The problem is the mixin lazily initializes the scheduler on first use and thus the actual start of the scheduler might take a considerable time (esp. on CI) - this is problematic in terms of testing behavior that is using the scheduler (and has been a cause of several 🔴 false positives in plugins).

The code itself is extracted from http_poller and the change immediately stabilized the CI build, since we do not want to have scheduler impl specific code all over the plugin I am moving the eager initialization into the mixin.


This is not a concern for currently used rufus 3.0 (which simply uses Time.now), thus the LoadError rescue. Once LS core updates the scheduler to >= 3.4 we could remove the rescue.

@kares kares self-assigned this Jun 14, 2022
scheduler.cron('* * * * * *') { counter.increment_and_get; sleep 3.25 } # every second
scheduler.cron('* * * * * *') do # every second
counter.increment_and_get
sleep_at_least 3.25
Copy link
Contributor Author

Choose a reason for hiding this comment

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

spec was failing occasionally (unrelated to the eager ::EtOrbi::EoTime.now above),
this + the lower frequency makes it more predictable

Copy link

@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.

LGTM

@kares kares merged commit 9b61224 into logstash-plugins:main Jun 14, 2022
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

Successfully merging this pull request may close these issues.

2 participants