diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-05-04 10:20:31 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-05-05 10:36:35 +0300 |
commit | b25bd0f037a2b8a4ee1973c5831a588615a3d990 (patch) | |
tree | b4a562eb276cc776273033154967c184e6e76720 | |
parent | dd2e90412e63dceb13e58638549c203ab5f6723c (diff) |
gitaly: Shorten maximum internal socket path length
With the migration to the new runtime directory, Gitaly has moved its
internal socket directory as well. Unfortunately, this change resulted
in an expansion of the internal socket path length, and because Unix
systems put a limit on Unix socket paths this has caused Gitaly to not
come up on some systems anymore.
This problem is entirely fixable by the administrator by configuring a
different runtime directory path that has a shorter prefix. But we can
at least try to shorten the paths we generate a bit. Right now, Gitaly
creates three different types of internal sockets:
- The internal socket that is used e.g. by `gitaly-hooks` to connect
back to Gitaly. This file is created as `internal_${PID}.sock`.
Including the PID is not required anymore though: the runtime
directory always contains `gitaly-${PID}` anyway. So we can easily
shorten this to just `intern`. This leaves us with a socket name
length of 6.
- The Ruby worker sockets, which are created as `ruby.$N`. `$N` is
the number of any worker, and typically shouldn't be larger than
in the tens. This leaves us with a typical socket name length of
7.
- The internal test socket that is created to verify whether we can
create and connect to internal sockets. This is done as a sanity
check to alert administrators early on in case the socket path
length exceeds the system's limits. Right now it's created as
`test-XXXX.sock`, but given that we're about to change the naming
strategy we must only ensure that it's as long as the longest
socket we're creating. Furthermore, we don't need to create it
with a random part anymore given that the runtime directory is
keyed by PID. We thus use `tsocket`, which has a socket name
length of 7.
With these changes in place we reduce the maximum socket name length
from 19 characters to 7 characters, which gives us administrators 12
characters more room to play with.
-rw-r--r-- | internal/gitaly/config/config.go | 20 | ||||
-rw-r--r-- | internal/gitaly/config/config_test.go | 4 |
2 files changed, 12 insertions, 12 deletions
diff --git a/internal/gitaly/config/config.go b/internal/gitaly/config/config.go index 96943a850..8ed64b565 100644 --- a/internal/gitaly/config/config.go +++ b/internal/gitaly/config/config.go @@ -18,7 +18,6 @@ import ( internallog "gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config/log" "gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config/prometheus" "gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config/sentry" - "gitlab.com/gitlab-org/gitaly/v14/internal/helper/text" ) const ( @@ -399,7 +398,7 @@ func (cfg *Cfg) InternalSocketDir() string { // InternalSocketPath is the path to the internal Gitaly socket. func (cfg *Cfg) InternalSocketPath() string { - return filepath.Join(cfg.InternalSocketDir(), fmt.Sprintf("internal_%d.sock", os.Getpid())) + return filepath.Join(cfg.InternalSocketDir(), "intern") } func (cfg *Cfg) validateBinDir() error { @@ -478,7 +477,7 @@ func (cfg *Cfg) validateInternalSocketDir() error { } if err := trySocketCreation(cfg.InternalSocketDir()); err != nil { - return fmt.Errorf("internal_socket_dir: try create socket: %w", err) + return fmt.Errorf("failed creating internal test socket: %w", err) } return nil @@ -487,13 +486,14 @@ func (cfg *Cfg) validateInternalSocketDir() error { func trySocketCreation(dir string) error { // To validate the socket can actually be created, we open and close a socket. // Any error will be assumed persistent for when the gitaly-ruby sockets are created - // and thus fatal at boot time - b, err := text.RandomHex(4) - if err != nil { - return err - } - - socketPath := filepath.Join(dir, fmt.Sprintf("test-%s.sock", b)) + // and thus fatal at boot time. + // + // There are two kinds of internal sockets we create: the internal server socket + // called "intern", and then the Ruby worker sockets called "ruby.$N", with "$N" + // being the number of the Ruby worker. Given that we typically wouldn't spawn + // hundreds of Ruby workers, the maximum internal socket path name would thus be 7 + // characters long. + socketPath := filepath.Join(dir, "tsocket") defer func() { _ = os.Remove(socketPath) }() // Attempt to create an actual socket and not just a file to catch socket path length problems diff --git a/internal/gitaly/config/config_test.go b/internal/gitaly/config/config_test.go index 02fc1855c..d683fa352 100644 --- a/internal/gitaly/config/config_test.go +++ b/internal/gitaly/config/config_test.go @@ -680,9 +680,9 @@ func TestValidateInternalSocketDir(t *testing.T) { require.Error(t, actualErr) require.Regexp(t, fmt.Sprintf( - "internal_socket_dir: try create socket: socket could not be created in %s: listen unix %s: bind: invalid argument", + "failed creating internal test socket: socket could not be created in %s: listen unix %s: bind: invalid argument", filepath.Join(runtimeDir, "sock\\.d"), - filepath.Join(runtimeDir, "sock\\.d", "test-.{8}\\.sock"), + filepath.Join(runtimeDir, "sock\\.d", "tsocket"), ), actualErr.Error(), ) |