From 65a90ba03d2803457adbb175535750d0ecad70be Mon Sep 17 00:00:00 2001
From: Ryan Cox <ryan_cox@byu.edu>
Date: Thu, 4 Dec 2014 13:38:07 -0800
Subject: [PATCH] Update code for the new fairshare algorithm.
---
src/plugins/priority/multifactor/fair_tree.c | 110 +++++++++++++------
testsuite/expect/test24.4 | 2 +-
2 files changed, 77 insertions(+), 35 deletions(-)
diff --git a/src/plugins/priority/multifactor/fair_tree.c b/src/plugins/priority/multifactor/fair_tree.c
index dfe16524d3d..8a66404c7cc 100644
--- a/src/plugins/priority/multifactor/fair_tree.c
+++ b/src/plugins/priority/multifactor/fair_tree.c
@@ -208,34 +208,52 @@ static void _calc_assoc_fs(slurmdb_association_rec_t *assoc)
}
-static slurmdb_association_rec_t** _append_children_to_array(
+/* Append list of associations to array
+ * IN list - list of associations
+ * IN merged - array of associations to append to
+ * IN/OUT merged_size - number of associations in merged array
+ * RET - New array. Must be freed.
+ */
+static slurmdb_association_rec_t** _append_list_to_array(
List list, slurmdb_association_rec_t** merged,
- size_t *child_count)
+ size_t *merged_size)
{
ListIterator itr;
slurmdb_association_rec_t *next;
- size_t i = *child_count;
- *child_count += list_count(list);
+ size_t bytes;
+ size_t i = *merged_size;
+ *merged_size += list_count(list);
- merged = xrealloc(merged, sizeof(slurmdb_association_rec_t*)
- * (*child_count + 1));
+ /* must be null-terminated, so add one extra slot */
+ bytes = sizeof(slurmdb_association_rec_t*) * (*merged_size + 1);
+ merged = xrealloc(merged, bytes);
itr = list_iterator_create(list);
while ((next = list_next(itr)))
merged[i++] = next;
list_iterator_destroy(itr);
+ /* null terminate the array */
+ merged[*merged_size] = NULL;
return merged;
}
+/* Returns number of tied sibling accounts.
+ * IN assocs - array of siblings, sorted by level_fs
+ * IN begin_ndx - begin looking for ties at this index
+ * RET - number of sibling accounts with equal level_fs values
+ */
static size_t _count_tied_accounts(slurmdb_association_rec_t** assocs,
- size_t i)
+ size_t begin_ndx)
{
slurmdb_association_rec_t* next_assoc;
- slurmdb_association_rec_t* assoc = assocs[i];
+ slurmdb_association_rec_t* assoc = assocs[begin_ndx];
+ size_t i = begin_ndx;
size_t tied_accounts = 0;
while ((next_assoc = assocs[++i])) {
+ /* Users are sorted to the left of accounts, so no user we
+ * encounter here will be equal to this account */
if (!next_assoc->user)
break;
if (assoc->usage->level_fs != next_assoc->usage->level_fs)
@@ -246,12 +264,20 @@ static size_t _count_tied_accounts(slurmdb_association_rec_t** assocs,
}
+/* Copy the children of accounts [begin, end] into a single array.
+ * IN siblings - array of siblings, sorted by level_fs
+ * IN begin - index of first account to merge
+ * IN end - index of last account to merge
+ * IN assoc_level - depth in the tree (root is 0)
+ * RET - Array of the children. Must be freed.
+ */
static slurmdb_association_rec_t** _merge_accounts(
slurmdb_association_rec_t** siblings,
size_t begin, size_t end, uint16_t assoc_level)
{
size_t i;
- size_t child_count = 0;
+ /* number of associations in merged array */
+ size_t merged_size = 0;
/* merged is a null terminated array */
slurmdb_association_rec_t** merged = (slurmdb_association_rec_t **)
xmalloc(sizeof(slurmdb_association_rec_t *));
@@ -260,6 +286,7 @@ static slurmdb_association_rec_t** _merge_accounts(
for (i = begin; i <= end; i++) {
List children = siblings[i]->usage->children_list;
+ /* the first account's debug was already printed */
if (priority_debug && i > begin)
_ft_debug(siblings[i], assoc_level, true);
@@ -267,8 +294,7 @@ static slurmdb_association_rec_t** _merge_accounts(
continue;
}
- merged = _append_children_to_array(children, merged,
- &child_count);
+ merged = _append_list_to_array(children, merged, &merged_size);
}
return merged;
}
@@ -279,61 +305,77 @@ static slurmdb_association_rec_t** _merge_accounts(
* This portion of the tree is now sorted and users are given a fairshare value
* based on the order they are operated on. The basic equation is
* (rank / g_user_assoc_count), though ties are allowed. The rank is decremented
- * for each user that is encountered.
+ * for each user that is encountered except when ties occur.
+ *
+ * Tie Handling Rules:
+ * 1) Sibling users with the same level_fs receive the same rank
+ * 2) Sibling accounts with the same level_fs have their children lists
+ * merged before sorting
+ * 3) A user with the same level_fs as a sibling account will receive
+ * the same rank as the account's highest ranked user
+ *
+ * IN siblings - array of siblings
+ * IN assoc_level - depth in the tree (root is 0)
+ * IN/OUT rank - current user ranking, starting at g_user_assoc_count
+ * IN/OUT rnt - rank, no ties (what rank would be if no tie exists)
+ * IN account_tied - is this account tied with the previous user
*/
static void _calc_tree_fs(slurmdb_association_rec_t** siblings,
- uint16_t assoc_level, uint32_t *rank, uint32_t *i,
- bool account_tied)
+ uint16_t assoc_level, uint32_t *rank,
+ uint32_t *rnt, bool account_tied)
{
slurmdb_association_rec_t *assoc = NULL;
long double prev_level_fs = (long double) NO_VAL;
bool tied = false;
- size_t ndx;
+ size_t i;
/* Calculate level_fs for each child */
- for (ndx = 0; (assoc = siblings[ndx]); ndx++)
+ for (i = 0; (assoc = siblings[i]); i++)
_calc_assoc_fs(assoc);
/* Sort children by level_fs */
- qsort(siblings, ndx, sizeof(slurmdb_association_rec_t *),
- _cmp_level_fs);
+ qsort(siblings, i, sizeof(slurmdb_association_rec_t *), _cmp_level_fs);
/* Iterate through children in sorted order. If it's a user, calculate
* fs_factor, otherwise recurse. */
- for (ndx = 0; (assoc = siblings[ndx]); ndx++) {
- if (account_tied) {
+ for (i = 0; (assoc = siblings[i]); i++) {
+ /* tied is used while iterating across siblings.
+ * account_tied preserves ties while recursing */
+ if (i == 0 && account_tied) {
+ /* The parent was tied so this level starts out tied */
tied = true;
- account_tied = false;
} else {
tied = prev_level_fs == assoc->usage->level_fs;
}
if (priority_debug)
_ft_debug(assoc, assoc_level, tied);
+
+ /* If user, set their final fairshare factor and handle ranking.
+ * If account, merge any tied accounts then recurse with the
+ * merged children array. */
if (assoc->user) {
if (!tied)
- *rank = *i;
+ *rank = *rnt;
- /* Set the final fairshare factor for this user */
assoc->usage->fs_factor =
*rank / (double) g_user_assoc_count;
- (*i)--;
+
+ (*rnt)--;
} else {
slurmdb_association_rec_t** children;
- size_t merge_count =
- _count_tied_accounts(siblings, ndx);
+ size_t merge_count = _count_tied_accounts(siblings, i);
/* Merging does not affect child level_fs calculations
* since the necessary information is stored on each
* assoc's usage struct */
- children = _merge_accounts(siblings, ndx,
- ndx + merge_count,
+ children = _merge_accounts(siblings, i, i + merge_count,
assoc_level);
- _calc_tree_fs(children, assoc_level + 1, rank, i, tied);
+ _calc_tree_fs(children, assoc_level+1, rank, rnt, tied);
/* Skip over any merged accounts */
- ndx += merge_count;
+ i += merge_count;
xfree(children);
}
@@ -348,21 +390,21 @@ static void _apply_priority_fs(void)
{
slurmdb_association_rec_t** children = NULL;
uint32_t rank = g_user_assoc_count;
- uint32_t i = rank;
+ uint32_t rnt = rank;
size_t child_count = 0;
if (priority_debug)
info("Fair Tree fairshare algorithm, starting at root:");
- assoc_mgr_root_assoc->usage->level_fs = 1L;
+ assoc_mgr_root_assoc->usage->level_fs = (long double) NO_VAL;
/* _calc_tree_fs requires an array instead of List */
- children = _append_children_to_array(
+ children = _append_list_to_array(
assoc_mgr_root_assoc->usage->children_list,
children,
&child_count);
- _calc_tree_fs(children, 0, &rank, &i, false);
+ _calc_tree_fs(children, 0, &rank, &rnt, false);
xfree(children);
}
diff --git a/testsuite/expect/test24.4 b/testsuite/expect/test24.4
index 3ea66e72972..7fb9c7ea466 100755
--- a/testsuite/expect/test24.4
+++ b/testsuite/expect/test24.4
@@ -78,7 +78,7 @@ expect {
exp_continue
}
- "root|||0.000000|240||1.000000||1.000000|0|0|" {
+ "root|||0.000000|240||1.000000|||0|0|" {
incr matches
exp_continue
}
--
GitLab