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

Astro: Schedule Jobs 30sec after midnight to ensure to be on the next… #4131

Merged
merged 1 commit into from Aug 31, 2017

Conversation

Projects
None yet
4 participants
@triller-telekom
Copy link
Contributor

commented Aug 25, 2017

… day

Fixes #4018

Signed-off-by: Stefan Triller stefan.triller@telekom.de

Astro: Schedule Jobs 30sec after midnight to ensure to be on the next…
… day

Signed-off-by: Stefan Triller <stefan.triller@telekom.de>
@maggu2810

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2017

Can you please give me some more details why 0h 0m 30s? is on the next day and 0h 0m 0s isn't?

@triller-telekom

This comment has been minimized.

Copy link
Contributor Author

commented Aug 28, 2017

The problem is that our scheduler calculates the milliseconds until the next execution and in the cases of the bug that I am fixing the scheduler and the system time are a few (milli)seconds off and thus the expression scheduled to be executed at 0h 0m 0s is executed before midnight which is the day before. And in case of the Astro binding we are only interested in the day and not not the actual time of the cron execution.

@maggu2810

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2017

the expression scheduled to be executed at 0h 0m 0s is executed before midnight which is the day before

Hm, it seems to me that the scheduler is then buggy and not the astro binding.
Isn't the normal behavior that the execution is at the given time or a little bit later.
If a cron expression that is using "00:00:00" as execution trigger is executed on the old day, I would say this is an unexpected behavior.

Changing the trigger cron expression will mask the error but not really solve the bug.

@triller-telekom

This comment has been minimized.

Copy link
Contributor Author

commented Aug 28, 2017

Indeed you are right. But there a quite a few bug reports regarding the random skipping of Astro events here and in the openHAB forum so the idea was to have a quick fix for the next ESH stable and then fix the scheduler. Because fixing the scheduler will take longer.

@kaikreuzer

This comment has been minimized.

Copy link
Member

commented Aug 31, 2017

As a temporary workaround fix, this looks ok for me - the "real" fix will have to address #4145 then.
@maggu2810 If you are fine with it as well, feel free to merge.

@maggu2810

This comment has been minimized.

Copy link
Contributor

commented Aug 31, 2017

Can we create an issue to revert that change after #4145 has been fixed?
It seems to be wrong that a cron expression using the name "DAILY_MIDNIGHT" has an offset of 30 seconds.

@triller-telekom

This comment has been minimized.

Copy link
Contributor Author

commented Aug 31, 2017

Done: #4167

@maggu2810 maggu2810 merged commit ee1f590 into eclipse:master Aug 31, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
ip-validation
Details

@triller-telekom triller-telekom deleted the triller-telekom:fixAstroMidnight branch Aug 31, 2017

@sjka sjka added the Binding-Astro label Sep 6, 2017

@kaikreuzer kaikreuzer added this to the 0.9.0 milestone Nov 30, 2017

@kaikreuzer kaikreuzer added the bug label Dec 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.