From 6e2312b4017480715d4806abdc7170c185942cc2 Mon Sep 17 00:00:00 2001 From: cmoussa1 Date: Thu, 14 Mar 2024 08:04:45 -0700 Subject: [PATCH 1/2] plugin: move add_missing_bank_info () Problem: The add_missing_bank_info () function is defined in the "callbacks" section of the plugin code when it is really a helper function. Move this function up where the other helper functions are defined. --- src/plugins/mf_priority.cpp | 54 ++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/src/plugins/mf_priority.cpp b/src/plugins/mf_priority.cpp index 13fde11c..f9cb35b8 100644 --- a/src/plugins/mf_priority.cpp +++ b/src/plugins/mf_priority.cpp @@ -132,6 +132,35 @@ static int update_jobspec_bank (flux_plugin_t *p, int userid) } +/* + * Create a special Association object for an association's job while the + * plugin waits for flux-accounting data to be loaded. + */ +static void add_missing_bank_info (flux_plugin_t *p, flux_t *h, int userid) +{ + Association *b; + + b = &users[userid]["DNE"]; + users_def_bank[userid] = "DNE"; + + b->bank_name = "DNE"; + b->fairshare = 0.1; + b->max_run_jobs = BANK_INFO_MISSING; + b->cur_run_jobs = 0; + b->max_active_jobs = 1000; + b->cur_active_jobs = 0; + b->active = 1; + b->held_jobs = std::vector(); + + if (flux_jobtap_job_aux_set (p, + FLUX_JOBTAP_CURRENT_JOB, + "mf_priority:bank_info", + b, + NULL) < 0) + flux_log_error (h, "flux_jobtap_job_aux_set"); +} + + /****************************************************************************** * * * Callbacks * @@ -436,31 +465,6 @@ static int priority_cb (flux_plugin_t *p, } -static void add_missing_bank_info (flux_plugin_t *p, flux_t *h, int userid) -{ - Association *b; - - b = &users[userid]["DNE"]; - users_def_bank[userid] = "DNE"; - - b->bank_name = "DNE"; - b->fairshare = 0.1; - b->max_run_jobs = BANK_INFO_MISSING; - b->cur_run_jobs = 0; - b->max_active_jobs = 1000; - b->cur_active_jobs = 0; - b->active = 1; - b->held_jobs = std::vector(); - - if (flux_jobtap_job_aux_set (p, - FLUX_JOBTAP_CURRENT_JOB, - "mf_priority:bank_info", - b, - NULL) < 0) - flux_log_error (h, "flux_jobtap_job_aux_set"); -} - - /* * Perform basic validation of a user/bank's submitted job. If a bank or * queue is specified on submission, ensure that the user is allowed to From 8ff3c85c536e1337fd6f805919219f22b19aad6b Mon Sep 17 00:00:00 2001 From: cmoussa1 Date: Thu, 14 Mar 2024 08:14:05 -0700 Subject: [PATCH 2/2] plugin: rename add_missing_bank_info () Problem: The add_missing_bank_info () function is poorly named and not very reflective of what the function is actually doing. Rename the function add_special_association (). Edit the calls to this function to reflect the function name change. Change the name of the Association variable in the function from "b" to "a". --- src/plugins/mf_priority.cpp | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/plugins/mf_priority.cpp b/src/plugins/mf_priority.cpp index f9cb35b8..38971d0c 100644 --- a/src/plugins/mf_priority.cpp +++ b/src/plugins/mf_priority.cpp @@ -136,26 +136,26 @@ static int update_jobspec_bank (flux_plugin_t *p, int userid) * Create a special Association object for an association's job while the * plugin waits for flux-accounting data to be loaded. */ -static void add_missing_bank_info (flux_plugin_t *p, flux_t *h, int userid) +static void add_special_association (flux_plugin_t *p, flux_t *h, int userid) { - Association *b; + Association *a; - b = &users[userid]["DNE"]; + a = &users[userid]["DNE"]; users_def_bank[userid] = "DNE"; - b->bank_name = "DNE"; - b->fairshare = 0.1; - b->max_run_jobs = BANK_INFO_MISSING; - b->cur_run_jobs = 0; - b->max_active_jobs = 1000; - b->cur_active_jobs = 0; - b->active = 1; - b->held_jobs = std::vector(); + a->bank_name = "DNE"; + a->fairshare = 0.1; + a->max_run_jobs = BANK_INFO_MISSING; + a->cur_run_jobs = 0; + a->max_active_jobs = 1000; + a->cur_active_jobs = 0; + a->active = 1; + a->held_jobs = std::vector(); if (flux_jobtap_job_aux_set (p, FLUX_JOBTAP_CURRENT_JOB, "mf_priority:bank_info", - b, + a, NULL) < 0) flux_log_error (h, "flux_jobtap_job_aux_set"); } @@ -519,7 +519,7 @@ static int validate_cb (flux_plugin_t *p, bool only_dne_data = check_map_for_dne_only (users, users_def_bank); if (users.empty () || only_dne_data) { - add_missing_bank_info (p, h, userid); + add_special_association (p, h, userid); return 0; } else { return flux_jobtap_reject_job (p, @@ -599,7 +599,7 @@ static int new_cb (flux_plugin_t *p, if (b == nullptr) { // the association could not be found in internal map, so create a // special Association object that will hold the job in PRIORITY - add_missing_bank_info (p, h, userid); + add_special_association (p, h, userid); return 0; }