diff options
author | John Cai <jcai@gitlab.com> | 2022-05-03 00:03:03 +0300 |
---|---|---|
committer | John Cai <jcai@gitlab.com> | 2022-05-03 00:03:03 +0300 |
commit | 9ed5fcef68a7f32c7da55dc6fbeee70acefe30f1 (patch) | |
tree | 559239c354cb8ffa216d1e05b31e14c8b511eb1c | |
parent | cc8901bc9389a9f85267fe820acc3777494ea337 (diff) | |
parent | be0ee060975bddbabdd27cad3e4e0c6881d5c468 (diff) |
Merge branch 'jc-cgroups-improved-config' into 'master'
config: Introduce new Cgroups config
See merge request gitlab-org/gitaly!4483
-rw-r--r-- | internal/cgroups/cgroups.go | 1 | ||||
-rw-r--r-- | internal/cgroups/mock_linux_test.go | 1 | ||||
-rw-r--r-- | internal/cgroups/v1_linux.go | 2 | ||||
-rw-r--r-- | internal/cgroups/v1_linux_test.go | 1 | ||||
-rw-r--r-- | internal/gitaly/config/cgroups/cgroups.go | 31 | ||||
-rw-r--r-- | internal/gitaly/config/config.go | 28 | ||||
-rw-r--r-- | internal/gitaly/config/config_test.go | 384 |
7 files changed, 309 insertions, 139 deletions
diff --git a/internal/cgroups/cgroups.go b/internal/cgroups/cgroups.go index 273746685..dc0cf0066 100644 --- a/internal/cgroups/cgroups.go +++ b/internal/cgroups/cgroups.go @@ -24,6 +24,7 @@ type Manager interface { // NewManager returns the appropriate Cgroups manager func NewManager(cfg cgroups.Config) Manager { + // nolint:staticcheck // we will deprecate the old cgroups config in 15.0 if cfg.Count > 0 { return newV1Manager(cfg) } diff --git a/internal/cgroups/mock_linux_test.go b/internal/cgroups/mock_linux_test.go index 4ee705fed..620ea06d5 100644 --- a/internal/cgroups/mock_linux_test.go +++ b/internal/cgroups/mock_linux_test.go @@ -103,6 +103,7 @@ func (m *mockCgroup) setupMockCgroupFiles( require.NoError(t, os.WriteFile(controlFilePath, []byte(content), 0o644)) } + // nolint:staticcheck // we will deprecate the old cgroups config in 15.0 for shard := uint(0); shard < manager.cfg.Count; shard++ { shardPath := filepath.Join(cgroupPath, fmt.Sprintf("shard-%d", shard)) require.NoError(t, os.MkdirAll(shardPath, 0o755)) diff --git a/internal/cgroups/v1_linux.go b/internal/cgroups/v1_linux.go index d98a75807..75573e96a 100644 --- a/internal/cgroups/v1_linux.go +++ b/internal/cgroups/v1_linux.go @@ -68,6 +68,7 @@ func (cg *CGroupV1Manager) Setup() error { } } + // nolint:staticcheck // we will deprecate the old cgroups config in 15.0 for i := 0; i < int(cg.cfg.Count); i++ { _, err := cgroups.New(cg.hierarchy, cgroups.StaticPath(cg.cgroupPath(i)), resources) if err != nil { @@ -83,6 +84,7 @@ func (cg *CGroupV1Manager) Setup() error { // exited. func (cg *CGroupV1Manager) AddCommand(cmd *command.Command) error { checksum := crc32.ChecksumIEEE([]byte(strings.Join(cmd.Args(), ""))) + // nolint:staticcheck // we will deprecate the old cgroups config in 15.0 groupID := uint(checksum) % cg.cfg.Count cgroupPath := cg.cgroupPath(int(groupID)) diff --git a/internal/cgroups/v1_linux_test.go b/internal/cgroups/v1_linux_test.go index 4db332830..7de3d910d 100644 --- a/internal/cgroups/v1_linux_test.go +++ b/internal/cgroups/v1_linux_test.go @@ -86,6 +86,7 @@ func TestAddCommand(t *testing.T) { require.NoError(t, v1Manager2.AddCommand(cmd2)) checksum := crc32.ChecksumIEEE([]byte(strings.Join(cmd2.Args(), ""))) + // nolint:staticcheck // we will deprecate the old cgroups config in 15.0 groupID := uint(checksum) % config.Count for _, s := range mock.subsystems { diff --git a/internal/gitaly/config/cgroups/cgroups.go b/internal/gitaly/config/cgroups/cgroups.go index dcfcdd571..00fb9f45e 100644 --- a/internal/gitaly/config/cgroups/cgroups.go +++ b/internal/gitaly/config/cgroups/cgroups.go @@ -2,22 +2,40 @@ package cgroups // Config is a struct for cgroups config type Config struct { - // Count is the number of cgroups to be created at startup - Count uint `toml:"count"` // Mountpoint is where the cgroup filesystem is mounted, usually under /sys/fs/cgroup/ Mountpoint string `toml:"mountpoint"` // HierarchyRoot is the parent cgroup under which Gitaly creates <Count> of cgroups. // A system administrator is expected to create such cgroup/directory under <Mountpoint>/memory // and/or <Mountpoint>/cpu depending on which resource is enabled. HierarchyRoot is expected to // be owned by the user and group Gitaly runs as. - HierarchyRoot string `toml:"hierarchy_root"` - // CPU holds CPU resource configurations - CPU CPU `toml:"cpu"` - // Memory holds memory resource configurations + HierarchyRoot string `toml:"hierarchy_root"` + Repositories Repositories `toml:"repositories"` + // MemoryBytes is the memory limit for the parent cgroup. 0 implies no memory limit. + MemoryBytes int64 `toml:"memory_bytes"` + // CPUShares are the shares of CPU the parent cgroup is allowed to utilize. A value of 1024 + // is full utilization of the CPU. 0 implies no CPU limit. + CPUShares uint64 `toml:"cpu_shares"` + + // Deprecated: No longer supported after 15.0 + Count uint `toml:"count"` + CPU CPU `toml:"cpu"` Memory Memory `toml:"memory"` } +// Repositories configures cgroups to be created that are isolated by repository. +type Repositories struct { + // Count is the number of cgroups that will be created for repository-level isolation + // of git commands. + Count uint `toml:"count"` + // MemoryBytes is the memory limit for each cgroup. 0 implies no memory limit. + MemoryBytes int64 `toml:"memory_bytes"` + // CPUShares are the shares of CPU that each cgroup is allowed to utilize. A value of 1024 + // is full utilization of the CPU. 0 implies no CPU limit. + CPUShares uint64 `toml:"cpu_shares"` +} + // Memory is a struct storing cgroups memory config +// Deprecated: Not in use after 15.0. type Memory struct { Enabled bool `toml:"enabled"` // Limit is the memory limit in bytes. Could be -1 to indicate unlimited memory. @@ -25,6 +43,7 @@ type Memory struct { } // CPU is a struct storing cgroups CPU config +// Deprecated: Not in use after 15.0. type CPU struct { Enabled bool `toml:"enabled"` // Shares is the number of CPU shares (relative weight (ratio) vs. other cgroups with CPU shares). diff --git a/internal/gitaly/config/config.go b/internal/gitaly/config/config.go index 8e9328104..96943a850 100644 --- a/internal/gitaly/config/config.go +++ b/internal/gitaly/config/config.go @@ -283,6 +283,14 @@ func (cfg *Cfg) setDefaults() error { cfg.DailyMaintenance = defaultMaintenanceWindow(cfg.Storages) } + if cfg.Cgroups.Mountpoint == "" { + cfg.Cgroups.Mountpoint = "/sys/fs/cgroup" + } + + if cfg.Cgroups.HierarchyRoot == "" { + cfg.Cgroups.HierarchyRoot = "gitaly" + } + return nil } @@ -542,24 +550,12 @@ func (cfg *Cfg) validateMaintenance() error { func (cfg *Cfg) validateCgroups() error { cg := cfg.Cgroups - if cg.Count == 0 { - return nil - } - - if cg.Mountpoint == "" { - return fmt.Errorf("cgroups.mountpoint: cannot be empty") - } - - if cg.HierarchyRoot == "" { - return fmt.Errorf("cgroups.hierarchy_root: cannot be empty") - } - - if cg.CPU.Enabled && cg.CPU.Shares == 0 { - return fmt.Errorf("cgroups.cpu.shares: has to be greater than zero") + if cg.MemoryBytes > 0 && (cg.Repositories.MemoryBytes > cg.MemoryBytes) { + return errors.New("cgroups.repositories: memory limit cannot exceed parent") } - if cg.Memory.Enabled && (cg.Memory.Limit == 0 || cg.Memory.Limit < -1) { - return fmt.Errorf("cgroups.memory.limit: has to be greater than zero or equal to -1") + if cg.MemoryBytes > 0 && (cg.Repositories.CPUShares > cg.CPUShares) { + return errors.New("cgroups.repositories: cpu shares cannot exceed parent") } return nil diff --git a/internal/gitaly/config/config_test.go b/internal/gitaly/config/config_test.go index 763fe4b1c..02fc1855c 100644 --- a/internal/gitaly/config/config_test.go +++ b/internal/gitaly/config/config_test.go @@ -826,139 +826,289 @@ func TestLoadDailyMaintenance(t *testing.T) { } func TestValidateCgroups(t *testing.T) { - for _, tt := range []struct { + type testCase struct { name string rawCfg string expect cgroups.Config validateErr error - }{ - { - name: "enabled success", - rawCfg: `[cgroups] - count = 10 - mountpoint = "/sys/fs/cgroup" - hierarchy_root = "gitaly" - [cgroups.memory] - enabled = true - limit = 1024 - [cgroups.cpu] - enabled = true - shares = 512`, - expect: cgroups.Config{ - Count: 10, - Mountpoint: "/sys/fs/cgroup", - HierarchyRoot: "gitaly", - Memory: cgroups.Memory{ - Enabled: true, - Limit: 1024, + } + + t.Run("old format", func(t *testing.T) { + testCases := []testCase{ + { + name: "enabled success", + rawCfg: `[cgroups] + count = 10 + mountpoint = "/sys/fs/cgroup" + hierarchy_root = "gitaly" + [cgroups.memory] + enabled = true + limit = 1024 + [cgroups.cpu] + enabled = true + shares = 512`, + expect: cgroups.Config{ + Count: 10, + Mountpoint: "/sys/fs/cgroup", + HierarchyRoot: "gitaly", + Memory: cgroups.Memory{ + Enabled: true, + Limit: 1024, + }, + CPU: cgroups.CPU{ + Enabled: true, + Shares: 512, + }, }, - CPU: cgroups.CPU{ - Enabled: true, - Shares: 512, + }, + { + name: "empty mount point", + rawCfg: `[cgroups] + count = 10 + mountpoint = "" + hierarchy_root = "baz" + `, + expect: cgroups.Config{ + Count: 10, + Mountpoint: "/sys/fs/cgroup", + HierarchyRoot: "baz", }, }, - }, - { - name: "disabled success", - rawCfg: `[cgroups] - count = 0`, - expect: cgroups.Config{ - Count: 0, + { + name: "empty hierarchy_root", + rawCfg: `[cgroups] + count = 10 + mountpoint = "/sys/fs/cgroup" + hierarchy_root = ""`, + expect: cgroups.Config{ + Count: 10, + Mountpoint: "/sys/fs/cgroup", + HierarchyRoot: "gitaly", + }, }, - }, - { - name: "empty mount point", - rawCfg: `[cgroups] - count = 10 - mountpoint = ""`, - expect: cgroups.Config{ - Count: 10, - Mountpoint: "", + { + name: "cpu shares - zero", + rawCfg: `[cgroups] + count = 10 + mountpoint = "/sys/fs/cgroup" + hierarchy_root = "gitaly" + [cgroups.memory] + enabled = true + limit = 1024 + [cgroups.cpu] + enabled = true + shares = 0`, + expect: cgroups.Config{ + Count: 10, + Mountpoint: "/sys/fs/cgroup", + HierarchyRoot: "gitaly", + Memory: cgroups.Memory{ + Enabled: true, + Limit: 1024, + }, + CPU: cgroups.CPU{ + Enabled: true, + Shares: 0, + }, + }, }, - validateErr: errors.New("cgroups.mountpoint: cannot be empty"), - }, - { - name: "empty hierarchy_root", - rawCfg: `[cgroups] - count = 10 - mountpoint = "/sys/fs/cgroup" - hierarchy_root = ""`, - expect: cgroups.Config{ - Count: 10, - Mountpoint: "/sys/fs/cgroup", - HierarchyRoot: "", + { + name: "memory limit - zero", + rawCfg: `[cgroups] + count = 10 + mountpoint = "/sys/fs/cgroup" + hierarchy_root = "gitaly" + [cgroups.memory] + enabled = true + limit = 0 + [cgroups.cpu] + enabled = true + shares = 512 + `, + expect: cgroups.Config{ + Count: 10, + Mountpoint: "/sys/fs/cgroup", + HierarchyRoot: "gitaly", + Memory: cgroups.Memory{ + Enabled: true, + Limit: 0, + }, + CPU: cgroups.CPU{ + Enabled: true, + Shares: 512, + }, + }, }, - validateErr: errors.New("cgroups.hierarchy_root: cannot be empty"), - }, - { - name: "invalid cpu shares", - rawCfg: `[cgroups] - count = 10 - mountpoint = "/sys/fs/cgroup" - hierarchy_root = "gitaly" - [cgroups.cpu] - enabled = true - shares = 0`, - expect: cgroups.Config{ - Count: 10, - Mountpoint: "/sys/fs/cgroup", - HierarchyRoot: "gitaly", - CPU: cgroups.CPU{ - Enabled: true, - Shares: 0, + { + name: "memory limit - negative", + rawCfg: `[cgroups] + count = 10 + mountpoint = "/sys/fs/cgroup" + hierarchy_root = "gitaly" + [cgroups.memory] + enabled = true + limit = -5 + [cgroups.cpu] + enabled = true + shares = 512 + `, + expect: cgroups.Config{ + Count: 10, + Mountpoint: "/sys/fs/cgroup", + HierarchyRoot: "gitaly", + Memory: cgroups.Memory{ + Enabled: true, + Limit: -5, + }, + CPU: cgroups.CPU{ + Enabled: true, + Shares: 512, + }, }, }, - validateErr: errors.New("cgroups.cpu.shares: has to be greater than zero"), - }, - { - name: "invalid memory limit - zero", - rawCfg: `[cgroups] - count = 10 - mountpoint = "/sys/fs/cgroup" - hierarchy_root = "gitaly" - [cgroups.memory] - enabled = true - limit = 0`, - expect: cgroups.Config{ - Count: 10, - Mountpoint: "/sys/fs/cgroup", - HierarchyRoot: "gitaly", - Memory: cgroups.Memory{ - Enabled: true, - Limit: 0, + { + name: "repositories - zero count", + rawCfg: `[cgroups] + mountpoint = "/sys/fs/cgroup" + hierarchy_root = "gitaly" + [cgroups.repositories] + `, + expect: cgroups.Config{ + Mountpoint: "/sys/fs/cgroup", + HierarchyRoot: "gitaly", }, }, - validateErr: errors.New("cgroups.memory.limit: has to be greater than zero or equal to -1"), - }, - { - name: "invalid memory limit - negative", - rawCfg: `[cgroups] - count = 10 - mountpoint = "/sys/fs/cgroup" - hierarchy_root = "gitaly" - [cgroups.memory] - enabled = true - limit = -5`, - expect: cgroups.Config{ - Count: 10, - Mountpoint: "/sys/fs/cgroup", - HierarchyRoot: "gitaly", - Memory: cgroups.Memory{ - Enabled: true, - Limit: -5, + } + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + tmpFile := strings.NewReader(tt.rawCfg) + cfg, err := Load(tmpFile) + require.NoError(t, err) + require.Equal(t, tt.expect, cfg.Cgroups) + require.Equal(t, tt.validateErr, cfg.validateCgroups()) + }) + } + }) + + t.Run("new format", func(t *testing.T) { + testCases := []testCase{ + { + name: "enabled success", + rawCfg: `[cgroups] + mountpoint = "/sys/fs/cgroup" + hierarchy_root = "gitaly" + [cgroups.repositories] + count = 10 + memory_bytes = 1024 + cpu_shares = 512 + `, + expect: cgroups.Config{ + Mountpoint: "/sys/fs/cgroup", + HierarchyRoot: "gitaly", + Repositories: cgroups.Repositories{ + Count: 10, + MemoryBytes: 1024, + CPUShares: 512, + }, }, }, - validateErr: errors.New("cgroups.memory.limit: has to be greater than zero or equal to -1"), - }, - } { - t.Run(tt.name, func(t *testing.T) { - tmpFile := strings.NewReader(tt.rawCfg) - cfg, err := Load(tmpFile) - require.NoError(t, err) - require.Equal(t, tt.expect, cfg.Cgroups) - require.Equal(t, tt.validateErr, cfg.validateCgroups()) - }) - } + { + name: "repositories count is zero", + rawCfg: `[cgroups] + mountpoint = "/sys/fs/cgroup" + hierarchy_root = "gitaly" + [cgroups.repositories] + count = 0 + memory_bytes = 1024 + cpu_shares = 512 + `, + expect: cgroups.Config{ + Mountpoint: "/sys/fs/cgroup", + HierarchyRoot: "gitaly", + Repositories: cgroups.Repositories{ + MemoryBytes: 1024, + CPUShares: 512, + }, + }, + }, + { + name: "memory is zero", + rawCfg: `[cgroups] + mountpoint = "/sys/fs/cgroup" + hierarchy_root = "gitaly" + [cgroups.repositories] + count = 10 + cpu_shares = 512`, + expect: cgroups.Config{ + Mountpoint: "/sys/fs/cgroup", + HierarchyRoot: "gitaly", + Repositories: cgroups.Repositories{ + Count: 10, + CPUShares: 512, + }, + }, + }, + { + name: "repositories memory exceeds parent", + rawCfg: `[cgroups] + mountpoint = "/sys/fs/cgroup" + hierarchy_root = "gitaly" + memory_bytes = 1073741824 + cpu_shares = 1024 + [cgroups.repositories] + count = 10 + memory_bytes = 2147483648 + cpu_shares = 128 + `, + expect: cgroups.Config{ + Mountpoint: "/sys/fs/cgroup", + HierarchyRoot: "gitaly", + MemoryBytes: 1073741824, + CPUShares: 1024, + Repositories: cgroups.Repositories{ + Count: 10, + MemoryBytes: 2147483648, + CPUShares: 128, + }, + }, + validateErr: errors.New("cgroups.repositories: memory limit cannot exceed parent"), + }, + { + name: "repositories cpu exceeds parent", + rawCfg: `[cgroups] + mountpoint = "/sys/fs/cgroup" + hierarchy_root = "gitaly" + memory_bytes = 1073741824 + cpu_shares = 128 + [cgroups.repositories] + count = 10 + memory_bytes = 1024 + cpu_shares = 512 + `, + expect: cgroups.Config{ + Mountpoint: "/sys/fs/cgroup", + HierarchyRoot: "gitaly", + MemoryBytes: 1073741824, + CPUShares: 128, + Repositories: cgroups.Repositories{ + Count: 10, + MemoryBytes: 1024, + CPUShares: 512, + }, + }, + validateErr: errors.New("cgroups.repositories: cpu shares cannot exceed parent"), + }, + } + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + tmpFile := strings.NewReader(tt.rawCfg) + cfg, err := Load(tmpFile) + require.NoError(t, err) + require.Equal(t, tt.expect, cfg.Cgroups) + require.Equal(t, tt.validateErr, cfg.validateCgroups()) + }) + } + }) } func TestConfigurePackObjectsCache(t *testing.T) { |