From 0018cdf463751f9d6e9a7d05386fd31f7683d494 Mon Sep 17 00:00:00 2001 From: Danny Auble <da@schedmd.com> Date: Wed, 30 Apr 2014 16:04:17 -0700 Subject: [PATCH] Fix issue where user is requesting --acctg-freq=0 and no memory limits. --- NEWS | 1 + doc/man/man1/salloc.1 | 3 ++ doc/man/man1/sbatch.1 | 3 ++ doc/man/man1/srun.1 | 3 ++ src/common/slurm_acct_gather.c | 45 ++++++++++++++++++++++++++ src/common/slurm_acct_gather.h | 2 ++ src/slurmctld/job_mgr.c | 26 +++------------ src/slurmd/slurmstepd/slurmstepd_job.c | 28 ++-------------- 8 files changed, 64 insertions(+), 47 deletions(-) diff --git a/NEWS b/NEWS index a9789c968f1..998f549e778 100644 --- a/NEWS +++ b/NEWS @@ -29,6 +29,7 @@ documents those changes that are of interest to users and admins. the job's priority. This is needed to support job arrays better. -- Correct squeue command not to merge jobs with state pending and completing together. + -- Fix issue where user is requesting --acctg-freq=0 and no memory limits. * Changes in Slurm 14.03.1-2 ========================== diff --git a/doc/man/man1/salloc.1 b/doc/man/man1/salloc.1 index 191c6e2a31e..a0d95b54d49 100644 --- a/doc/man/man1/salloc.1 +++ b/doc/man/man1/salloc.1 @@ -62,6 +62,9 @@ may be specified. Supported datatypes are as follows: where \fI<interval>\fR is the task sampling interval in seconds for the jobacct_gather plugins and for task profiling by the acct_gather_profile plugin. +NOTE: This frequency is used to monitor memory usage. If memory limits +are enforced the highest frequency a user can request is what is configured in +the slurm.conf file. They can not turn it off (=0) either. .TP \fBenergy=\fI<interval>\fR where \fI<interval>\fR is the sampling interval in seconds diff --git a/doc/man/man1/sbatch.1 b/doc/man/man1/sbatch.1 index 00f95225964..fa7651c60f2 100644 --- a/doc/man/man1/sbatch.1 +++ b/doc/man/man1/sbatch.1 @@ -77,6 +77,9 @@ may be specified. Supported datatypes are as follows: where \fI<interval>\fR is the task sampling interval in seconds for the jobacct_gather plugins and for task profiling by the acct_gather_profile plugin. +NOTE: This frequency is used to monitor memory usage. If memory limits +are enforced the highest frequency a user can request is what is configured in +the slurm.conf file. They can not turn it off (=0) either. .TP \fBenergy=\fI<interval>\fR where \fI<interval>\fR is the sampling interval in seconds diff --git a/doc/man/man1/srun.1 b/doc/man/man1/srun.1 index 3227ce39e1c..d198bf4debc 100644 --- a/doc/man/man1/srun.1 +++ b/doc/man/man1/srun.1 @@ -46,6 +46,9 @@ may be specified. Supported datatypes are as follows: where \fI<interval>\fR is the task sampling interval in seconds for the jobacct_gather plugins and for task profiling by the acct_gather_profile plugin. +NOTE: This frequency is used to monitor memory usage. If memory limits +are enforced the highest frequency a user can request is what is configured in +the slurm.conf file. They can not turn it off (=0) either. .TP \fBenergy=\fI<interval>\fR where \fI<interval>\fR is the sampling interval in seconds diff --git a/src/common/slurm_acct_gather.c b/src/common/slurm_acct_gather.c index ad8799c282c..e15cd414216 100644 --- a/src/common/slurm_acct_gather.c +++ b/src/common/slurm_acct_gather.c @@ -197,6 +197,51 @@ extern int acct_gather_parse_freq(int type, char *freq) return freq_int; } +extern int acct_gather_check_acct_freq_task( + uint32_t job_mem_lim, char *acctg_freq) +{ + int task_freq; + static uint32_t acct_freq_task = NO_VAL; + + if (acct_freq_task == NO_VAL) { + char *acct_freq = slurm_get_jobacct_gather_freq(); + int i = acct_gather_parse_freq(PROFILE_TASK, acct_freq); + xfree(acct_freq); + + /* If the value is -1 lets set the freq to something + really high so we don't check this again. + */ + if (i == -1) + acct_freq_task = (uint16_t)NO_VAL; + else + acct_freq_task = i; + } + + if (!job_mem_lim || !acct_freq_task) + return 0; + + task_freq = acct_gather_parse_freq(PROFILE_TASK, acctg_freq); + + if (task_freq == -1) + return 0; + + if (task_freq == 0) { + error("Can't turn accounting frequency off. " + "We need it to monitor memory usage."); + slurm_seterrno(ESLURMD_INVALID_ACCT_FREQ); + return 1; + } else if (task_freq > acct_freq_task) { + error("Can't set frequency to %d, it is higher than %u. " + "We need it to be at least at this level to " + "monitor memory usage.", + task_freq, acct_freq_task); + slurm_seterrno(ESLURMD_INVALID_ACCT_FREQ); + return 1; + } + + return 0; +} + extern void acct_gather_suspend_poll(void) { acct_gather_suspended = true; diff --git a/src/common/slurm_acct_gather.h b/src/common/slurm_acct_gather.h index e17d84cb2a3..e6533a92a01 100644 --- a/src/common/slurm_acct_gather.h +++ b/src/common/slurm_acct_gather.h @@ -65,6 +65,8 @@ extern int acct_gather_conf_destroy(void); /* don't forget to free this */ extern List acct_gather_conf_values(void); extern int acct_gather_parse_freq(int type, char *freq); +extern int acct_gather_check_acct_freq_task( + uint32_t job_mem_lim, char *acctg_freq); extern void acct_gather_suspend_poll(void); extern void acct_gather_resume_poll(void); diff --git a/src/slurmctld/job_mgr.c b/src/slurmctld/job_mgr.c index 8519b9550e6..90b6e8b4bb2 100644 --- a/src/slurmctld/job_mgr.c +++ b/src/slurmctld/job_mgr.c @@ -4380,7 +4380,6 @@ static int _job_create(job_desc_msg_t * job_desc, int allocate, int will_run, char **err_msg) { static int launch_type_poe = -1; - static uint32_t acct_freq_task = NO_VAL; int error_code = SLURM_SUCCESS, i, qos_error; struct part_record *part_ptr = NULL; List part_ptr_list = NULL; @@ -4394,7 +4393,6 @@ static int _job_create(job_desc_msg_t * job_desc, int allocate, int will_run, static uint32_t node_scaling = 1; static uint32_t cpus_per_mp = 1; acct_policy_limit_set_t acct_policy_limit_set; - int acctg_freq; #ifdef HAVE_BG uint16_t geo[SYSTEM_DIMENSIONS]; @@ -4440,25 +4438,6 @@ static int _job_create(job_desc_msg_t * job_desc, int allocate, int will_run, if (error_code != SLURM_SUCCESS) return error_code; - /* Validate a job's accounting frequency, if specified */ - if (acct_freq_task == NO_VAL) { - char *acct_freq = slurm_get_jobacct_gather_freq(); - int i = acct_gather_parse_freq(PROFILE_TASK, acct_freq); - xfree(acct_freq); - if (i != -1) - acct_freq_task = i; - else - acct_freq_task = (uint16_t) NO_VAL; - } - acctg_freq = acct_gather_parse_freq(PROFILE_TASK, job_desc->acctg_freq); - if ((acctg_freq != -1) && - ((acctg_freq == 0) || (acctg_freq > acct_freq_task))) { - error("Invalid accounting frequency (%d > %u)", - acctg_freq, acct_freq_task); - error_code = ESLURMD_INVALID_ACCT_FREQ; - goto cleanup_fail; - } - /* insure that selected nodes are in this partition */ if (job_desc->req_nodes) { error_code = node_name2bitmap(job_desc->req_nodes, false, @@ -5989,6 +5968,11 @@ static int _validate_job_desc(job_desc_msg_t * job_desc_msg, int allocate, } else if (!_validate_min_mem_partition(job_desc_msg, part_ptr, part_list)) return ESLURM_INVALID_TASK_MEMORY; + /* Validate a job's accounting frequency, if specified */ + if (acct_gather_check_acct_freq_task( + job_desc_msg->pn_min_memory, job_desc_msg->acctg_freq)) + return ESLURMD_INVALID_ACCT_FREQ; + if (job_desc_msg->min_nodes == NO_VAL) job_desc_msg->min_nodes = 1; /* default node count of 1 */ if (job_desc_msg->min_cpus == NO_VAL) diff --git a/src/slurmd/slurmstepd/slurmstepd_job.c b/src/slurmd/slurmstepd/slurmstepd_job.c index d837e5c33b1..2fb824fea60 100644 --- a/src/slurmd/slurmstepd/slurmstepd_job.c +++ b/src/slurmd/slurmstepd/slurmstepd_job.c @@ -77,30 +77,6 @@ static void _job_init_task_info(stepd_step_rec_t *job, uint32_t **gtid, char *ifname, char *ofname, char *efname); static void _task_info_destroy(stepd_step_task_info_t *t, uint16_t multi_prog); -static int _check_acct_freq_task(uint32_t job_mem_lim, char *acctg_freq) -{ - int task_freq; - - if (!job_mem_lim || !conf->acct_freq_task) - return 0; - - task_freq = acct_gather_parse_freq(PROFILE_TASK, acctg_freq); - - if (task_freq == -1) - return 0; - - if ((task_freq == 0) || (task_freq > conf->acct_freq_task)) { - error("Can't set frequency to %d, it is higher than %u. " - "We need it to be at least at this level to " - "monitor memory usage.", - task_freq, conf->acct_freq_task); - slurm_seterrno (ESLURMD_INVALID_ACCT_FREQ); - return 1; - } - - return 0; -} - /* returns 0 if invalid gid, otherwise returns 1. Set gid with * correct gid if root launched job. Also set user_name * if not already set. */ @@ -318,7 +294,7 @@ stepd_step_rec_create(launch_tasks_request_msg_t *msg) if (!_valid_uid_gid((uid_t)msg->uid, &(msg->gid), &(msg->user_name))) return NULL; - if (_check_acct_freq_task(msg->job_mem_lim, msg->acctg_freq)) + if (acct_gather_check_acct_freq_task(msg->job_mem_lim, msg->acctg_freq)) return NULL; job = xmalloc(sizeof(stepd_step_rec_t)); @@ -491,7 +467,7 @@ batch_stepd_step_rec_create(batch_job_launch_msg_t *msg) if (!_valid_uid_gid((uid_t)msg->uid, &(msg->gid), &(msg->user_name))) return NULL; - if (_check_acct_freq_task(msg->job_mem, msg->acctg_freq)) + if (acct_gather_check_acct_freq_task(msg->job_mem, msg->acctg_freq)) return NULL; job = xmalloc(sizeof(stepd_step_rec_t)); -- GitLab