tobymao / saq

Simple Async Queues
https://saq-py.readthedocs.io/en/latest/
MIT License
576 stars 39 forks source link

cron job updating #116

Closed darinkishore closed 7 months ago

darinkishore commented 7 months ago

hi! i'm interested in and have a rough draft for a PR for updating already scheduled cron jobs with new times. is this something you'd be interested in/open to?

tobymao commented 7 months ago

so youd make a call to update an existing cron jobs schedule? the issue though is its not reflected in the code so would get lost on restart right?

tobymao commented 7 months ago

closing this issue because it’s not really an issue. but happy to continue the discussion and potentially open to receiving a pr once i learn a bit more.

darinkishore commented 7 months ago

hi!

i think you're right? but i'm unsure. let me explain a bit about the usecase?

it's for using SAQ through the litestar-saq library! so they have these things called queueConfigs

queue_configs=[
        QueueConfig(
            name="system-tasks",
            tasks=["app.domain.system.tasks.system_task", "app.domain.system.tasks.system_upkeep"],
            scheduled_tasks=[
                CronJob(
                    function="app.domain.system.tasks.system_upkeep",
                    unique=True,
                    cron="0 * * * *",
                    timeout=500,
                ),
            ],
        ),

definition: https://github.com/cofin/litestar-saq/blob/main/litestar_saq/config.py#L187

if you're curious about the SAQ wrappers, that's here: https://github.com/cofin/litestar-saq/blob/main/litestar_saq/base.py , but they're very lightweight and basically correspond to the SAQ classes.

so if I define a CronJob task via there, and decide i want to change it, would have to manually manage the task and figure it out on the SAQ end.

but if the SAQ workers compare the calculated next run time with the stored scheduled value (in redis) and then update it,

then on restart, whenever the schedule method is called, it'd add a check to see if the job exists against the job key. If there's a discrepancy between scheduled time and the calculated run time via the cron string, it'd update the scheduled value with a new scheduled value.

and in this case, by the time it does that, the SAQ CronJob will also have the new updated cron job string and everything should work out happily, assuming that we can rely on the updated/scheduled difference to be precise enough.

current issues with doing it this way are:

hope that wasn't too long winded, have been hyperfocusing on this problem for some reason for a couple hours.

tobymao commented 7 months ago

i’m a bit confused, let’s say in your code it’s run daily at utc. you use an api to change it to 5pm.

you decide to change your code to now schedule at 6pm, won’t it be stuck at 5pm and your code doesn’t matter? how do you figure which is the source of truth, the code or what’s stored in state?

darinkishore commented 7 months ago

oh. okay. i see what you mean.

i think the code should specify source of truth. redis state should always be updated to match however the code initializes the scheduled tasks.

so, when we re-initialize the worker with the new schedule, it identifies the change and updates the "scheduled" key in Redis accordingly. so if a task was originally set to run at 5pm and now we say 6pm, the system changes the redis values to reflect this new schedule.

we can find this out from difference between queued and scheduled and checking for discrepancies there

tobymao commented 7 months ago

if that’s the case i don’t see much value in this because as i said your in memory change gets wiped out when you reset the server.