From 5224f79d7d048aecf77e84ba02b6e66484fe32da Mon Sep 17 00:00:00 2001
From: Lukas Rieger <0x53A@users.noreply.github.com>
Date: Fri, 26 Nov 2021 22:10:44 +0100
Subject: [PATCH] [snapshot restore] use non-atomic writes (#1534)
* don't flush every file two times on snapshot restore
---
cli/command_restore.go | 3 +++
htmlui/src/BeginRestore.js | 5 +++++
snapshot/restore/local_fs_output.go | 35 +++++++++++++++++++++++++++--
3 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/cli/command_restore.go b/cli/command_restore.go
index ceb1e7740..94392aebf 100644
--- a/cli/command_restore.go
+++ b/cli/command_restore.go
@@ -105,6 +105,7 @@ type commandRestore struct {
restoreMode string
restoreParallel int
restoreIgnorePermissionErrors bool
+ restoreWriteFilesAtomically bool
restoreSkipTimes bool
restoreSkipOwners bool
restoreSkipPermissions bool
@@ -131,6 +132,7 @@ func (c *commandRestore) setup(svc appServices, parent commandParent) {
cmd.Flag("skip-permissions", "Skip permissions during restore").BoolVar(&c.restoreSkipPermissions)
cmd.Flag("skip-times", "Skip times during restore").BoolVar(&c.restoreSkipTimes)
cmd.Flag("ignore-permission-errors", "Ignore permission errors").Default("true").BoolVar(&c.restoreIgnorePermissionErrors)
+ cmd.Flag("write-files-atomically", "Write files atomically to disk, ensuring they are either fully committed, or not written at all, preventing partially written files").Default("false").BoolVar(&c.restoreWriteFilesAtomically)
cmd.Flag("ignore-errors", "Ignore all errors").BoolVar(&c.restoreIgnoreErrors)
cmd.Flag("skip-existing", "Skip files and symlinks that exist in the output").BoolVar(&c.restoreIncremental)
cmd.Flag("shallow", "Shallow restore the directory hierarchy starting at this level (default is to deep restore the entire hierarchy.)").Int32Var(&c.restoreShallowAtDepth)
@@ -214,6 +216,7 @@ func (c *commandRestore) restoreOutput(ctx context.Context) (restore.Output, err
OverwriteFiles: c.restoreOverwriteFiles,
OverwriteSymlinks: c.restoreOverwriteSymlinks,
IgnorePermissionErrors: c.restoreIgnorePermissionErrors,
+ WriteFilesAtomically: c.restoreWriteFilesAtomically,
SkipOwners: c.restoreSkipOwners,
SkipPermissions: c.restoreSkipPermissions,
SkipTimes: c.restoreSkipTimes,
diff --git a/htmlui/src/BeginRestore.js b/htmlui/src/BeginRestore.js
index 3f65fac80..2e3f582ab 100644
--- a/htmlui/src/BeginRestore.js
+++ b/htmlui/src/BeginRestore.js
@@ -23,6 +23,7 @@ export class BeginRestore extends Component {
overwriteDirectories: false,
overwriteSymlinks: false,
ignorePermissionErrors: true,
+ writeFilesAtomically: false,
restoreDirEntryAtDepth: 1000,
minSizeForPlaceholder: 0,
restoreTask: "",
@@ -67,6 +68,7 @@ export class BeginRestore extends Component {
overwriteFiles: this.state.overwriteFiles,
overwriteDirectories: this.state.overwriteDirectories,
overwriteSymlinks: this.state.overwriteSymlinks,
+ writeFilesAtomically: this.state.writeFilesAtomically
}
}
@@ -123,6 +125,9 @@ export class BeginRestore extends Component {
{RequiredBoolean(this, "Overwrite Symbolic Links", "overwriteSymlinks")}
+
+ {RequiredBoolean(this, "Write files atomically", "writeFilesAtomically")}
+
diff --git a/snapshot/restore/local_fs_output.go b/snapshot/restore/local_fs_output.go
index ad653891e..35addddaa 100644
--- a/snapshot/restore/local_fs_output.go
+++ b/snapshot/restore/local_fs_output.go
@@ -14,6 +14,7 @@
"github.com/kopia/kopia/fs"
"github.com/kopia/kopia/fs/localfs"
"github.com/kopia/kopia/internal/atomicfile"
+ "github.com/kopia/kopia/internal/iocopy"
"github.com/kopia/kopia/snapshot"
)
@@ -44,6 +45,9 @@ type FilesystemOutput struct {
// IgnorePermissionErrors causes restore to ignore errors due to invalid permissions.
IgnorePermissionErrors bool `json:"ignorePermissionErrors"`
+ // When set to true, first write to a temp file and rename it, to ensure there are no partially written files in case of a crash.
+ WriteFilesAtomically bool `json:"writeFilesAtomically"`
+
// SkipOwners when set to true causes restore to skip restoring owner information.
SkipOwners bool `json:"skipOwners"`
@@ -299,6 +303,29 @@ func (o *FilesystemOutput) createDirectory(ctx context.Context, path string) err
}
}
+func write(targetPath string, r fs.Reader) error {
+ f, err := os.OpenFile(targetPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o600) //nolint:gosec,gomnd
+ if err != nil {
+ return err //nolint:wrapcheck
+ }
+
+ // ensure we always close f. Note that this does not conflict with the
+ // close below, as close is idempotent.
+ defer f.Close() //nolint:errcheck,gosec
+
+ name := f.Name()
+
+ if err := iocopy.JustCopy(f, r); err != nil {
+ return errors.Wrap(err, "cannot write data to file %q "+name)
+ }
+
+ if err := f.Close(); err != nil {
+ return err //nolint:wrapcheck
+ }
+
+ return nil
+}
+
func (o *FilesystemOutput) copyFileContent(ctx context.Context, targetPath string, f fs.File) error {
switch _, err := os.Stat(targetPath); {
case os.IsNotExist(err): // copy file below
@@ -320,8 +347,12 @@ func (o *FilesystemOutput) copyFileContent(ctx context.Context, targetPath strin
log(ctx).Debugf("copying file contents to: %v", targetPath)
- // nolint:wrapcheck
- return atomicfile.Write(targetPath, r)
+ if o.WriteFilesAtomically {
+ // nolint:wrapcheck
+ return atomicfile.Write(targetPath, r)
+ }
+
+ return write(atomicfile.MaybePrefixLongFilenameOnWindows(targetPath), r)
}
func isEmptyDirectory(name string) (bool, error) {