Skip to content
This repository has been archived by the owner on Sep 14, 2020. It is now read-only.

Distinguish "no handlers at all" and "no handers for now" cases #285

Merged
merged 2 commits into from
Jan 8, 2020

Conversation

nolar
Copy link
Contributor

@nolar nolar commented Jan 3, 2020

What do these changes do?

Fixes an issue with time discrepancies that stopped the reaction cycle if the operator was unlucky enough to hit the scheduled awakening time with 1ms precision.

Description

Previously, "now" of handler.awakened and "now" of state.delay were different (maybe for less than 1 ms). If the scheduled awakening time was unlucky to be between these two points in time, the operator stopped reacting on this object until any other events or changes arrive.

Specifically, the operator neither selected the handlers to be executed (because it was "1ms too early" for the handler.awakened), nor did it sleep (because it was "1ms too late" in the state.delay property) — with the same value of delayed field, just the code being executed exactly during this awakening time.

Due to how zero-time delay is treated (i.e. same as None — no sleep at all), the dummy patch was not applied to trigger the next steps of the reaction cycle.

After the fix, 0-delay and None-delay are treated differently: the zero-time delay does not sleep, but produces a dummy patch to trigger the reaction cycle after the sleep is over (as if it was an actual zero-time sleep). The None delay does not do any sleeping or dummy patching.

This eliminates the problem with the "now" time inconsistencies, while keeps the multiple datetime.datetime.utcnow() possible — to properly calculate the needed sleep time without the code overhead included.

The change does not affect the users directly. The probability of hitting this issue was low anyway.

Issues/PRs

Issues: closes #284

Type of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • The code addresses only the mentioned problem, and this problem only
  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt

@nolar nolar added the bug Something isn't working label Jan 3, 2020
@zincr
Copy link

zincr bot commented Jan 3, 2020

🤖 zincr found 0 problems , 0 warnings

✅ Large Commits
✅ Approvals
✅ Specification
✅ Dependency Licensing

@zincr
Copy link

zincr bot commented Jan 3, 2020

🤖 zincr found 1 problem , 0 warnings

❌ Approvals
✅ Large Commits
✅ Specification
✅ Dependency Licensing

Details on how to resolve are provided below


Approvals

All proposed changes must be reviewed by project maintainers before they can be merged

Not enough people have approved this pull request - please ensure that 1 additional user, who have not contributed to this pull request approve the changes.

  • ✅ Approved by PR author @nolar
  • ❌ 1 additional approval needed
     

@nolar nolar merged commit 1719184 into zalando-incubator:master Jan 8, 2020
@nolar nolar deleted the consistent-now-when-awakening branch January 8, 2020 10:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reaction cycle _sometimes_ just stops for no reason
2 participants