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

Feature/add better scheduling via croniter #473

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

Conversation

om-henners
Copy link
Contributor

Add a column to set a crontab expression for resource checks.

The current run_frequency chooses a random start time within the next run_frequency minutes as the start time and the repeats at even intervals. In most cases this is all that's required but in others it would be nice to have more control over how the service runs. This adds an alternative to the current method of setting the frequency on tests runs by adding a column to set a crontab expression to determine the timing of probes.

Wasn't sure where (or if) adding tests is required, but I'm happy to do so.

Henry Walshaw added 7 commits August 28, 2024 16:14
The goal is that the user can optionally store a cron value against a
Resource to set the exact time a particular probe is run rather than
the frequency with a random starting point.
If the resource has a ``cron`` set then use it to determine the
trigger for the job rather than the ``run_frequency`` (which is
ignored).
``Resource.run_frequency`` and ``ResourceLock.resource_identifier``
are not nullable according to the migrations, so that's now reflected
in the model code.
@justb4
Copy link
Member

justb4 commented Aug 29, 2024

@om-henners thank you for this contribution! In the current scheme the random start time was deliberately implemented to prevent an "all at same time processing intensity" which occurred in first versions. Like "Soldiers are asked to break steps while crossing the bridge". I am not sure if that effect can occur when using cron patterns. Depends strongly on the patterns.

Do you have use cases?

There have been requests (cannot find the issue now) to schedule per Probe i.s.o. per endpoint/Resource, for example to do a heavy, thorough Probe once a day and lighter weight ones every N minutes. Would that cover your requirements?

(We are a bit understaffed, so bear with us...)

@om-henners
Copy link
Contributor Author

@justb4 No worries! It's a great tool we use all the time so we're definitely keen to help :)

Our case is similar to the one you describe - it would be useful to run a particular probe at specific times. Maybe we're doing things about outside the expectation but we tend to keep the number of probes per resource down to a minimum but there are probes that we want to guarantee run at particular points rather than it being dependent on server startup.

In our case we use geohealthcheck as the final check to make sure data we're serving to our clients is available. We want to know this data is not available as soon as possible if an upstream job has failed for whatever reason (and somehow missed other alarms). So for example if our data production job runs and finished at 12:30 every day. To check the data is a relatively heavy probe so we don't want to run it too frequently. And really if the job runs once a day then we only need to check once that the data is there. But if we set the frequency to 24 hours then there's a good chance we'll end up waiting a big chunk of time to know the data is missing. On the other hand if we wanted to guarantee we'd know the data was missing within (say) 10 minutes so we can do something about it then we have to run the probe once every 10 minutes. On the other hand if we have a method of setting the schedule (like cron) then we can set the check to run once a day at 12:31 and get an answer to whether the data is available.

In terms of the method of setting this time, I'm not marries to cron patterns by any stretch, but I'm familiar with them already and they're supported out of the box in APScheduler so I figured it was an easy choice. The equivalent would just be a field that stored a start time to determine frequency from rather than the random start. But I felt that wasn't as flexible.

In most of our general probes the random start time is perfectly fine. The goal with the PR is that this shouldn't change which is why the cron attribute is optional - if it's not set the run_frequency should set the random start time as before.

@Dukestep
Copy link

Any updates getting this MR merged? The functionality described here aligns closely to what we would like to have for some of our resources as well!

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.

3 participants