From 5651d8550199b49c0ecb9e39221c54bce8a3994f Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Fri, 8 Dec 2023 16:59:04 +0000 Subject: [PATCH 1/4] fix: handle no memory limit in coder stat mem --- cli/clistat/cgroup.go | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/cli/clistat/cgroup.go b/cli/clistat/cgroup.go index 8f3ad4b71c0f0..8c9dc699dd692 100644 --- a/cli/clistat/cgroup.go +++ b/cli/clistat/cgroup.go @@ -44,6 +44,13 @@ const ( cgroupV2MemoryStat = "/sys/fs/cgroup/memory.stat" ) +const ( + // 9223372036854771712 is the highest positive signed 64-bit integer (263-1), + // rounded down to multiples of 4096 (2^12), the most common page size on x86 systems. + // This is used by docker to indicate no memory limit. + UnlimitedMemory int64 = 9223372036854771712 +) + // ContainerCPU returns the CPU usage of the container cgroup. // This is calculated as difference of two samples of the // CPU usage of the container cgroup. @@ -219,12 +226,29 @@ func (s *Statter) ContainerMemory(p Prefix) (*Result, error) { return nil, nil //nolint:nilnil } + var ( + r *Result + err error + ) if s.isCGroupV2() { - return s.cGroupV2Memory(p) + r, err = s.cGroupV2Memory(p) + if err != nil { + return nil, xerrors.Errorf("get cgroupv2 memory: %w", err) + } + } else { + // Fall back to CGroupv1 + r, err = s.cGroupV1Memory(p) + if err != nil { + return nil, xerrors.Errorf("get cgroupv1 memory: %w", err) + } } - // Fall back to CGroupv1 - return s.cGroupV1Memory(p) + // If there is no memory limit set on the container use the host value. + if int64(*r.Total) == UnlimitedMemory { + return s.HostMemory(p) + } + + return r, nil } func (s *Statter) cGroupV2Memory(p Prefix) (*Result, error) { From 7bc56cf7c3c9f4155379da00683e152f5d145dae Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Fri, 8 Dec 2023 17:18:43 +0000 Subject: [PATCH 2/4] handle unlimited differently --- cli/clistat/cgroup.go | 27 +++++++-------------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/cli/clistat/cgroup.go b/cli/clistat/cgroup.go index 8c9dc699dd692..47787748a12d1 100644 --- a/cli/clistat/cgroup.go +++ b/cli/clistat/cgroup.go @@ -226,29 +226,12 @@ func (s *Statter) ContainerMemory(p Prefix) (*Result, error) { return nil, nil //nolint:nilnil } - var ( - r *Result - err error - ) if s.isCGroupV2() { - r, err = s.cGroupV2Memory(p) - if err != nil { - return nil, xerrors.Errorf("get cgroupv2 memory: %w", err) - } - } else { - // Fall back to CGroupv1 - r, err = s.cGroupV1Memory(p) - if err != nil { - return nil, xerrors.Errorf("get cgroupv1 memory: %w", err) - } + return s.cGroupV2Memory(p) } - // If there is no memory limit set on the container use the host value. - if int64(*r.Total) == UnlimitedMemory { - return s.HostMemory(p) - } - - return r, nil + // Fall back to CGroupv1 + return s.cGroupV1Memory(p) } func (s *Statter) cGroupV2Memory(p Prefix) (*Result, error) { @@ -295,6 +278,10 @@ func (s *Statter) cGroupV1Memory(p Prefix) (*Result, error) { // Nonetheless, if it is not, assume there is no limit set. maxUsageBytes = -1 } + // Set to unlimited if we detect the unlimited docker value. + if maxUsageBytes == UnlimitedMemory { + maxUsageBytes = -1 + } // need a space after total_rss so we don't hit something else usageBytes, err := readInt64(s.fs, cgroupV1MemoryUsageBytes) From d591a9557b7633148c0a41bd57f388b50a317f8d Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Fri, 8 Dec 2023 17:35:08 +0000 Subject: [PATCH 3/4] add test --- cli/clistat/stat_internal_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/cli/clistat/stat_internal_test.go b/cli/clistat/stat_internal_test.go index 283c455129aa7..1823d7bf1ed0d 100644 --- a/cli/clistat/stat_internal_test.go +++ b/cli/clistat/stat_internal_test.go @@ -48,6 +48,9 @@ func TestResultString(t *testing.T) { Expected: "0.5/8 cores (6%)", Result: Result{Used: 0.5, Total: ptr.To(8.0), Unit: "cores"}, }, + { + Result: Result{Used: 1, Total: ptr.To(8.0), Unit: "Gb"}, + }, } { assert.Equal(t, tt.Expected, tt.Result.String()) } @@ -197,6 +200,18 @@ func TestStatter(t *testing.T) { assert.Nil(t, mem.Total) assert.Equal(t, "B", mem.Unit) }) + t.Run("ContainerMemory/NoLimit", func(t *testing.T) { + t.Parallel() + fs := initFS(t, fsContainerCgroupV1DockerNoMemoryLimit) + s, err := New(WithFS(fs), withNoWait) + require.NoError(t, err) + mem, err := s.ContainerMemory(PrefixDefault) + require.NoError(t, err) + require.NotNil(t, mem) + assert.Equal(t, 268435456.0, mem.Used) + assert.Nil(t, mem.Total) + assert.Equal(t, "B", mem.Unit) + }) }) t.Run("CGroupV2", func(t *testing.T) { @@ -384,6 +399,17 @@ proc /proc/sys proc ro,nosuid,nodev,noexec,relatime 0 0`, cgroupV1MemoryUsageBytes: "536870912", cgroupV1MemoryStat: "total_inactive_file 268435456", } + fsContainerCgroupV1DockerNoMemoryLimit = map[string]string{ + procOneCgroup: "0::/docker/aa86ac98959eeedeae0ecb6e0c9ddd8ae8b97a9d0fdccccf7ea7a474f4e0bb1f", + procMounts: `overlay / overlay rw,relatime,lowerdir=/some/path:/some/path,upperdir=/some/path:/some/path,workdir=/some/path:/some/path 0 0 +proc /proc/sys proc ro,nosuid,nodev,noexec,relatime 0 0`, + cgroupV1CPUAcctUsage: "0", + cgroupV1CFSQuotaUs: "-1", + cgroupV1CFSPeriodUs: "100000", + cgroupV1MemoryMaxUsageBytes: "9223372036854771712", + cgroupV1MemoryUsageBytes: "536870912", + cgroupV1MemoryStat: "total_inactive_file 268435456", + } fsContainerCgroupV1AltPath = map[string]string{ procOneCgroup: "0::/docker/aa86ac98959eeedeae0ecb6e0c9ddd8ae8b97a9d0fdccccf7ea7a474f4e0bb1f", procMounts: `overlay / overlay rw,relatime,lowerdir=/some/path:/some/path,upperdir=/some/path:/some/path,workdir=/some/path:/some/path 0 0 From 54de7521bcb6c2655914d33075d3a3c169e546ae Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Fri, 8 Dec 2023 17:36:04 +0000 Subject: [PATCH 4/4] fix --- cli/clistat/stat_internal_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/cli/clistat/stat_internal_test.go b/cli/clistat/stat_internal_test.go index 1823d7bf1ed0d..10a09c178f8e8 100644 --- a/cli/clistat/stat_internal_test.go +++ b/cli/clistat/stat_internal_test.go @@ -48,9 +48,6 @@ func TestResultString(t *testing.T) { Expected: "0.5/8 cores (6%)", Result: Result{Used: 0.5, Total: ptr.To(8.0), Unit: "cores"}, }, - { - Result: Result{Used: 1, Total: ptr.To(8.0), Unit: "Gb"}, - }, } { assert.Equal(t, tt.Expected, tt.Result.String()) }