From f12cc324580a35c3e4377b4c963d9ef5bf3338d2 Mon Sep 17 00:00:00 2001 From: Brian Christiansen <brian@schedmd.com> Date: Fri, 14 Aug 2015 10:50:23 -0700 Subject: [PATCH] Refactor TRESBillingWeights to dynamically handle TRES types. --- doc/man/man5/slurm.conf.5 | 25 +--- slurm/slurm.h.in | 2 +- src/api/partition_info.c | 4 +- src/common/read_config.c | 63 +------- src/common/read_config.h | 13 +- src/common/slurm_protocol_pack.c | 2 +- .../multifactor/priority_multifactor.c | 137 +++++------------- src/slurmctld/partition_mgr.c | 2 +- src/slurmctld/read_config.c | 117 +++++---------- src/slurmctld/slurmctld.h | 18 ++- 10 files changed, 101 insertions(+), 282 deletions(-) diff --git a/doc/man/man5/slurm.conf.5 b/doc/man/man5/slurm.conf.5 index ce85835e889..109391b7385 100644 --- a/doc/man/man5/slurm.conf.5 +++ b/doc/man/man5/slurm.conf.5 @@ -4057,29 +4057,8 @@ will be used in calcuating the usage of a job. Billing weights are specified as a comma-separated list of \fI<TRES Type>\fR=\fI<TRES Billing Weight>\fR pairs. -Available TRES types include: -.RS -.RS -.TP 10 -\fBCPU\fP -Charge per allocated CPU. Charge rates other than 1.0 can be used to charge -more or less for CPUs in a partition that are considered to be more or less -capable than those in other partitions. -.TP -\fBMem\fP -Charge per allocated GB of memory -.TP -\fBNode\fP -Charge per allocated node -.TP -\fBGRES/<name>[:<type>]\fP -Charge per allocated GRES of type \fI<name>\fR -.TP -\fBLicense/<name>\fP -Charge per allocated license of type \fI<name>\fR -.RE - -The CPU billing weight defaults to 1.0 and all others to 0.0. +Any TRES Type is available for billing. Note that the memory is weighted per +gigabyte. By default the billing of TRES is calculated as the sum of all TRES types multiplied by their corresponding billing weight. For example, when a job is diff --git a/slurm/slurm.h.in b/slurm/slurm.h.in index b34971a31b9..485a6dd8403 100644 --- a/slurm/slurm.h.in +++ b/slurm/slurm.h.in @@ -1985,7 +1985,7 @@ typedef struct partition_info { char *allow_qos; /* comma delimited list of qos, * null indicates all */ char *alternate; /* name of alternate partition */ - char *billing_weights; /* per TRES billing weights string */ + char *billing_weights_str;/* per TRES billing weights string */ uint16_t cr_type; /* see CR_* values */ uint32_t def_mem_per_cpu; /* default MB memory per allocated CPU */ uint32_t default_time; /* minutes, NO_VAL or INFINITE */ diff --git a/src/api/partition_info.c b/src/api/partition_info.c index e43a8877f6c..9e88739e140 100644 --- a/src/api/partition_info.c +++ b/src/api/partition_info.c @@ -428,14 +428,14 @@ char *slurm_sprint_partition_info ( partition_info_t * part_ptr, } /****** Line 10 ******/ - if (part_ptr->billing_weights) { + if (part_ptr->billing_weights_str) { if (one_liner) xstrcat(out, " "); else xstrcat(out, "\n "); snprintf(tmp_line, sizeof(tmp_line), "TRESBillingWeights=%s", - part_ptr->billing_weights); + part_ptr->billing_weights_str); xstrcat(out, tmp_line); } diff --git a/src/common/read_config.c b/src/common/read_config.c index 2d78fc4a768..d3b33560a99 100644 --- a/src/common/read_config.c +++ b/src/common/read_config.c @@ -1138,11 +1138,11 @@ static int _parse_partitionname(void **dest, slurm_parser_enum_t type, if (!s_p_get_string(&p->alternate, "Alternate", tbl)) s_p_get_string(&p->alternate, "Alternate", dflt); - if (!s_p_get_string(&p->billing_weights, "TRESBillingWeights", - tbl) && - !s_p_get_string(&p->billing_weights, "TRESBillingWeights", - dflt)) - xfree(p->billing_weights); + if (!s_p_get_string(&p->billing_weights_str, + "TRESBillingWeights", tbl) && + !s_p_get_string(&p->billing_weights_str, + "TRESBillingWeights", dflt)) + xfree(p->billing_weights_str); if (!s_p_get_boolean(&p->default_flag, "Default", tbl) && !s_p_get_boolean(&p->default_flag, "Default", dflt)) @@ -4866,59 +4866,6 @@ extern int sort_key_pairs(void *v1, void *v2) return 0; } -extern void destroy_config_key_double_pair(void *object) -{ - config_key_double_pair_t *key_double_pair_ptr = - (config_key_double_pair_t *)object; - - if (key_double_pair_ptr) { - xfree(key_double_pair_ptr->name); - xfree(key_double_pair_ptr); - } -} - -extern void pack_config_key_double_pair(void *in, uint16_t rpc_version, - Buf buffer) -{ - config_key_double_pair_t *object = (config_key_double_pair_t *)in; - packstr(object->name, buffer); - packdouble(object->value, buffer); -} - -extern int unpack_config_key_double_pair(void **object, uint16_t rpc_version, - Buf buffer) -{ - uint32_t uint32_tmp; - config_key_double_pair_t *object_ptr = - xmalloc(sizeof(config_key_double_pair_t)); - - *object = object_ptr; - safe_unpackstr_xmalloc(&object_ptr->name, &uint32_tmp, buffer); - safe_unpackdouble(&object_ptr->value, buffer); - - return SLURM_SUCCESS; - -unpack_error: - destroy_config_key_double_pair(object_ptr); - *object = NULL; - return SLURM_ERROR; -} - -extern int sort_key_double_pairs(void *v1, void *v2) -{ - config_key_double_pair_t *key_a = *(config_key_double_pair_t **)v1; - config_key_double_pair_t *key_b = *(config_key_double_pair_t **)v2; - - int size_a = strcmp(key_a->name, key_b->name); - - if (size_a < 0) - return -1; - else if (size_a > 0) - return 1; - - return 0; -} - /* * Return the pathname of the extra .conf file */ diff --git a/src/common/read_config.h b/src/common/read_config.h index db799dcc29b..1b8afc183d0 100644 --- a/src/common/read_config.h +++ b/src/common/read_config.h @@ -251,7 +251,7 @@ typedef struct slurm_conf_partition { char *alternate; /* name of alternate partition */ uint16_t cr_type; /* Custom CR values for partition (supported * by select/cons_res plugin only) */ - char *billing_weights; /* per TRES billing weights */ + char *billing_weights_str;/* per TRES billing weights */ uint32_t def_mem_per_cpu; /* default MB memory per allocated CPU */ bool default_flag; /* Set if default partition */ uint32_t default_time; /* minutes or INFINITE */ @@ -293,11 +293,6 @@ typedef struct { char *value; } config_key_pair_t; -typedef struct { - char *name; - double value; -} config_key_double_pair_t; - /* Destroy a front_end record built by slurm_conf_frontend_array() */ extern void destroy_frontend(void *ptr); @@ -569,12 +564,6 @@ extern void pack_config_key_pair(void *in, uint16_t rpc_version, Buf buffer); extern int unpack_config_key_pair(void **object, uint16_t rpc_version, Buf buffer); extern int sort_key_pairs(void *v1, void *v2); -extern void destroy_config_key_double_pair(void *object); -extern void pack_config_key_double_pair(void *in, uint16_t rpc_version, - Buf buffer); -extern int unpack_config_key_double_pair(void **object, uint16_t rpc_version, - Buf buffer); -extern int sort_key_double_pairs(void *v1, void *v2); /* * Return the pathname of the extra .conf file * return value must be xfreed diff --git a/src/common/slurm_protocol_pack.c b/src/common/slurm_protocol_pack.c index 8541ab86710..8cf9e302f53 100644 --- a/src/common/slurm_protocol_pack.c +++ b/src/common/slurm_protocol_pack.c @@ -5307,7 +5307,7 @@ _unpack_partition_info_members(partition_info_t * part, Buf buffer, xfree(node_inx_str); node_inx_str = NULL; } - safe_unpackstr_xmalloc(&part->billing_weights, &uint32_tmp, + safe_unpackstr_xmalloc(&part->billing_weights_str, &uint32_tmp, buffer); } else if (protocol_version >= SLURM_14_03_PROTOCOL_VERSION) { diff --git a/src/plugins/priority/multifactor/priority_multifactor.c b/src/plugins/priority/multifactor/priority_multifactor.c index 0666537acd3..46291499ffc 100644 --- a/src/plugins/priority/multifactor/priority_multifactor.c +++ b/src/plugins/priority/multifactor/priority_multifactor.c @@ -681,14 +681,10 @@ static time_t _next_reset(uint16_t reset_period, time_t last_reset) */ static double _calc_billable_tres(struct job_record *job_ptr, time_t start_time) { - double to_bill = 0.0; - double bill_weight_mem_mb = 0.0; - double bill_cpus = 0.0, bill_mem = 0.0, bill_nodes = 0.0; + double to_bill_node = 0.0; + double to_bill_global = 0.0; struct part_record *part_ptr = job_ptr->part_ptr; - uint32_t total_memory = 0; - uint32_t gres_alloc_count = 0; - uint32_t lic_alloc_count = 0; - config_key_double_pair_t *bill_weight_pair; + tres_billing_weight_t *billing_weight; ListIterator itr; /* Don't recalculate unless the job is new or resized */ @@ -708,107 +704,50 @@ static double _calc_billable_tres(struct job_record *job_ptr, time_t start_time) if (priority_debug) info("BillingWeight: job %d using \"%s\" from partition %s", - job_ptr->job_id, part_ptr->billing_weights, + job_ptr->job_id, part_ptr->billing_weights_str, job_ptr->part_ptr->name); - /* Calculate total memory since it is stored either per node or cpu */ - if (job_ptr->details->pn_min_memory & MEM_PER_CPU) - total_memory = (job_ptr->details->pn_min_memory & - (~MEM_PER_CPU)) * job_ptr->total_cpus; - else - total_memory = job_ptr->details->pn_min_memory * - job_ptr->total_nodes; - - bill_weight_mem_mb = part_ptr->bill_weight_mem_gb / 1024.0l; - - bill_mem = (double)total_memory * bill_weight_mem_mb; - bill_cpus = (double)job_ptr->total_cpus * part_ptr->bill_weight_cpu; - bill_nodes = (double)job_ptr->total_nodes * part_ptr->bill_weight_node; - - to_bill = bill_cpus; - if (flags & PRIORITY_FLAGS_MAX_TRES) { - to_bill = MAX(to_bill, bill_nodes); - to_bill = MAX(to_bill, bill_mem); - } else { - to_bill += bill_nodes; - to_bill += bill_mem; - } - - /* - * Potential performance improvement: - * It would likely be lower overhead to iterate through allocated - * gres and maybe licenses first then compare to bill weights - * rather than iterate through all defined bill weights then compare to - * gres and licenses. Site-specific differences are likely. - * - * It may be very uncommon for per partition per license charges to - * exist but still have licenses requested per job. It is much more - * likely to define a gres charge but have many jobs that don't request - * gres. This means that the license code is probably fine for most - * sites but gres may be better or worse the current way depending - * on a site's configuration and job patterns. - */ - - /* Calculate for each gres */ - if(job_ptr->gres_list && !list_is_empty(job_ptr->gres_list)) { - itr = list_iterator_create(part_ptr->bill_weight_gres); - while ((bill_weight_pair = list_next(itr))) { - gres_alloc_count = - gres_get_value_by_type(job_ptr->gres_list, - bill_weight_pair->name); - if(gres_alloc_count > 0) { - double bill_gres = gres_alloc_count * - (double)bill_weight_pair->value; - - if (priority_debug) - info("BillingWeight: GRES:%s = %d * %f", - bill_weight_pair->name, - gres_alloc_count, - bill_weight_pair->value); - - if (flags & PRIORITY_FLAGS_MAX_TRES) - to_bill = MAX(to_bill, bill_gres); - else - to_bill += bill_gres; - } + itr = list_iterator_create(part_ptr->billing_weights); + while ((billing_weight = list_next(itr))) { + bool is_mem = false; + double tres_weight = billing_weight->weight; + char *tres_type = billing_weight->type; + char *tres_name = billing_weight->name; + double tres_value = + job_ptr->tres_alloc_cnt[billing_weight->tres_id]; + + if (!strcasecmp(tres_type, "mem")) { + is_mem = true; + tres_weight /= 1024; /* mem is weighted by gb. */ } - list_iterator_destroy(itr); - } - /* Calculate for each license */ - if(job_ptr->license_list && !list_is_empty(job_ptr->license_list)) { - itr = list_iterator_create(part_ptr->bill_weight_lic); - while ((bill_weight_pair = list_next(itr))) { - lic_alloc_count = license_get_total_cnt_from_list( - job_ptr->license_list, - bill_weight_pair->name); - if(lic_alloc_count > 0) { - if (priority_debug) - info("BillingWeight: Lic:%s = %d * %f", - bill_weight_pair->name, - lic_alloc_count, - bill_weight_pair->value); - to_bill += lic_alloc_count * - (double)bill_weight_pair->value; - } - } - list_iterator_destroy(itr); + if (priority_debug) + info("BillingWeight: %s%s%s = %f * %f", tres_type, + (tres_name) ? ":" : "", + (tres_name) ? tres_name : "", + tres_value, tres_weight); + + tres_value *= tres_weight; + + if ((flags & PRIORITY_FLAGS_MAX_TRES) && + ((is_mem) || + (!strcasecmp(tres_type, "cpu")) || + (!strcasecmp(tres_type, "gres")))) + to_bill_node = MAX(to_bill_node, tres_value); + else + to_bill_global += tres_value; } + list_iterator_destroy(itr); - if (priority_debug) { - info("BillingWeight: CPU = %d * %f", job_ptr->total_cpus, - part_ptr->bill_weight_cpu); - info("BillingWeight: Node = %d * %f", job_ptr->total_nodes, - part_ptr->bill_weight_node); - info("BillingWeight: Mem (MB) = %d * (%f/1024)", total_memory, - part_ptr->bill_weight_mem_gb); + job_ptr->billable_tres = to_bill_node + to_bill_global; + + if (priority_debug) info("BillingWeight: Job %d %s = %f", job_ptr->job_id, (flags & PRIORITY_FLAGS_MAX_TRES) ? "MAX(node TRES) + SUM(Global TRES)" : "SUM(TRES)", - to_bill); - } - job_ptr->billable_tres = to_bill; - return to_bill; + job_ptr->billable_tres); + + return job_ptr->billable_tres; } diff --git a/src/slurmctld/partition_mgr.c b/src/slurmctld/partition_mgr.c index 5a5161b4ad3..7c556c95f39 100644 --- a/src/slurmctld/partition_mgr.c +++ b/src/slurmctld/partition_mgr.c @@ -1144,7 +1144,7 @@ void pack_part(struct part_record *part_ptr, Buf buffer, packstr(part_ptr->deny_qos, buffer); packstr(part_ptr->nodes, buffer); pack_bit_fmt(part_ptr->node_bitmap, buffer); - packstr(part_ptr->billing_weights, buffer); + packstr(part_ptr->billing_weights_str, buffer); } else if (protocol_version >= SLURM_14_03_PROTOCOL_VERSION) { if (default_part_loc == part_ptr) part_ptr->flags |= PART_FLAG_DEFAULT; diff --git a/src/slurmctld/read_config.c b/src/slurmctld/read_config.c index d12790be615..afebb94264c 100644 --- a/src/slurmctld/read_config.c +++ b/src/slurmctld/read_config.c @@ -135,93 +135,59 @@ static int _compare_hostnames(struct node_record *old_node_table, int node_count); -static bool _is_configured_tres(char *type, char *name) +static int _get_tres_id(char *type, char *name) { - bool valid = false; slurmdb_tres_rec_t tres_rec; - memset(&tres_rec, 0, sizeof(slurmdb_tres_rec_t)); tres_rec.type = type; tres_rec.name = name; - if (assoc_mgr_find_tres_pos(&tres_rec, false) != -1) - valid = true; - - return valid; + return assoc_mgr_find_tres_pos(&tres_rec, false); } -/* Convert the value of a k=v pair to a double. Inputs are the k=v string and an - * integer offset representing the start of the value */ -static double _bill_weight_item_to_double(char *value) +extern void destroy_tres_billing_weight(void *object) { - double d; - errno = 0; + tres_billing_weight_t *item= (tres_billing_weight_t *)object; - d = strtod(value, NULL); - if(errno) - fatal("Unable to convert %s value to double in " - "TRESBillingWeights", value); - - return d; -} - -static void _bill_weight_item_add_to_list(List l, char *name, char *value) -{ - config_key_double_pair_t *pair = xmalloc( - sizeof(config_key_double_pair_t)); - - pair->name = xstrdup(name); - pair->value = _bill_weight_item_to_double(value); - - list_append(l, pair); + if (item) { + xfree(item->type); + xfree(item->name); + xfree(item); + } } -static void _bill_weight_item(struct part_record *p, char *item_str) +static void _billing_weight_item(struct part_record *p, char *item_str) { char *type = NULL, *value = NULL, *name = NULL; + int tres_id; + tres_billing_weight_t *item; if (!item_str) fatal("TRESBillingWeights item is null"); type = strtok_r(item_str, "=", &value); + if (strchr(type, '/')) + type = strtok_r(type, "/", &name); if (!value || !*value) fatal("\"%s\" is an invalid TRESBillingWeight entry", item_str); - if (!strcasecmp(type, "Mem")) { - if (!_is_configured_tres(type, NULL)) - goto invalid_tres; - p->bill_weight_mem_gb = _bill_weight_item_to_double(value); - } else if (!strcasecmp(type, "CPU")) { - if (!_is_configured_tres(type, NULL)) - goto invalid_tres; - p->bill_weight_cpu = _bill_weight_item_to_double(value); - } else if (!strcasecmp(type, "Node")) { - if (!_is_configured_tres(type, NULL)) - goto invalid_tres; - p->bill_weight_node = _bill_weight_item_to_double(value); - } else if (!strncasecmp(type, "GRES/", 5)) { - type = strtok_r(type, "/", &name); - if (!_is_configured_tres("gres", name)) - goto invalid_tres; - - _bill_weight_item_add_to_list(p->bill_weight_gres, name, value); - } else if (!strncasecmp(type, "License/", 8)) { - type = strtok_r(type, "/", &name); - if (!_is_configured_tres("license", name)) - goto invalid_tres; - - _bill_weight_item_add_to_list(p->bill_weight_lic, name, value); - } else { - goto invalid_tres; - } - return; + if ((tres_id = _get_tres_id(type, name)) == -1) + fatal("TRESBillingWeights '%s%s%s' is not a configured TRES " + "type. Please add the TRES Type to " + "\"AccountingStorageTRES\" in the slurm.conf", + type, (name) ? ":" : "", (name) ? name : ""); + + item = xmalloc(sizeof(tres_billing_weight_t)); + item->type = xstrdup(type); + item->name = xstrdup(name); + item->weight = strtod(value, NULL); + item->tres_id = tres_id; + if(errno) + fatal("Unable to convert %s value to double in " + "TRESBillingWeights", value); -invalid_tres: - fatal("TRESBillingWeights '%s%s%s' is not a configured TRES type. " - "Please add the TRES Type to " "\"AccountingStorageTRES\" in " - "the slurm.conf", - type, (name) ? ":" : "", (name) ? name : ""); + list_append(p->billing_weights, item); } static void _billing_weights(struct part_record *p, char *bill_weight_str) @@ -229,21 +195,13 @@ static void _billing_weights(struct part_record *p, char *bill_weight_str) char *tmp_str = xstrdup(bill_weight_str); char *token, *last = NULL; - p->bill_weight_cpu = 1.0; - p->bill_weight_mem_gb = 0.0; - p->bill_weight_node = 0.0; - - if(p->bill_weight_gres) - list_destroy(p->bill_weight_gres); - p->bill_weight_gres = list_create(destroy_config_key_double_pair); - - if(p->bill_weight_lic) - list_destroy(p->bill_weight_lic); - p->bill_weight_lic = list_create(destroy_config_key_double_pair); + if(p->billing_weights) + list_destroy(p->billing_weights); + p->billing_weights = list_create(destroy_tres_billing_weight); token = strtok_r(tmp_str, ",", &last); while (token) { - _bill_weight_item(p, token); + _billing_weight_item(p, token); token = strtok_r(NULL, ",", &last); } xfree(tmp_str); @@ -804,10 +762,11 @@ static int _build_single_partitionline_info(slurm_conf_partition_t *part) part_ptr->grace_time = part->grace_time; part_ptr->cr_type = part->cr_type; - if (part->billing_weights) { - xfree(part_ptr->billing_weights); - part_ptr->billing_weights = xstrdup(part->billing_weights); - _billing_weights(part_ptr, part_ptr->billing_weights); + if (part->billing_weights_str) { + xfree(part_ptr->billing_weights_str); + part_ptr->billing_weights_str = + xstrdup(part->billing_weights_str); + _billing_weights(part_ptr, part_ptr->billing_weights_str); } if (part->allow_accounts) { diff --git a/src/slurmctld/slurmctld.h b/src/slurmctld/slurmctld.h index 511bdf5fd3d..fb157edd23e 100644 --- a/src/slurmctld/slurmctld.h +++ b/src/slurmctld/slurmctld.h @@ -330,12 +330,8 @@ struct part_record { bitstr_t *allow_qos_bitstr; /* (DON'T PACK) assocaited with * char *allow_qos but used internally */ char *alternate; /* name of alternate partition */ - double bill_weight_cpu; /* bill weight per allocated CPU */ - List bill_weight_gres; /* list of per allocated GRES bill weights */ - List bill_weight_lic; /* list of per allocated license bill weights */ - double bill_weight_mem_gb;/* bill weight per allocated memory (GB) */ - double bill_weight_node;/* bill weight per allocated node */ - char *billing_weights; /* per TRES billing weight string */ + List billing_weights; /* list of TRES billing weights */ + char *billing_weights_str;/* per TRES billing weight string */ uint32_t def_mem_per_cpu; /* default MB memory per allocated CPU */ uint32_t default_time; /* minutes, NO_VAL or INFINITE */ char *deny_accounts; /* comma delimited list of denied accounts */ @@ -384,6 +380,13 @@ extern char *default_part_name; /* name of default partition */ extern struct part_record *default_part_loc; /* default partition ptr */ extern uint16_t part_max_priority; /* max priority in all partitions */ +typedef struct { + char *name; + char *type; + double weight; + int tres_id; +} tres_billing_weight_t; + /*****************************************************************************\ * RESERVATION parameters and data structures \*****************************************************************************/ @@ -2360,4 +2363,7 @@ extern void trace_job(struct job_record *, const char *, const char *); int waitpid_timeout(const char *, pid_t, int *, int); +/* Destroy tres_billing_weight_t */ +extern void destroy_tres_billing_weight(void *object); + #endif /* !_HAVE_SLURMCTLD_H */ -- GitLab