Skip to content

Commit

Permalink
client: do not disable memory swappiness if kernel does not support it
Browse files Browse the repository at this point in the history
This PR adds a workaround for very old Linux kernels which do not support
the memory swappiness interface file. Normally we write a "0" to the file
to explicitly disable swap. In the case the kernel does not support it,
give libcontainer a nil value so it does not write anything.

Fixes #17448
  • Loading branch information
shoenig committed Jun 20, 2023
1 parent f73b454 commit 78e67ad
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 3 deletions.
3 changes: 3 additions & 0 deletions .changelog/17625.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
client: Fixed a bug where Nomad incorrectly write to memory swappiness cgroup on old kernels
```
22 changes: 22 additions & 0 deletions client/lib/cgutil/cgutil_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"path/filepath"

"github.com/hashicorp/go-hclog"
"github.com/hashicorp/nomad/helper/pointer"
"github.com/hashicorp/nomad/helper/uuid"
"github.com/opencontainers/runc/libcontainer/cgroups"
lcc "github.com/opencontainers/runc/libcontainer/configs"
Expand Down Expand Up @@ -148,3 +149,24 @@ func CopyCpuset(source, destination string) error {

return nil
}

// MaybeDisableMemorySwappiness will disable memory swappiness, if that controller
// is available. Always the case for cgroups v2, but is not always the case on
// very old kernels with cgroups v1.
func MaybeDisableMemorySwappiness() *uint64 {
bypass := (*uint64)(nil)
zero := pointer.Of[uint64](0)

// cgroups v2 always set zero
if UseV2 {
return zero
}

// cgroups v1 check if the root and swappiness interface exist
_, err := os.Stat("/sys/fs/cgroup/memory/memory.swappiness")

This comment has been minimized.

Copy link
@akamensky

akamensky Jun 21, 2023

@shoenig I see the sys path file is present and can be stat'd, but it cannot be written to:

[root@redacted nomad]# stat /sys/fs/cgroup/memory/memory.swappiness
  File: ‘/sys/fs/cgroup/memory/memory.swappiness’
  Size: 0         	Blocks: 0          IO Block: 4096   regular empty file
Device: 18h/24d	Inode: 9307        Links: 1
Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2023-06-19 16:40:58.501000000 +0800
Modify: 2023-06-19 16:40:58.501000000 +0800
Change: 2023-06-19 16:40:58.501000000 +0800
 Birth: -
[root@redacted nomad]# cat /sys/fs/cgroup/memory/memory.swappiness
10
[root@redacted nomad]# echo 0 > /sys/fs/cgroup/memory/memory.swappiness
bash: echo: write error: Invalid argument
[root@redacted nomad]#
if os.IsNotExist(err) {
return bypass
}

return zero
}
9 changes: 9 additions & 0 deletions client/lib/cgutil/cgutil_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,3 +130,12 @@ func TestUtil_CopyCpuset(t *testing.T) {
require.Equal(t, "0-1", strings.TrimSpace(value))
})
}

func TestUtil_MaybeDisableMemorySwappiness(t *testing.T) {
ci.Parallel(t)

// will return 0 on any reasonable kernel (both cgroups v1 and v2)
value := MaybeDisableMemorySwappiness()
must.NotNil(t, value)
must.Eq(t, 0, *value)
}
5 changes: 2 additions & 3 deletions drivers/shared/executor/executor_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -689,9 +689,8 @@ func configureCgroups(cfg *lconfigs.Config, command *ExecCommand) error {
cfg.Cgroups.Resources.Memory = memHard * 1024 * 1024
cfg.Cgroups.Resources.MemoryReservation = memSoft * 1024 * 1024

// Disable swap to avoid issues on the machine
var memSwappiness uint64
cfg.Cgroups.Resources.MemorySwappiness = &memSwappiness
// Disable swap if possible, to avoid issues on the machine
cfg.Cgroups.Resources.MemorySwappiness = cgutil.MaybeDisableMemorySwappiness()
}

cpuShares := res.Cpu.CpuShares
Expand Down

0 comments on commit 78e67ad

Please sign in to comment.