fix: better job status queries for slurm executor

[dlaehnemann]

Description

This PR changes two things:

1. Previously, checking active_jobs was only initiated after sleeping for 1/max_status_checks_per_second seconds, which meant a minimum sleeping time of 30 seconds. This sleeping interval is now set to a fixed 30 seconds. However, one idea could also be to turn this into a new variable for the generic executor, and expose it to the user via a command-line argument -- it is also manually set for other executors (e.g. the google executor), and might require fine-tuning for specific workflows and/or clusters.
2. Previously, a max_status_checks_per_second > 1 specified by the user was set down to max_status_checks_per_second = 0.03 (no clue why there were different values in the comparison and the final setting) and the default value was also 0.03. This means that only one job was checked for its status every 30 seconds. For workflows with hundreds or thousands of short-running jobs, this meant that it could easily take hours for finished jobs to be registered as COMPLETED and until that point, the respective resources were not available for scheduling new jobs. The new default and maximum of max_status_checks_per_second = 2 considerably speeds this up, but stays well below the number of status checks that would be possible on the Slurm cluster I tested this on (which would be almost 20 for sacct and more than 50 for scontrol). Here, it would be useful if users on other Slurm clusters could check, whether scontrol is also always quicker for them, compared to sacct. If so, it would make sense to make scontrol the (cheaper) default (which also works for jobs that are not yet started), and sacct the fall-back (which also works for jobs that have already finished a while ago and might not be found by scontrol any more).

QC

• The PR contains a test case for the changes or the changes are already covered by an existing test case.
• The documentation (docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).

[github-actions]

Please format your code with black: black snakemake tests/*.py.

[dlaehnemann]

This is quick code to get average and standard deviation of sacct timings that this PR introduces (along with the respective scontrol timings) into the debugging output. If others could use it to also provide timings, this could help decide whether scontrol is the better default for checking the status of active_jobs.

[dlaehnemann]

I have just highlighted the timing code in the comments. Also, I have just asked on the slurm-users mailing list, whether scontrol is always expected to be quicker than sacct and whether scontrol would thus make a better default that puts less strain on the scheduler. We might want to wait for answer to that question, before moving along with this PR.

[wickberg]

From the slurmctld's perspective, sacct is great. Because it's querying the slurmdbd instead of the slurmctld. :)

Note that there are minor delays in propagating job status info to the slurmdbd, and you're probably better off getting this status from slurmctld in most instances.

I haven't looked in depth at what you're doing, so apologies if this doesn't line up well, but: a single 'scontrol show job' (or an equivalent squeue command with appropriate filtering options set) that grabs the entire job queue, rather than 10s to 1000s of individual 'scontrol show job ' calls is preferred from the slurmctld's perspective. The trade-offs are around central job lock contention, and a single RPC (although grabbing the entire queue state rather than the state of one individual job) vs. a slew of them is vastly preferred as it limits the lock contention.

[dlaehnemann]

Thanks already for the insights, and this confirms what I have also been hearing on the slurm-users mailing list. So from what I understand:

1. sacct is the better default command compared to scontrol when it comes to straining the slurm system.
2. Both sacct and scontrol should preferably be queried with one batch request with a list of all the jobids of interest, not one individual request for each jobid.

So I guess I'll use this PR to also address this, changing the code to do batch requests.

[dlaehnemann]

Just documenting some further context I gathered from the very helpful slurm-users mailing list:

1. For the suggested batch queries, it would make sense to have an adaptive interval in which they are done, something like the maximum of a fixed interval (say 30 or 45 seconds) and something like 5x the time that the previous batch query took. That way, whenever the system is not very responsive, we do not overstrain it further.
2. It might make sense to use one name for all of the jobs in one snakemake run, as this might speed up queries if they are done with this (single and unique) name. However, we then have to ensure that this job name is really unique for each snakemake run (a UUID?) and will have to still parse every job id from the respective output internally.
3. squeue might be an alternative as a fallback command, instead of scontrol, because it can handle job names. However, both seem to query slurmctld, and one should be careful not to overstrain slurmctld with queries. Hopefully, using it only as a fallback when sacct doesn't return anything, already helps reducing the strain a lot. I have usually only seen sacct not return very shortly after submission, probably because there is a lag before they show up in the slurmdbd database that sacct queries. So an initial waiting period before the first query at the start of a snakemake run, plus a reasonable minimum waiting time between batch queries, should further minimize the instances where snakemake has to even use the fallback command.

[wickberg]

For (3), note that the job name filtering for squeue is handled entirely within the client-command. It won't make a difference in terms of the load on slurmctld. (For sacct, the SlurmDBD does use the name as part of the underlying MySQL query to limit the returned results.)

Both squeue and 'scontrol show job' are using the same underlying API calls (slurm_load_jobs), so I'd encourage you to use whichever command is more convenient for you to parse and manipulate.

[dlaehnemann]

Many thanks again, @wickberg , that's useful info for the implementation.

For squeue and scontrol show job, it probably still makes sense to submit batch queries with either a name (that many jobs share) or a list of JobIds, right? Or would querying each JobId individually be the exact same amount of load, anyways? Such individual queries might keep the code for handling the JobIds that need the fallback call after sacct a bit simpler. But maybe I am also overthinking this, I should just jump in and try it some time this week!

[johanneskoester]

Awesome guys! I am very much looking forward to the outcome of this PR! Just keep going!

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
0.0% Duplication

[johanneskoester]

That was me being logged into the wrong account, sorry :-)