-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Added delayed enqueuing proposal. #3038
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
base: main
Are you sure you want to change the base?
Conversation
This was split out from the custom main and global executors proposal.
| /// - after: A `Duration` specifying the time after which the job | ||
| /// is to run. The job will not be executed before this | ||
| /// time has elapsed. | ||
| /// - tolerance: The maximum additional delay permissible before the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if the "impermissible" happens and the time has exceeded the tolerance given? Is the job then never run, or is it run as soon as possible? Is there some sort of error raised?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The behaviour I'd expect from an executor implementation is that it will try to run jobs before the deadline (e.g. a simple strategy might be to run the jobs in deadline order), but never before after, that job scheduling will be best effort and that all jobs will execute. So maybe "permissible" is the wrong word.
I can imagine that there might be a use-case for a hard deadline, for instance if you have a real-time scheduler, but I think we would want to add more enqueue methods to support that kind of behaviour (for example, I think you'd want an estimate of the time the job could run for, and you'd want the enqueue method to return a Bool to tell you whether it had actually been able to schedule the job). That's outside of the scope of this work though, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense--just want to make sure that this is sufficiently specified.
This was split out from the custom main and global executors proposal.