diff options
author | Quang-Minh Nguyen <qmnguyen@gitlab.com> | 2023-07-12 09:23:50 +0300 |
---|---|---|
committer | Quang-Minh Nguyen <qmnguyen@gitlab.com> | 2023-07-12 09:23:50 +0300 |
commit | ea4a95606d3647943f63609a9b03dcfad8467be9 (patch) | |
tree | fae78592ef240ab3924a3b3cf0349f47bc7c3918 | |
parent | c6808769ff2763037ab7765a349e328e9788eb91 (diff) | |
parent | db1f9b7ab83e264701509fc306285e730931961d (diff) |
Merge branch 'backup_goroutine_leak' into 'master'
Ensure backup.Sink get cleaned up properly
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/6046
Merged-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Reviewed-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Reviewed-by: John Cai <jcai@gitlab.com>
Co-authored-by: James Fargher <jfargher@gitlab.com>
-rw-r--r-- | internal/backup/backup.go | 1 | ||||
-rw-r--r-- | internal/backup/backup_test.go | 12 | ||||
-rw-r--r-- | internal/backup/filesystem_sink.go | 5 | ||||
-rw-r--r-- | internal/backup/sink_test.go | 2 | ||||
-rw-r--r-- | internal/backup/testhelper_test.go | 4 |
5 files changed, 21 insertions, 3 deletions
diff --git a/internal/backup/backup.go b/internal/backup/backup.go index 78caa51f3..c06a17110 100644 --- a/internal/backup/backup.go +++ b/internal/backup/backup.go @@ -26,6 +26,7 @@ var ( // Sink is an abstraction over the real storage used for storing/restoring backups. type Sink interface { + io.Closer // GetWriter saves the written data to relativePath. It is the callers // responsibility to call Close and check any subsequent errors. GetWriter(ctx context.Context, relativePath string) (io.WriteCloser, error) diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go index 70cea1550..81fb28d77 100644 --- a/internal/backup/backup_test.go +++ b/internal/backup/backup_test.go @@ -48,6 +48,8 @@ func TestManager_RemoveAllRepositories(t *testing.T) { backupRoot := testhelper.TempDir(t) sink := backup.NewFilesystemSink(backupRoot) + defer testhelper.MustClose(t, sink) + locator, err := backup.ResolveLocator("pointer", sink) require.NoError(t, err) @@ -169,6 +171,8 @@ func TestManager_Create(t *testing.T) { customHooksPath := joinBackupPath(t, backupRoot, vanityRepo, backupID, "001.custom_hooks.tar") sink := backup.NewFilesystemSink(backupRoot) + defer testhelper.MustClose(t, sink) + locator, err := backup.ResolveLocator("pointer", sink) require.NoError(t, err) @@ -335,6 +339,8 @@ func TestManager_Create_incremental(t *testing.T) { bundlePath := joinBackupPath(t, backupRoot, repo, backupID, tc.expectedIncrement+".bundle") sink := backup.NewFilesystemSink(backupRoot) + defer testhelper.MustClose(t, sink) + locator, err := backup.ResolveLocator("pointer", sink) require.NoError(t, err) @@ -587,6 +593,8 @@ func TestManager_Restore_latest(t *testing.T) { repo, expectedChecksum := tc.setup(t) sink := backup.NewFilesystemSink(backupRoot) + defer testhelper.MustClose(t, sink) + locator, err := backup.ResolveLocator(locatorName, sink) require.NoError(t, err) @@ -776,6 +784,8 @@ func TestManager_Restore_specific(t *testing.T) { repo, expectedChecksum := tc.setup(t) sink := backup.NewFilesystemSink(backupRoot) + defer testhelper.MustClose(t, sink) + locator, err := backup.ResolveLocator("pointer", sink) require.NoError(t, err) @@ -841,6 +851,8 @@ func TestManager_CreateRestore_contextServerInfo(t *testing.T) { defer testhelper.MustClose(t, pool) sink := backup.NewFilesystemSink(backupRoot) + defer testhelper.MustClose(t, sink) + locator, err := backup.ResolveLocator("pointer", sink) require.NoError(t, err) diff --git a/internal/backup/filesystem_sink.go b/internal/backup/filesystem_sink.go index 6eb86c06d..6d68502db 100644 --- a/internal/backup/filesystem_sink.go +++ b/internal/backup/filesystem_sink.go @@ -54,3 +54,8 @@ func (fs *FilesystemSink) GetReader(ctx context.Context, relativePath string) (i } return f, nil } + +// Close is a no-op to implement the Sink interface +func (fs *FilesystemSink) Close() error { + return nil +} diff --git a/internal/backup/sink_test.go b/internal/backup/sink_test.go index 24ed35ddb..0c40ee6ff 100644 --- a/internal/backup/sink_test.go +++ b/internal/backup/sink_test.go @@ -104,6 +104,8 @@ func TestResolveSink(t *testing.T) { require.EqualError(t, err, tc.errMsg) return } + defer testhelper.MustClose(t, sink) + tc.verify(t, sink) }) } diff --git a/internal/backup/testhelper_test.go b/internal/backup/testhelper_test.go index 2ef254a27..53f50c4ae 100644 --- a/internal/backup/testhelper_test.go +++ b/internal/backup/testhelper_test.go @@ -7,7 +7,5 @@ import ( ) func TestMain(m *testing.M) { - // gocloud.dev/blob leaks the HTTP connection even if we make sure to close all buckets. - //nolint:staticcheck - testhelper.Run(m, testhelper.WithDisabledGoroutineChecker()) + testhelper.Run(m) } |