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

perf(scheduler): do not save offset value when every is provided #3142

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

roggervalf
Copy link
Collaborator

Why

  1. Why is this change necessary? Offset does not need to be saved as it can be generated using other values.

How

  1. How did you implement this? It can be generated using prevMillis and every

Additional Notes (Optional)

Any extra info here.

@manast
Copy link
Contributor

manast commented Mar 14, 2025

I am not sure this is correct. You must store offset because the offset is calculated in the first iteration and then you must use that offset from that time, otherwise the time between the first and second iteration will be incorrect. For example, say you have an every of 1 year. The first iteration will be immediately, and the second iteration should be after 1 year. But if you do not store the offset, then for example if you add the job the 31sth of December, it will process a job directly and then another one the 1st of January, not waiting a whole year.

@roggervalf
Copy link
Collaborator Author

I am not sure this is correct. You must store offset because the offset is calculated in the first iteration and then you must use that offset from that time, otherwise the time between the first and second iteration will be incorrect. For example, say you have an every of 1 year. The first iteration will be immediately, and the second iteration should be after 1 year. But if you do not store the offset, then for example if you add the job the 31sth of December, it will process a job directly and then another one the 1st of January, not waiting a whole year.

I could verify that delay when adding next iteration is respected with every value. Also I added a new test case where I can validate that this slot period is respected when moving a job to active happens some time after the expected delay. In that case next iteration still need to happen after the delay value. With current implementation as offset is only generated the first time, test fail

@manast
Copy link
Contributor

manast commented Mar 17, 2025

But if you do not save the offset, how can you know which delay to apply to the next iteration job?

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