Skip to content

Commit

Permalink
Free memory report now accurate in nested cgroups
Browse files Browse the repository at this point in the history
This commit causes 'determineMemoryLimit' to read the maximum available
memory from the 'heirarchical_memory_limit' field in the cgroup's
'memory.stat' file, rather than reading it from the
'memory.limit_in_bytes' file.

Without this change, gosigar will report the memory constraints of the
outermost non-root cgroup that contains the one we're running in, rather
than the constraints of the cgroup that contains us.

People have reported that both Docker and Kubernetes constrain
containers in this way, so this change makes gosigar correct in those
conditions.

Kenneth Lakin reports that he is now unsure why we would ever consult
the 'memory.limit_in_bytes' file, as 'heirarchical_memory_limit' seems
to always give us the information we want, but that he is too cautious
to suggest that we stop consulting 'memory.limit_in_bytes' entirely.

(This commit message was written by Kenneth Lakin. Blame him for
any inaccuracies, typos, or purple prose.)

Authored-by: Nicholaswang <wzhever@gmail.com>
  • Loading branch information
Nicholaswang authored May 25, 2022
1 parent 6b0b3cb commit 60d9d42
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 3 deletions.
22 changes: 20 additions & 2 deletions sigar_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ import (
"syscall"
)

const MaxUint64 = ^uint64(0)
const (
MaxUint64 = ^uint64(0)
// UnlimitedMemorySize defines the bytes size when memory limit is not set (2 ^ 63 - 4096)
UnlimitedMemorySize = "9223372036854771712"
)

var system struct {
ticks uint64
Expand Down Expand Up @@ -489,10 +493,24 @@ func determineMemoryLimit(cgroup string) (uint64, error) {
}

limitAsString, err = ioutil.ReadFile(Sysd1 + cgroup + "/memory.limit_in_bytes")
if err == nil {
if string(limitAsString) != UnlimitedMemorySize && err == nil {
return strtoull(strings.Split(string(limitAsString), "\n")[0])
}

var limit uint64
table := map[string]*uint64{
"hierarchical_memory_limit": &limit,
}

err, found := parseCgroupMeminfo(Sysd1+cgroup, table)
if err == nil {
if !found {
// If no data was found, simply claim `zero limit set`.
return 0, errors.New("no hierarchical memory limit found")
}
return limit, nil
}

return 0, err
}

Expand Down
16 changes: 15 additions & 1 deletion sigar_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,15 @@ memory2 /smart/fox/jumped/by/lazy/dog/duplicate cgroup2 irrelevant,options
It("fails for missing files", func() {
limit, err := determineMemoryLimit(``)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(Equal("open " + procd + "/memory/memory.limit_in_bytes: no such file or directory"))
// it will falls back to memory.stat when memory.limit_in_bytes not found
Expect(err.Error()).To(Equal("open " + procd + "/memory/memory.stat: no such file or directory"))
Expect(limit).To(BeNumerically("==", 0))
})
It("fails for missing data in memory.stat file", func() {
memStatSetup(``, ``)
limit, err := determineMemoryLimit(``)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(Equal(`no hierarchical memory limit found`))
Expect(limit).To(BeNumerically("==", 0))
})
It("fails for missing data in v1 file", func() {
Expand Down Expand Up @@ -331,6 +339,12 @@ memory2 /smart/fox/jumped/by/lazy/dog/duplicate cgroup2 irrelevant,options
Expect(err).ToNot(HaveOccurred())
Expect(limit).To(BeNumerically("==", 1111))
})
It("returns hierarchyMemoryLimit when limit_in_bytes is unlimited", func() {
memStatSetup(``, `hierarchical_memory_limit 3333`)
limit, err := determineMemoryLimit(``)
Expect(err).ToNot(HaveOccurred())
Expect(limit).To(BeNumerically("==", 3333))
})
It("signals v2 no limit with failure", func() {
memLimitSetup2(``, `max`)
limit, err := determineMemoryLimit(``)
Expand Down

0 comments on commit 60d9d42

Please sign in to comment.