diff options
author | Jacob Vosmaer (GitLab) <jacob@gitlab.com> | 2018-03-05 21:33:46 +0300 |
---|---|---|
committer | Zeger-Jan van de Weg <zegerjan@gitlab.com> | 2018-03-05 21:33:46 +0300 |
commit | 3b751d3116d21ed1a8739bfa2855a8ff23dea3b5 (patch) | |
tree | 30a4d849d1b48181dbf0e17be63abd3583f1e4c7 | |
parent | 37c8b66e3ad87e68d18c677f0155fc01c8e6d4cb (diff) |
Use only 1 gitaly-ruby process in test
-rw-r--r-- | CHANGELOG.md | 2 | ||||
-rw-r--r-- | doc/configuration/README.md | 1 | ||||
-rw-r--r-- | internal/config/config_test.go | 25 | ||||
-rw-r--r-- | internal/config/ruby.go | 6 | ||||
-rw-r--r-- | internal/rubyserver/rubyserver.go | 2 | ||||
-rw-r--r-- | internal/rubyserver/worker.go | 7 | ||||
-rw-r--r-- | internal/testhelper/testhelper.go | 6 |
7 files changed, 41 insertions, 8 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index f367e2e28..160d5c73b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ UNRELEASED +- Use only 1 gitaly-ruby process in test + https://gitlab.com/gitlab-org/gitaly/merge_requests/615 - Bump github-linguist to 5.3.3 https://gitlab.com/gitlab-org/gitaly/merge_requests/613 diff --git a/doc/configuration/README.md b/doc/configuration/README.md index a5e9b5265..8dae81773 100644 --- a/doc/configuration/README.md +++ b/doc/configuration/README.md @@ -110,6 +110,7 @@ max\_rss limit. |max_rss|integer|no|Resident set size limit that triggers a gitaly-ruby restart, in bytes. Default 300MB.| |graceful_restart_timeout|string|no|Grace period to allow a gitaly-ruby process to finish ongoing requests. Default 10 minutes ("10m").| |restart_delay|string|no|Time memory must be high before a restart is triggered, in seconds. Default 5 minutes ("5m").| +|num_workers|integer|no|Number of gitaly-ruby worker processes. Default 2, minimum 2.| ## Environment variables diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 3e50a8e64..cdd8f3561 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -2,6 +2,7 @@ package config import ( "bytes" + "fmt" "io" "io/ioutil" "os" @@ -381,6 +382,30 @@ func TestConfigureRuby(t *testing.T) { } } +func TestConfigureRubyNumWorkers(t *testing.T) { + defer func(oldRuby Ruby) { + Config.Ruby = oldRuby + }(Config.Ruby) + + testCases := []struct { + in, out int + }{ + {in: -1, out: 2}, + {in: 0, out: 2}, + {in: 1, out: 2}, + {in: 2, out: 2}, + {in: 3, out: 3}, + } + + for _, tc := range testCases { + t.Run(fmt.Sprintf("%+v", tc), func(t *testing.T) { + Config.Ruby = Ruby{Dir: "/", NumWorkers: tc.in} + require.NoError(t, ConfigureRuby()) + require.Equal(t, tc.out, Config.Ruby.NumWorkers) + }) + } +} + func TestValidateListeners(t *testing.T) { defer func(cfg config) { Config = cfg diff --git a/internal/config/ruby.go b/internal/config/ruby.go index 4d65939b2..462dd93a6 100644 --- a/internal/config/ruby.go +++ b/internal/config/ruby.go @@ -13,6 +13,7 @@ type Ruby struct { GracefulRestartTimeoutToml duration `toml:"graceful_restart_timeout"` RestartDelay time.Duration RestartDelayToml duration `toml:"restart_delay"` + NumWorkers int `toml:"num_workers"` } // This type is a trick to let our TOML library parse durations from strings. @@ -46,5 +47,10 @@ func ConfigureRuby() error { return fmt.Errorf("gitaly-ruby.dir is not set") } + minWorkers := 2 + if Config.Ruby.NumWorkers < minWorkers { + Config.Ruby.NumWorkers = minWorkers + } + return validateIsDirectory(Config.Ruby.Dir, "gitaly-ruby.dir") } diff --git a/internal/rubyserver/rubyserver.go b/internal/rubyserver/rubyserver.go index 35dcdcca0..a333432ae 100644 --- a/internal/rubyserver/rubyserver.go +++ b/internal/rubyserver/rubyserver.go @@ -114,7 +114,7 @@ func Start() (*Server, error) { gitalyRuby := path.Join(cfg.Ruby.Dir, "bin/gitaly-ruby") s := &Server{} - for i := 0; i < numWorkers; i++ { + for i := 0; i < cfg.Ruby.NumWorkers; i++ { name := fmt.Sprintf("gitaly-ruby.%d", i) socketPath := socketPath(i) diff --git a/internal/rubyserver/worker.go b/internal/rubyserver/worker.go index 41de27a6d..a06e09618 100644 --- a/internal/rubyserver/worker.go +++ b/internal/rubyserver/worker.go @@ -13,13 +13,6 @@ import ( log "github.com/sirupsen/logrus" ) -const ( - // We might want numWorkers to be configurable in the future. But at the - // moment we only support 'pick first' balancing so there is no point in - // having more than 2 workers yet. - numWorkers = 2 -) - var ( terminationCounter = prometheus.NewCounterVec( prometheus.CounterOpts{ diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go index 0b1388566..5b0c4957b 100644 --- a/internal/testhelper/testhelper.go +++ b/internal/testhelper/testhelper.go @@ -260,6 +260,12 @@ func ConfigureRuby() { if err := config.ConfigureRuby(); err != nil { log.Fatal("validate ruby config: %v", err) } + + // This speeds up test boot-up. The reason we use more than 1 worker in + // production is to handle memory leaks, but the tests don't run for long + // enough to hit those memory leaks anyway. Most tests also run faster + // than the minimum delay before a ruby worker failover can even happen. + config.Config.Ruby.NumWorkers = 1 } // NewTestGrpcServer creates a GRPC Server for testing purposes |