Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add balloon type specific pinMemory option #451

Merged
merged 4 commits into from
Dec 17, 2024
Merged

Add balloon type specific pinMemory option #451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
905787c
balloons: add pinMemory option to balloon type
askervin Dec 16, 2024
fea2d2f
balloons: implement balloon type specific pinMemory
askervin Dec 16, 2024
71250c2
e2e: test pinMemory override in balloon types
askervin Dec 16, 2024
9fa8220
doc: document balloon type specific pinMemory
askervin Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions cmd/plugins/balloons/policy/balloons-policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1411,7 +1411,7 @@ func (p *balloons) updatePinning(blns ...*Balloon) {
} else {
allowedCpus = pinnableCpus
}
p.pinCpuMem(c, allowedCpus, bln.Mems, bln.memTypeMask)
p.pinCpuMem(c, allowedCpus, bln.Mems, bln.memTypeMask, bln.Def.PinMemory)
}
}
}
Expand Down Expand Up @@ -1517,7 +1517,7 @@ func (p *balloons) dismissContainer(c cache.Container, bln *Balloon) {
}

// pinCpuMem pins container to CPUs and memory nodes if flagged
func (p *balloons) pinCpuMem(c cache.Container, cpus cpuset.CPUSet, mems idset.IDSet, memTypeMask libmem.TypeMask) {
func (p *balloons) pinCpuMem(c cache.Container, cpus cpuset.CPUSet, mems idset.IDSet, memTypeMask libmem.TypeMask, blnDefPinMemory *bool) {
if p.bpoptions.PinCPU == nil || *p.bpoptions.PinCPU {
log.Debug(" - pinning %s to cpuset: %s", c.PrettyName(), cpus)
c.SetCpusetCpus(cpus.String())
Expand All @@ -1526,7 +1526,13 @@ func (p *balloons) pinCpuMem(c cache.Container, cpus cpuset.CPUSet, mems idset.I
c.SetCPUShares(int64(cache.MilliCPUToShares(int64(mCpu))))
}
}
if p.bpoptions.PinMemory == nil || *p.bpoptions.PinMemory {
// Start from policy-level PinMemory...
pinMemory := p.bpoptions.PinMemory == nil || *p.bpoptions.PinMemory
// ...and allow override in balloon-type-level PinMemory
if blnDefPinMemory != nil {
pinMemory = *blnDefPinMemory
}
if pinMemory {
Copy link
Collaborator

@klihub klihub Dec 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should let also libmem know about the new container consuming memory, even if we don't ask libmem to determine the pinning according to its own taste. Otherwise even in theory it cannot determine correctly when a node is in danger of running out of memory.

if c.PreserveMemoryResources() {
log.Debug(" - preserving %s pinning to memory %q", c.PrettyName, c.GetCpusetMems())
preserveMems, err := parseIDSet(c.GetCpusetMems())
Expand Down
5 changes: 5 additions & 0 deletions config/crd/bases/config.nri_balloonspolicies.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,11 @@ spec:
items:
type: string
type: array
pinMemory:
description: |-
PinMemory controls pinning containers to memory nodes.
Overrides the policy level PinMemory setting in this balloon type.
type: boolean
preferCloseToDevices:
description: |-
PreferCloseToDevices: prefer creating new balloons of this
Expand Down
5 changes: 5 additions & 0 deletions deployment/helm/balloons/crds/config.nri_balloonspolicies.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,11 @@ spec:
items:
type: string
type: array
pinMemory:
description: |-
PinMemory controls pinning containers to memory nodes.
Overrides the policy level PinMemory setting in this balloon type.
type: boolean
preferCloseToDevices:
description: |-
PreferCloseToDevices: prefer creating new balloons of this
Expand Down
14 changes: 9 additions & 5 deletions docs/resource-policy/policy/balloons.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,11 @@ Balloons policy parameters:
default is `true`: the container cannot use other CPUs.
- `pinMemory` controls pinning a container to the memories that are
closest to the CPUs of its balloon. The default is `true`: allow
using memory only from the closest NUMA nodes. Warning: this may
cause kernel to kill containers due to out-of-memory error when
closest NUMA nodes do not have enough memory. In this situation
consider switching this option `false`.
using memory only from the closest NUMA nodes. Can be overridden by
pinMemory in balloon types. Warning: pinning memory may cause kernel
to kill containers due to out-of-memory error when allowed NUMA
nodes do not have enough memory. In this situation consider
switching this option `false`.
- `preserve` specifies containers whose resource pinning must not be
modified by the policy.
- `matchExpressions` if a container matches an expression in this
Expand Down Expand Up @@ -174,10 +175,13 @@ Balloons policy parameters:
- `cpuClass` specifies the name of the CPU class according to which
CPUs of balloons are configured. Class properties are defined in
separate `cpu.classes` objects, see below.
- `pinMemory` overrides policy-level `pinMemory` in balloons of this
type.
- `memoryTypes` is a list of allowed memory types for containers in
a balloon. Supported types are "HBM", "DRAM" and "PMEM". This
setting can be overridden by a pod/container specific
`memory-type` annotation.
`memory-type` annotation. Memory types have no when not pinning
memory (see `pinMemory`).
- `preferCloseToDevices`: prefer creating new balloons close to
listed devices. List of strings
- `preferCoreType`: specifies preferences of the core type which
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/config/v1alpha1/resmgr/policy/balloons/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,9 @@ type BalloonDef struct {
// +listType=set
// +kubebuilder:validation:items:XValidation:rule="self == 'DRAM' || self == 'HBM' || self == 'PMEM'",messageExpression="\"invalid memory type: \" + self + \", expected DRAM, HBM, or PMEM\""
MemoryTypes []string `json:"memoryTypes,omitempty"`
// PinMemory controls pinning containers to memory nodes.
// Overrides the policy level PinMemory setting in this balloon type.
PinMemory *bool `json:"pinMemory,omitempty"`
// AllocatorPriority (High, Normal, Low, None)
// This parameter is passed to CPU allocator when creating or
// resizing a balloon. At init, balloons with highest priority
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/config/v1alpha1/resmgr/policy/balloons/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions ...e2e/policies.test-suite/balloons/n6-hbm-cxl/test01-memory-types/balloons-memory-types.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ config:
preferSpreadingPods: true
preferNewBalloons: true

- name: no-pin-mem
minCPUs: 1
maxCPUs: 1
preferNewBalloons: true
pinMemory: false

instrumentation:
httpEndpoint: ":8891"
prometheusExport: true
Expand Down
6 changes: 4 additions & 2 deletions test/e2e/policies.test-suite/balloons/n6-hbm-cxl/test01-memory-types/code.var.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ POD_ANNOTATION[5]="memory-type.resource-policy.nri.io/container.pod0c5: hbm,dram
POD_ANNOTATION[10]="balloon.balloons.resource-policy.nri.io/container.pod0c0: mem-types"
POD_ANNOTATION[16]="balloon.balloons.resource-policy.nri.io/container.pod0c6: mem-types"
POD_ANNOTATION[17]="balloon.balloons.resource-policy.nri.io/container.pod0c7: no-mem-types"
CPUREQ="200m" MEMREQ="300M" CPULIM="" MEMLIM="300M" CONTCOUNT=8 create balloons-busybox
POD_ANNOTATION[18]="balloon.balloons.resource-policy.nri.io/container.pod0c8: no-pin-mem"
CPUREQ="200m" MEMREQ="300M" CPULIM="" MEMLIM="300M" CONTCOUNT=9 create balloons-busybox
report allowed
verify 'mems["pod0c0"] == {hbm0} if packages["pod0c0"] == {"package0"} else mems["pod0c0"] == {hbm1}' \
'mems["pod0c1"] == {dram0} if packages["pod0c1"] == {"package0"} else mems["pod0c1"] == {dram1}' \
Expand All @@ -32,7 +33,8 @@ verify 'mems["pod0c0"] == {hbm0} if packages["pod0c0"] == {"package0
'mems["pod0c4"] == {dram0,pmem0} if packages["pod0c4"] == {"package0"} else mems["pod0c4"] == {dram1,pmem1}' \
'mems["pod0c5"] == {hbm0,dram0,pmem0} if packages["pod0c5"] == {"package0"} else mems["pod0c5"] == {hbm1,dram1,pmem1}' \
'mems["pod0c6"] == {hbm0,pmem0} if packages["pod0c6"] == {"package0"} else mems["pod0c6"] == {hbm1,pmem1}' \
'mems["pod0c7"] == {dram0} if packages["pod0c7"] == {"package0"} else mems["pod0c7"] == {dram1}'
'mems["pod0c7"] == {dram0} if packages["pod0c7"] == {"package0"} else mems["pod0c7"] == {dram1}' \
'mems["pod0c8"] == {dram0,dram1,hbm0,hbm1,pmem0,pmem1}'

cleanup

Expand Down
Loading