mllg / batchtools

Tools for computation on batch systems
https://mllg.github.io/batchtools/
GNU Lesser General Public License v3.0
171 stars 51 forks source link

More flexible Job listing/killing for Slurm #173

Open ja-thomas opened 6 years ago

ja-thomas commented 6 years ago

We frequently change clusters/partition on our HPC and setting the clusters in makeClusterFunctionSlurm() is not really practical (multiple users are linking to the same template/config file).

So we handle the clusters/partitions via resources. To make listing/killing of the jobs possible we need to set the squeue arguments in the functions accordingly.

This is not really nice, but I can't think of another solution. As far as I know you can't change the clusters argument of makeClusterFunctionSlurm not after or while creating the registry.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 93.744% when pulling eda37eaee86ef85f234d1d510bff99be89790dfd on ja-thomas:master into 5c008ee9c8ef54bac0be2e373da74147d51ee657 on mllg:master.

mllg commented 6 years ago

Can you call squeue without specifying --clusters? Can there be duplicated job.ids if you query multiple clusters?

ja-thomas commented 6 years ago

no, squeue without the clusters argument will always return no jobs (the partition argument is optional and only used if a specific partition is actually used, which means there is a default partition but no default cluster).

There are no duplicated job ids as far as I know. I can double check, but until now the job.id was always a unique identifier over all clusters

mllg commented 6 years ago

How about this approach:

mllg commented 6 years ago

Ah wait, this does not work for killJobs() 😞

berndbischl commented 6 years ago

@mllg why dont you simply expose the "args" from listJobs in the constructor, with your settings as default?

and then users can overwrite this flexibly? isnt that the normal trick? and this changes nothing for anybody else or the internal code?

berndbischl commented 6 years ago

this here:

 listJobsQueued = function(reg) {
 args = c("-h", "-o %i", "-u $USER", "-t PD", sprintf("--clusters=%s", clusters))

just expose this as args.listjobsqueued (or whatever), with the string as a default?

mllg commented 6 years ago

https://github.com/mllg/batchtools/pull/179

Can you please comment if

  1. This is now flexible enough for you guys
  2. if we still need the clusters argument
  3. if this PR is now obsolete
ja-thomas commented 6 years ago

1) I don't think this helps. The problem is that the args are evaluated (at least when we have them optional with sprintf) at creation time of the clusterFunctions and not when they are actually called.

2) I hope we don't need the clusters argument anymore if we get that to work.

3) I think I'll take this rather ugly fix here and keep them as clusterFunctionsSlurLRZ or whatever in the config repository for our project on the lrz. Since all cluster users are linking against my batchtools.conf file anyways...

The perfect solution for us would be that clusters + partitions are resources that can be set on a job level (which is already possible, I think) and have the listing/killing calls take the values from there

mllg commented 6 years ago

180 ?

mllg commented 6 years ago

I could do the same thing for partitions, but I really don't know what I'm doing. :confused:

lcomm commented 6 years ago

This does not solve the original cluster/partition issue, but @berndbischl's suggestion of exposing the arguments would solve a problem I am encountering where all of my SLURM jobs show up as expired until done. My computing cluster has its own version of squeue (see here), but it only recognizes --noheader and not -h as assumed by the listJobs functions. Allowing users to tweak the listJobsQueued and listJobsRunning args would make it easier to use with nonstandard SLURM configurations.

mllg commented 6 years ago

@lcomm The arguments will be exported in the next version of batchtools, I'm just waiting for some feedback on #180 before exposing the args.

@ja-thomas @berndbischl

mllg commented 6 years ago

Ok it looks like --no-header is supported now by rc-squeue. I've changed the Slurm cluster functions to always use the longer command line arguments nevertheless.