Commit Graph

22 Commits

Author SHA1 Message Date
Kevin Klues
e370b7335c Add extensive unit testing for TopologyManager hint generation algorithm
Signed-off-by: Kevin Klues <kklues@nvidia.com>
2022-03-01 17:30:24 +00:00
Kevin Klues
99c57828ce Update TopologyManager algorithm for selecting "best" non-preferred hint
For the 'single-numa' and 'restricted' TopologyManager policies, pods are only
admitted if all of their containers have perfect alignment across the set of
resources they are requesting. The best-effort policy, on the other hand, will
prefer allocations that have perfect alignment, but fall back to a non-preferred
alignment if perfect alignment can't be achieved.

The existing algorithm of how to choose the best hint from the set of
"non-preferred" hints is fairly naive and often results in choosing a
sub-optimal hint. It works fine in cases where all resources would end up
coming from a single NUMA node (even if its not the same NUMA nodes), but
breaks down as soon as multiple NUMA nodes are required for the "best"
alignment.  We will never be able to achieve perfect alignment with these
non-preferred hints, but we should try and do something more intelligent than
simply choosing the hint with the narrowest mask.

In an ideal world, we would have the TopologyManager return a set of
"resources-relative" hints (as opposed to a common hint for all resources as is
done today). Each resource-relative hint would indicate how many other
resources could be aligned to it on a given NUMA node, and a  hint provider
would use this information to allocate its resources in the most aligned way
possible. There are likely some edge cases to consider here, but such an
algorithm would allow us to do partial-perfect-alignment of "some" resources,
even if all resources could not be perfectly aligned.

Unfortunately, supporting something like this would require a major redesign to
how the TopologyManager interacts with its hint providers (as well as how those
hint providers make decisions based on the hints they get back).

That said, we can still do better than the naive algorithm we have today, and
this patch provides a mechanism to do so.

We start by looking at the set of hints passed into the TopologyManager for
each resource and generate a list of the minimum number of NUMA nodes required
to satisfy an allocation for a given resource. Each entry in this list then
contains the 'minNUMAAffinity.Count()' for a given resources. Once we have this
list, we find the *maximum* 'minNUMAAffinity.Count()' from the list and mark
that as the 'bestNonPreferredAffinityCount' that we would like to have
associated with whatever "bestHint" we ultimately generate. The intuition being
that we would like to (at the very least) get alignment for those resources
that *require* multiple NUMA nodes to satisfy their allocation. If we can't
quite get there, then we should try to come as close to it as possible.

Once we have this 'bestNonPreferredAffinityCount', the algorithm proceeds as
follows:

If the mergedHint and bestHint are both non-preferred, then try and find a hint
whose affinity count is as close to (but not higher than) the
bestNonPreferredAffinityCount as possible. To do this we need to consider the
following cases and react accordingly:

  1. bestHint.NUMANodeAffinity.Count() >  bestNonPreferredAffinityCount
  2. bestHint.NUMANodeAffinity.Count() == bestNonPreferredAffinityCount
  3. bestHint.NUMANodeAffinity.Count() <  bestNonPreferredAffinityCount

For case (1), the current bestHint is larger than the
bestNonPreferredAffinityCount, so updating to any narrower mergeHint is
preferred over staying where we are.

For case (2), the current bestHint is equal to the
bestNonPreferredAffinityCount, so we would like to stick with what we have
*unless* the current mergedHint is also equal to bestNonPreferredAffinityCount
and it is narrower.

For case (3), the current bestHint is less than bestNonPreferredAffinityCount,
so we would like to creep back up to bestNonPreferredAffinityCount as close as
we can. There are three cases to consider here:

  3a. mergedHint.NUMANodeAffinity.Count() >  bestNonPreferredAffinityCount
  3b. mergedHint.NUMANodeAffinity.Count() == bestNonPreferredAffinityCount
  3c. mergedHint.NUMANodeAffinity.Count() <  bestNonPreferredAffinityCount

For case (3a), we just want to stick with the current bestHint because choosing
a new hint that is greater than bestNonPreferredAffinityCount would be
counter-productive.

For case (3b), we want to immediately update bestHint to the current
mergedHint, making it now equal to bestNonPreferredAffinityCount.

For case (3c), we know that *both* the current bestHint and the current
mergedHint are less than bestNonPreferredAffinityCount, so we want to choose
one that brings us back up as close to bestNonPreferredAffinityCount as
possible. There are three cases to consider here:

  3ca. mergedHint.NUMANodeAffinity.Count() >  bestHint.NUMANodeAffinity.Count()
  3cb. mergedHint.NUMANodeAffinity.Count() <  bestHint.NUMANodeAffinity.Count()
  3cc. mergedHint.NUMANodeAffinity.Count() == bestHint.NUMANodeAffinity.Count()

For case (3ca), we want to immediately update bestHint to mergedHint because
that will bring us closer to the (higher) value of
bestNonPreferredAffinityCount.

For case (3cb), we want to stick with the current bestHint because choosing the
current mergedHint would strictly move us further away from the
bestNonPreferredAffinityCount.

Finally, for case (3cc), we know that the current bestHint and the current
mergedHint are equal, so we simply choose the narrower of the 2.

This patch implements this algorithm for the case where we must choose from a
set of non-preferred hints and provides a set of unit-tests to verify its
correctness.

