From e36a40db10e58ccc15c791988db1ec453648f14c Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 1 May 2022 20:07:29 +0200 Subject: [PATCH] upgrade_repo_v2: Use atomic replace for supported backends --- internal/backend/azure/azure.go | 5 ++++ internal/backend/b2/b2.go | 5 ++++ internal/backend/dryrun/dry_backend.go | 4 ++++ internal/backend/gs/gs.go | 5 ++++ internal/backend/local/local.go | 5 ++++ internal/backend/mem/mem_backend.go | 5 ++++ internal/backend/rest/rest.go | 6 +++++ internal/backend/s3/s3.go | 5 ++++ internal/backend/sftp/sftp.go | 6 +++++ internal/backend/swift/swift.go | 5 ++++ internal/migrations/upgrade_repo_v2.go | 12 ++++++---- internal/mock/backend.go | 33 ++++++++++++++++---------- internal/restic/backend.go | 3 +++ 13 files changed, 82 insertions(+), 17 deletions(-) diff --git a/internal/backend/azure/azure.go b/internal/backend/azure/azure.go index 243a1eaef..ff89a6b01 100644 --- a/internal/backend/azure/azure.go +++ b/internal/backend/azure/azure.go @@ -125,6 +125,11 @@ func (be *Backend) Hasher() hash.Hash { return md5.New() } +// HasAtomicReplace returns whether Save() can atomically replace files +func (be *Backend) HasAtomicReplace() bool { + return true +} + // Path returns the path in the bucket that is used for this backend. func (be *Backend) Path() string { return be.prefix diff --git a/internal/backend/b2/b2.go b/internal/backend/b2/b2.go index 6108aaf5c..7f8019a74 100644 --- a/internal/backend/b2/b2.go +++ b/internal/backend/b2/b2.go @@ -147,6 +147,11 @@ func (be *b2Backend) Hasher() hash.Hash { return nil } +// HasAtomicReplace returns whether Save() can atomically replace files +func (be *b2Backend) HasAtomicReplace() bool { + return true +} + // IsNotExist returns true if the error is caused by a non-existing file. func (be *b2Backend) IsNotExist(err error) bool { return b2.IsNotExist(errors.Cause(err)) diff --git a/internal/backend/dryrun/dry_backend.go b/internal/backend/dryrun/dry_backend.go index 44eee9a45..31012df43 100644 --- a/internal/backend/dryrun/dry_backend.go +++ b/internal/backend/dryrun/dry_backend.go @@ -67,6 +67,10 @@ func (be *Backend) Hasher() hash.Hash { return be.b.Hasher() } +func (be *Backend) HasAtomicReplace() bool { + return be.b.HasAtomicReplace() +} + func (be *Backend) IsNotExist(err error) bool { return be.b.IsNotExist(err) } diff --git a/internal/backend/gs/gs.go b/internal/backend/gs/gs.go index c87211be3..92de75887 100644 --- a/internal/backend/gs/gs.go +++ b/internal/backend/gs/gs.go @@ -201,6 +201,11 @@ func (be *Backend) Hasher() hash.Hash { return md5.New() } +// HasAtomicReplace returns whether Save() can atomically replace files +func (be *Backend) HasAtomicReplace() bool { + return true +} + // Path returns the path in the bucket that is used for this backend. func (be *Backend) Path() string { return be.prefix diff --git a/internal/backend/local/local.go b/internal/backend/local/local.go index 77bc026ad..22fb8c8e5 100644 --- a/internal/backend/local/local.go +++ b/internal/backend/local/local.go @@ -102,6 +102,11 @@ func (b *Local) Hasher() hash.Hash { return nil } +// HasAtomicReplace returns whether Save() can atomically replace files +func (b *Local) HasAtomicReplace() bool { + return true +} + // IsNotExist returns true if the error is caused by a non existing file. func (b *Local) IsNotExist(err error) bool { return errors.Is(err, os.ErrNotExist) diff --git a/internal/backend/mem/mem_backend.go b/internal/backend/mem/mem_backend.go index 69476b693..b14149d52 100644 --- a/internal/backend/mem/mem_backend.go +++ b/internal/backend/mem/mem_backend.go @@ -268,6 +268,11 @@ func (be *MemoryBackend) Hasher() hash.Hash { return md5.New() } +// HasAtomicReplace returns whether Save() can atomically replace files +func (be *MemoryBackend) HasAtomicReplace() bool { + return false +} + // Delete removes all data in the backend. func (be *MemoryBackend) Delete(ctx context.Context) error { be.m.Lock() diff --git a/internal/backend/rest/rest.go b/internal/backend/rest/rest.go index 1e372229a..b9824bb53 100644 --- a/internal/backend/rest/rest.go +++ b/internal/backend/rest/rest.go @@ -121,6 +121,12 @@ func (b *Backend) Hasher() hash.Hash { return nil } +// HasAtomicReplace returns whether Save() can atomically replace files +func (b *Backend) HasAtomicReplace() bool { + // rest-server prevents overwriting + return false +} + // Save stores data in the backend at the handle. func (b *Backend) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error { if err := h.Valid(); err != nil { diff --git a/internal/backend/s3/s3.go b/internal/backend/s3/s3.go index 1bdf2d795..ac1a1d5ce 100644 --- a/internal/backend/s3/s3.go +++ b/internal/backend/s3/s3.go @@ -269,6 +269,11 @@ func (be *Backend) Hasher() hash.Hash { return nil } +// HasAtomicReplace returns whether Save() can atomically replace files +func (be *Backend) HasAtomicReplace() bool { + return true +} + // Path returns the path in the bucket that is used for this backend. func (be *Backend) Path() string { return be.cfg.Prefix diff --git a/internal/backend/sftp/sftp.go b/internal/backend/sftp/sftp.go index a8a2a185d..ebbaaddad 100644 --- a/internal/backend/sftp/sftp.go +++ b/internal/backend/sftp/sftp.go @@ -267,6 +267,12 @@ func (r *SFTP) Hasher() hash.Hash { return nil } +// HasAtomicReplace returns whether Save() can atomically replace files +func (r *SFTP) HasAtomicReplace() bool { + // we use sftp's 'Rename()' in 'Save()' which does not allow overwriting + return false +} + // Join joins the given paths and cleans them afterwards. This always uses // forward slashes, which is required by sftp. func Join(parts ...string) string { diff --git a/internal/backend/swift/swift.go b/internal/backend/swift/swift.go index 6157002b5..b127cb832 100644 --- a/internal/backend/swift/swift.go +++ b/internal/backend/swift/swift.go @@ -129,6 +129,11 @@ func (be *beSwift) Hasher() hash.Hash { return md5.New() } +// HasAtomicReplace returns whether Save() can atomically replace files +func (be *beSwift) HasAtomicReplace() bool { + return true +} + // Load runs fn with a reader that yields the contents of the file at h at the // given offset. func (be *beSwift) Load(ctx context.Context, h restic.Handle, length int, offset int64, fn func(rd io.Reader) error) error { diff --git a/internal/migrations/upgrade_repo_v2.go b/internal/migrations/upgrade_repo_v2.go index ada77444e..02806e468 100644 --- a/internal/migrations/upgrade_repo_v2.go +++ b/internal/migrations/upgrade_repo_v2.go @@ -53,17 +53,19 @@ func (*UpgradeRepoV2) Check(ctx context.Context, repo restic.Repository) (bool, func (*UpgradeRepoV2) upgrade(ctx context.Context, repo restic.Repository) error { h := restic.Handle{Type: restic.ConfigFile} - // now remove the original file - err := repo.Backend().Remove(ctx, h) - if err != nil { - return fmt.Errorf("remove config failed: %w", err) + if !repo.Backend().HasAtomicReplace() { + // remove the original file for backends which do not support atomic overwriting + err := repo.Backend().Remove(ctx, h) + if err != nil { + return fmt.Errorf("remove config failed: %w", err) + } } // upgrade config cfg := repo.Config() cfg.Version = 2 - _, err = repo.SaveJSONUnpacked(ctx, restic.ConfigFile, cfg) + _, err := repo.SaveJSONUnpacked(ctx, restic.ConfigFile, cfg) if err != nil { return fmt.Errorf("save new config file failed: %w", err) } diff --git a/internal/mock/backend.go b/internal/mock/backend.go index 05fe1dc6e..655499b15 100644 --- a/internal/mock/backend.go +++ b/internal/mock/backend.go @@ -11,18 +11,19 @@ import ( // Backend implements a mock backend. type Backend struct { - CloseFn func() error - IsNotExistFn func(err error) bool - SaveFn func(ctx context.Context, h restic.Handle, rd restic.RewindReader) error - OpenReaderFn func(ctx context.Context, h restic.Handle, length int, offset int64) (io.ReadCloser, error) - StatFn func(ctx context.Context, h restic.Handle) (restic.FileInfo, error) - ListFn func(ctx context.Context, t restic.FileType, fn func(restic.FileInfo) error) error - RemoveFn func(ctx context.Context, h restic.Handle) error - TestFn func(ctx context.Context, h restic.Handle) (bool, error) - DeleteFn func(ctx context.Context) error - ConnectionsFn func() uint - LocationFn func() string - HasherFn func() hash.Hash + CloseFn func() error + IsNotExistFn func(err error) bool + SaveFn func(ctx context.Context, h restic.Handle, rd restic.RewindReader) error + OpenReaderFn func(ctx context.Context, h restic.Handle, length int, offset int64) (io.ReadCloser, error) + StatFn func(ctx context.Context, h restic.Handle) (restic.FileInfo, error) + ListFn func(ctx context.Context, t restic.FileType, fn func(restic.FileInfo) error) error + RemoveFn func(ctx context.Context, h restic.Handle) error + TestFn func(ctx context.Context, h restic.Handle) (bool, error) + DeleteFn func(ctx context.Context) error + ConnectionsFn func() uint + LocationFn func() string + HasherFn func() hash.Hash + HasAtomicReplaceFn func() bool } // NewBackend returns new mock Backend instance @@ -66,6 +67,14 @@ func (m *Backend) Hasher() hash.Hash { return m.HasherFn() } +// HasAtomicReplace returns whether Save() can atomically replace files +func (m *Backend) HasAtomicReplace() bool { + if m.HasAtomicReplaceFn == nil { + return false + } + return m.HasAtomicReplaceFn() +} + // IsNotExist returns true if the error is caused by a missing file. func (m *Backend) IsNotExist(err error) bool { if m.IsNotExistFn == nil { diff --git a/internal/restic/backend.go b/internal/restic/backend.go index 1203bf3d3..6ec10e685 100644 --- a/internal/restic/backend.go +++ b/internal/restic/backend.go @@ -24,6 +24,9 @@ type Backend interface { // Hasher may return a hash function for calculating a content hash for the backend Hasher() hash.Hash + // HasAtomicReplace returns whether Save() can atomically replace files + HasAtomicReplace() bool + // Test a boolean value whether a File with the name and type exists. Test(ctx context.Context, h Handle) (bool, error)