diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-10-06 09:56:06 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-10-07 10:13:22 +0300 |
commit | ff726e130fc73c2b47419ac836a5112cc9d3a181 (patch) | |
tree | a13fc8fdec2b20a67af0974f7be83e0a6ae77769 | |
parent | a980fb11b294e3eff34d793effd7780bcbc4fc90 (diff) |
gitaly-backup: Inject connection pool to unleak its Goroutines
The gitaly-backup manager currently creates an ad-hoc instance of the
connection pool. This pool never gets cleaned up and thus results in
leakage of its connections.
Inject the connection pool into the manager and clean it up as required.
-rw-r--r-- | cmd/gitaly-backup/create.go | 6 | ||||
-rw-r--r-- | cmd/gitaly-backup/restore.go | 6 | ||||
-rw-r--r-- | internal/backup/backup.go | 4 | ||||
-rw-r--r-- | internal/backup/backup_test.go | 10 |
4 files changed, 20 insertions, 6 deletions
diff --git a/cmd/gitaly-backup/create.go b/cmd/gitaly-backup/create.go index 9dae3d4a5..0ceac7d8e 100644 --- a/cmd/gitaly-backup/create.go +++ b/cmd/gitaly-backup/create.go @@ -9,6 +9,7 @@ import ( "runtime" log "github.com/sirupsen/logrus" + "gitlab.com/gitlab-org/gitaly/v14/client" "gitlab.com/gitlab-org/gitaly/v14/internal/backup" "gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb" @@ -46,7 +47,10 @@ func (cmd *createSubcommand) Run(ctx context.Context, stdin io.Reader, stdout io return fmt.Errorf("create: resolve locator: %w", err) } - manager := backup.NewManager(sink, locator) + pool := client.NewPool() + defer pool.Close() + + manager := backup.NewManager(sink, locator, pool) var pipeline backup.Pipeline pipeline = backup.NewLoggingPipeline(log.StandardLogger()) diff --git a/cmd/gitaly-backup/restore.go b/cmd/gitaly-backup/restore.go index 695e98cfb..6e119ba92 100644 --- a/cmd/gitaly-backup/restore.go +++ b/cmd/gitaly-backup/restore.go @@ -10,6 +10,7 @@ import ( "runtime" log "github.com/sirupsen/logrus" + "gitlab.com/gitlab-org/gitaly/v14/client" "gitlab.com/gitlab-org/gitaly/v14/internal/backup" "gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb" @@ -48,7 +49,10 @@ func (cmd *restoreSubcommand) Run(ctx context.Context, stdin io.Reader, stdout i return fmt.Errorf("restore: resolve locator: %w", err) } - manager := backup.NewManager(sink, locator) + pool := client.NewPool() + defer pool.Close() + + manager := backup.NewManager(sink, locator, pool) var pipeline backup.Pipeline pipeline = backup.NewLoggingPipeline(log.StandardLogger()) diff --git a/internal/backup/backup.go b/internal/backup/backup.go index c59591b59..9543a4483 100644 --- a/internal/backup/backup.go +++ b/internal/backup/backup.go @@ -122,10 +122,10 @@ type Manager struct { } // NewManager creates and returns initialized *Manager instance. -func NewManager(sink Sink, locator Locator) *Manager { +func NewManager(sink Sink, locator Locator, pool *client.Pool) *Manager { return &Manager{ sink: sink, - conns: client.NewPool(), + conns: pool, locator: locator, backupID: time.Now().UTC().Format("20060102150405"), } diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go index ab8304965..059f9267c 100644 --- a/internal/backup/backup_test.go +++ b/internal/backup/backup_test.go @@ -89,7 +89,10 @@ func TestManager_Create(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - fsBackup := NewManager(NewFilesystemSink(path), LegacyLocator{}) + pool := client.NewPool() + defer testhelper.MustClose(t, pool) + + fsBackup := NewManager(NewFilesystemSink(path), LegacyLocator{}, pool) err := fsBackup.Create(ctx, &CreateRequest{ Server: storage.ServerInfo{Address: gitalyAddr, Token: cfg.Auth.Token}, Repository: tc.repo, @@ -351,11 +354,14 @@ func testManagerRestore(t *testing.T, cfg config.Cfg, gitalyAddr string) { repo, bundles := tc.setup(t) repoPath := filepath.Join(cfg.Storages[0].Path, repo.RelativePath) + pool := client.NewPool() + defer testhelper.MustClose(t, pool) + sink := NewFilesystemSink(path) locator, err := ResolveLocator(locatorName, sink) require.NoError(t, err) - fsBackup := NewManager(sink, locator) + fsBackup := NewManager(sink, locator, pool) err = fsBackup.Restore(ctx, &RestoreRequest{ Server: storage.ServerInfo{Address: gitalyAddr, Token: cfg.Auth.Token}, Repository: repo, |