topology-aware: add CPU class support. - #729
Conversation
65af27a to
a216b23
Compare
askervin
left a comment
askervin
left a comment
There was a problem hiding this comment.
Adding one note... I'll continue review soon...
| errs = append(errs, err) | ||
| } | ||
|
|
||
| if c.SharedCpuClass == "" { |
There was a problem hiding this comment.
For convenience, balloons policy handles CPU class name "default" differently than others: if a balloon type does not specify any CPU class, the "default" CPU class will be applied on CPUs of its instances.
https://containers.github.io/nri-plugins/stable/docs/resource-policy/policy/balloons.html#cpu-tuning
I was thinking if it would make sense to use c.{Shared,Reserved,DefaultExclusive}CpuClass = "default" when a user has specified "default" CPU class. Pros: one could use the same CPU class configuration if evaluating and switching between policies, and it might be more intuitive than exclusive allocations using sharedCPUClass by default. Cons: if the existence of the "default" CPU class is not required and the current mechanism for figuring out a default class would be used in that case, it would make make the whole thing unnecessarily complex.
Having special semantics for a class named "default" fits quite naturally in balloons, as there is the "default" balloon type, too. In T-A, there is no similar context, so this might not be a good idea.
There was a problem hiding this comment.
and it might be more intuitive than exclusive allocations using sharedCPUClass by default.
Well, we don't do that ( that explicitly) by default, although that is the effective end result (for non-isolated CPUs) if there is no DefaultExclusiveCpuClass set and the CPUs were ever part of a shared pool after which they get assigned exclusively to a container which does not have an annotated class.
Maybe we could use a defined default class as DefaultExclusiveCpuClass if one is not given, and log this fact. Then again, if we did that then there would be 2 ways of setting a default class, and one of them would be quite implicit, not obvious without reading the documentation: does default apply by default to all CPUs, shared CPUs only, exclusively allocated CPUs only, or something else ?
| }) | ||
| if len(hints.Prefer) > 0 { | ||
| ccCpus := cpus.Intersection(hints.Prefer[0].Cpus) | ||
| if ccCpus.Equals(cpus) { |
There was a problem hiding this comment.
This will be more useful with #719 because then we can go through alternative (equally acceptable) cpuset hints. Then it'll be more likely that there is a cpuset that is aligned with the set from GetCPUOffer().
| if !p.cpuClasses.IsKnownClass(cc) { | ||
| return nil, policyError("%s: nonexistent CPU class %q requested", | ||
| container.PrettyName(), cc) | ||
| } |
There was a problem hiding this comment.
Would it be good to log a warning, an error, or even return policyError if a non-Guaranteed container is annotated with cpu-class annotation?
There was a problem hiding this comment.
Logging a warning definitely makes sense. Though we need to do it for containers not eligible for exclusive CPU allocation, which is not quite the same as non-Guaranteed QoS class.
Why it's not that obvious/I'm a bit hesitant on outright erroring out is that with effective annotations a common pattern is to assign a pod-wide default with .../pod: default-class and then punch exceptions into it with container-specific further annotations like .../container.$CTR0: some-class and .../container.$CTR1: other-class. And in that case if you have containers with partial/shared pool CPU allocations, it is normal/an expected side-effect that their CPU class resolves to default-class. Though, it would be technically possible to possible to check for this and allow it, but error out in other cases...
a216b23 to
ff182b1
Compare
Satisfying CPU affinity to multiple preferences encoded in preferCloseToDevice requires more flexibility than provided in CPU class preference. For example, a container requires PCT HP (high priority) CPUs and CPU affinity to a GPU device. There are enough HP CPUs available in both sockets. Because socket 0 happens to have fewer HP CPUs available, cpuclass hint considers it worse choice than socket 1 with more HP CPUs. However, the GPU may be connected to socket 0. As a result, preferCloseToDevices consists of a GPU-affinity hint to all CPUs on socket 0, and HP CPU affinity hint to HP CPUs on socket 1. The latter hint will be ignored, in which order they ever happen to be. This commit changes cpuclass (HP CPUs) hints so that all good-enough CPU sets will be published. Given that cpuclass hints are applied after device (GPU) hints, above example works as follows. GPU-affinity hints all CPUs on socket 0. Next, best HP CPU affinity would hint HP CPUs on socket 1. As the intersection hinted CPUs is empty, cputree.go allocator ignores this hint. Finally, second-best HP CPU affinity hints HP CPUs on socket 0. Now intersection with already taken hints is not empty, so this hint is applied, and the container will get HP CPUs near the GPU. Technically, this commit changes CPU class AllocationHints to contain best-first-ordered list of good-enough CPU sets as alternatives for CPU allocators to prefer. Signed-off-by: Antti Kervinen <antti.kervinen@intel.com>
Force a cache save after CreateContainer, ensuring its state gets updated to Created on disk, too. Failing to do so could leave the last created container in the synthetic Creating state which is ignored during startup. This in turn could cause the last container to not get properly reallocated if the policy gets restarted or crashes. Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Avoid using 'balloon' in cpuclass configuration comments / description. Use the more neutral 'policy' term. Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Rework pool CPU allocation, splitting out exclusive and shared CPU picking logic to separate functions and making them dry-runnable. This should allow scoring to evaluate the actual pool offered CPUs at will. Add pool/supply.GetCPUOffer() which dry-runs pool CPU allocation for a given request. Rename existing GetOffer() to GetMemOffer() for symmetricity/better consistency. Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Add basic CPU class support. Exlusively allocated CPUs (of
Guaranteed QoS class containers), CPUs of shared pools and
CPUs of the reserved pool can be assigned to CPU classes.
Shared and reserved pool classes are configured. Exclusive
CPU classes can be configured (default class) or annotated
using an effective annotation with key 'cpu-class' on the
container's pod.
There are still missing bits in the current implementation:
- extra checks on reconfiguration
o all CPU classes in use are defined
- (maybe) extra checks on startup
o all CPU classes in use are defined
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
ff182b1 to
d2228e8
Compare
2 participants
This patch series adds basic CPU class support to the topology-aware policy.
With these patches in place, CPU classes can now be assigned to
CPU class support is turned on by defining some CPU classes in the configuration and specifying the CPU class for shared pool CPU by setting
sharedCPUClassin the configuration. This is the minimal required/valid configuration. Additionally, an optional reserved pool CPU class can be specified, for CPUs of the reserved pool, by settingreservedCPUClassin the configuration. An unset reserved class defaults to the shared class.Also optionally a default CPU class for exclusive CPUs can be specified by setting
defaultExclusiveCPUClassin the configuration. If set, exclusive CPUs of otherwise unnannotated containers are assigned to the default exclusive class. Containers can be annotated with a CPU class using thecpu-class[.resource-policy.nri.io]effective annotation base key. Exclusive CPUs of annotated containers are assigned to the annotated class.Once CPUs are released from exclusive use and returned to a shared pool, they are assigned to the shared CPU class.
This patch series still misses some checks for reconfiguration and end-to-end test cases. Hence it is marked as draft.