jupyter-scheduler icon indicating copy to clipboard operation
jupyter-scheduler copied to clipboard

task runner scheduling logic concerns

Open dlqqq opened this issue 3 years ago • 1 comments

    For clarity, I suggest we make the following changes:
  1. When comparing times, I suggest we always convert them to UTC first for clarity. This logic converts the current time to the task's specified time zone, which I think is much more confusing (although equivalent). This means we can delete get_localized_timestamp() and instead just convert task.next_run_time to UTC.
  2. It's very confusing that the default timezone (UTC) is set by TaskRunner.compute_time_diff() in a ternary statement. When setting default values for records, we should set them as "high up" as possible for visibility. I suggest making job definition timezones non-optional and enforcing the default value in either the JobDefinition table model or in the CreateJobDefinition request model.

Originally posted by @dlqqq in https://github.com/jupyter-server/jupyter-scheduler/pull/106#discussion_r990568083

dlqqq avatar Oct 11 '22 17:10 dlqqq

Thoughts I had while reviewing #253:

The bug documented in #253 partly originated from the fact that we're "overly eager" about removing tasks from the queue even when we don't need to. I believe the workaround I suggested in #253 could be removed if we change the body of the while loop in process_queue().

  • Call continue and pop the task from the queue if cache record doesn't exist
    • Needs to happen before the below line
    • Current implementation
  • Move the if time_diff < 0: break conditional as far up in the while loop as possible
  • Remove the elif time_diff >= (self.poll_interval * 1000): break block
  • Rest of the while loop continues as normal

dlqqq avatar Nov 01 '22 23:11 dlqqq