Update code for the new fairshare algorithm.

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);
 }

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
 	}