Ticket 9056 - scontrol show job jobid can now accept a coma separaed list of job-id
Summary: scontrol show job jobid can now accept a coma separaed list of job-id
Status: OPEN
Alias: None
Product: Slurm
Classification: Unclassified
Component: User Commands (show other tickets)
Version: 20.02.3
Hardware: Linux Linux
: C - Contributions
Assignee: Tim Wickberg
QA Contact:
URL:
Depends on:
Blocks:
 
Reported: 2020-05-15 03:46 MDT by Tazio Ceri
Modified: 2022-05-27 06:09 MDT (History)
1 user (show)

See Also:
Site: -Other-
Slinky Site: ---
Alineos Sites: ---
Atos/Eviden Sites: ---
Confidential Site: ---
Coreweave sites: ---
Cray Sites: ---
DS9 clusters: ---
Google sites: ---
HPCnow Sites: ---
HPE Sites: ---
IBM Sites: ---
NOAA SIte: ---
NoveTech Sites: ---
Nvidia HWinf-CS Sites: ---
OCF Sites: ---
Recursion Pharma Sites: ---
SFW Sites: ---
SNIC sites: ---
Tzag Elita Sites: ---
Linux Distro: ---
Machine Name:
CLE Version:
Version Fixed:
Target Release: ---
DevPrio: ---
Emory-Cloud Sites: ---

Attachments
Patch against slurm20 but it applies also to slurm19 (2.11 KB, patch)
2020-05-15 03:46 MDT, Tazio Ceri 		Details | Diff
Patch against slurm20 (6.25 KB, patch)
2020-06-15 01:43 MDT, Tazio Ceri 		Details | Diff
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this ticket.
Description Tazio Ceri 2020-05-15 03:46:18 MDT 
Created attachment 14256 [details]
Patch against slurm20 but it applies also to slurm19

This patch improves the performances of our software to retrieve information from jobs when there is a huge number of jobs, because otherwise we would have to call scontrol every time.
Comment 1 Tim Wickberg 2020-05-15 12:32:56 MDT 
Hi Tazio -

While I'm not necessarily against the idea of supporting a comma-separated list of jobids here, the implementation you've chosen still issues successive RPCs to load each and every job record from the slurmctld. If we were going to do this, I'd want to see further changes to use slurm_load_jobs() instead, and handle the filtering client-side. That'd be considerably faster than the approach proposed here.

I'm happy to review that if you'd like to submit such a patch, otherwise I'm marking this as resolved/wontfix at this time.

I'll also take the time to note that the output from 'scontrol show job' is not necessarily intended for downstream consumption as it does change release to release, and we'd recommend using 'squeue' with specific formatting options to obtain such data instead.

- Tim
Comment 2 Tazio Ceri 2020-05-22 04:55:28 MDT 
Hi Tim!

We want to build that patch, we are starting to work on it. 

Just to be sure that we are on the same page:
you would accept a patch that uses slurm_load_jobs inside scontrol_load_job and filters there the ids or would you prefer an entirely new function, to keep down cyclomatic complexity?
Comment 3 Tim Wickberg 2020-05-26 18:51:42 MDT 
(In reply to Tazio Ceri from comment #2)
> Hi Tim!
> 
> We want to build that patch, we are starting to work on it. 
> 
> Just to be sure that we are on the same page:
> you would accept a patch that uses slurm_load_jobs inside scontrol_load_job
> and filters there the ids or would you prefer an entirely new function, to
> keep down cyclomatic complexity?

I think it can happen within the existing scontrol_print_job() function without too much difficulty.
Comment 4 Tazio Ceri 2020-06-15 01:43:24 MDT 
Created attachment 14664 [details]
Patch against slurm20
Comment 5 Taras Shapovalov 2020-07-14 09:53:16 MDT 
Hi Tim, 

When do you have a plan to try the new patch?

Best regards,
Taras
Comment 6 Taras Shapovalov 2022-05-27 06:09:08 MDT 
What is the status of the patch?