Signed-off-by: Kevin Klues <kklues@nvidia.com>
2022-03-01 14:38:26 +00:00
Kevin Klues
155562dd2e Fix bug in TopologyManager with merging hints when NUM_NUMA > 2
Before this fix, hint permutations such as:

	permutation: [{11 true} {0101 true}]

Could result in merged hints of:

	mergedHint: {01 true}

This was possible because both hints in the permutation container a "preferred"
allocation (i.e. the full set of NUMA nodes set in the affinity bitmask are
*required* to satisfy the allocation). With this in place, the simplified logic
we had simply kept the merged hint as preferred as well.

However, what we really want is to ensure that the merged hint is only
preferred if *true* alignment of all resources is possible (i.e. if all hints
in the permutation are preferred AND their affinities are exactly equal).

The only exception to this is if *no* topology information is provided by a
given hint provider. In this case, we assume alignment doesn't matter and only
consider the resources that actually have hints provided for them.

This changes the semantics of permutations of the form:

	permutation: [{111 true} {011 true}]

To now result in the merged hint of:

	mergedHint: {011 false}

Instead of:

	mergedHint: {011 true}

This is arguably how it should always have been though (because a hint should
not be preferred if true alignment isn't possible), and two tests have had to
change to accomodate these new semantics.

This commit changes the merge function to implement the updated logic, adds a
test to verify it is functioning correctly, and updates the two tests mentioned
above to adjust to the new semantics.

Signed-off-by: Kevin Klues <kklues@nvidia.com>
2022-02-10 22:07:51 +00:00
Kevin Klues
bc686ea27b Update TopologyManager.GetTopologyHints() to take pointers
Previously, this function was taking full Pod and Container objects
unnecessarily. This commit updates this so that they will take pointers
instead.
2020-02-03 17:13:28 +00:00
nolancon
5e23517ebf Use reflect.DeepEqual check in policy_test.go 2020-01-16 08:13:07 +00:00
nolancon
92eb7cd601 Update "Single NUMA hint generation" expected affinity to nil 2020-01-16 08:13:07 +00:00
nolancon
8b3f6e61a2 Move test case "Two providers, 1 with 2 hints, 1 with single
non-preferred hint matching" into specific policy tests
2020-01-16 08:13:07 +00:00
nolancon
681c42bfc2 Move test case "Two providers, 1 hint each, same mask, 1 preferred, 1
not 2/2" into specific policy tests
2020-01-16 08:13:07 +00:00
nolancon
a38a2562b2 Move test case "Two providers, 1 hint each, same mask, 1 preferred, 1
not 1/2" into specific policy test.
2020-01-16 08:13:07 +00:00
nolancon
f639da7637 Move test case "Two providers, 1 hint each, no common mask" into
specific policy tests.
2020-01-16 08:13:07 +00:00
nolancon
401a2bb285 Move test case "Single TopologyHint with Preferred as false and
NUMANodeAffinity as nil" into specific policy tests.
2020-01-16 08:13:06 +00:00
nolancon
6460ef6392 Move test case "Single TopologyHint with Preferred as true and
NUMANodeAffinity as nil" into specific policy tests.
2020-01-16 08:13:06 +00:00
nolancon
baeff9ec5d Move test case "HintProvider returns empty non-nil map[string][]TopologyHint from
provider" into specific policy tests.
2020-01-16 08:13:06 +00:00
nolancon
599217d482 Move test case "HintProvider returns -nil map[string][]TopologyHint from provider" into specific policy tests 2020-01-16 08:13:06 +00:00
nolancon
57661ee946 Move test case 'HintProvider returns empty non-nil map[string][]TopologyHint' into specific policy tests. 2020-01-16 08:13:06 +00:00
nolancon
51f1af0395 Move test case 'TopologyHint not set' into individual policy tests 2020-01-16 08:13:06 +00:00
nolancon
8466a5852a Restore policy_test.go to upstream
Following commits will contain incremental changes to this file to ease
review process and ensure all tests are accounted for.
2020-01-16 08:13:06 +00:00
nolancon
59bb6c4d6f Update checks in mergeProvidersHints:
- Initialize best Hint to TopologyHint{}
- Update checks.
- Move generic unit test case into policy specific tests and updated
expected outcome to reflect changes.
2020-01-16 08:13:06 +00:00
nolancon
4cc5b9e46c Edit hints returned from policies and unit tests:
- Best Effort Policy: Return hint with nil affinity as opposed to
defaultAffinity when provider has no preference for NUMA affinty or no
possible NUMA affinities.
- Single NUMA Node Policy: Remove defaultHint from mergeProvidersHints.
Instead return appropriate TopologyHint where required.
- Update unit tests to reflect changes. Some test cases moved into
individual policy test functions due to differing returned affinties
per policy.
2020-01-16 08:13:05 +00:00
nolancon
b5ca4989e3 Update unit tests:
- Update filterHints test to reflect changes in previous commit.
- Some common test cases achieve differing expected results based on
policy due to independent merge strategies. These cases are moved into
individual policy based test functions.
2020-01-16 08:13:05 +00:00
Kevin Klues
b5f52e6072 Modularize TopologyManager policy Merge() tests
These changes make it so that a set of common test cases can be used for
all merge strategies, with specific test cases being able to be
specified on a policy-by-policy basis.
2019-11-04 18:43:07 +01:00
Kevin Klues
7ea1fc9be4 Move TopologyManager TestPolicyMerge() to shared test file 2019-11-04 18:43:07 +01:00