From 958dc6aafcd7bf0a2c0f2749542d3d7097c6f0b3 Mon Sep 17 00:00:00 2001
From: Hugo Gonzalez Labrador <github@hugo.labkode.com>
Date: Fri, 2 Oct 2020 14:09:01 +0200
Subject: [PATCH] Skip fopen for file change check to avoid network penalty

---
 changelog/unreleased/issue-2969 |  9 +++++++
 internal/archiver/archiver.go   | 48 ++++++++++++++++-----------------
 2 files changed, 33 insertions(+), 24 deletions(-)
 create mode 100644 changelog/unreleased/issue-2969

diff --git a/changelog/unreleased/issue-2969 b/changelog/unreleased/issue-2969
new file mode 100644
index 000000000..2bccc2ed1
--- /dev/null
+++ b/changelog/unreleased/issue-2969
@@ -0,0 +1,9 @@
+Enhancement: Optimize check for unchanged files during backup
+
+During a backup restic skips processing files which have not changed since the last backup run.
+Previously this required opening each file once which can be slow on network filesystems. The
+backup command now checks for file changes before opening a file. This considerably reduces
+the time to create a backup on network filesystems.
+
+https://github.com/restic/restic/issues/2969
+https://github.com/restic/restic/pull/2970
diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go
index f4fe5f7db..ff1628f3a 100644
--- a/internal/archiver/archiver.go
+++ b/internal/archiver/archiver.go
@@ -364,6 +364,30 @@ func (arch *Archiver) Save(ctx context.Context, snPath, target string, previous
 		debug.Log("  %v regular file", target)
 		start := time.Now()
 
+		// check if the file has not changed before performing a fopen operation (more expensive, specially
+		// in network filesystems)
+		if previous != nil && !fileChanged(fi, previous, arch.IgnoreInode) {
+			if arch.allBlobsPresent(previous) {
+				debug.Log("%v hasn't changed, using old list of blobs", target)
+				arch.CompleteItem(snPath, previous, previous, ItemStats{}, time.Since(start))
+				arch.CompleteBlob(snPath, previous.Size)
+				fn.node, err = arch.nodeFromFileInfo(target, fi)
+				if err != nil {
+					return FutureNode{}, false, err
+				}
+
+				// copy list of blobs
+				fn.node.Content = previous.Content
+
+				return fn, false, nil
+			}
+
+			debug.Log("%v hasn't changed, but contents are missing!", target)
+			// There are contents missing - inform user!
+			err := errors.Errorf("parts of %v not found in the repository index; storing the file again", target)
+			arch.error(abstarget, fi, err)
+		}
+
 		// reopen file and do an fstat() on the open file to check it is still
 		// a file (and has not been exchanged for e.g. a symlink)
 		file, err := arch.FS.OpenFile(target, fs.O_RDONLY|fs.O_NOFOLLOW, 0)
@@ -398,30 +422,6 @@ func (arch *Archiver) Save(ctx context.Context, snPath, target string, previous
 			return FutureNode{}, true, nil
 		}
 
-		// use previous list of blobs if the file hasn't changed
-		if previous != nil && !fileChanged(fi, previous, arch.IgnoreInode) {
-			if arch.allBlobsPresent(previous) {
-				debug.Log("%v hasn't changed, using old list of blobs", target)
-				arch.CompleteItem(snPath, previous, previous, ItemStats{}, time.Since(start))
-				arch.CompleteBlob(snPath, previous.Size)
-				fn.node, err = arch.nodeFromFileInfo(target, fi)
-				if err != nil {
-					return FutureNode{}, false, err
-				}
-
-				// copy list of blobs
-				fn.node.Content = previous.Content
-
-				_ = file.Close()
-				return fn, false, nil
-			}
-
-			debug.Log("%v hasn't changed, but contents are missing!", target)
-			// There are contents missing - inform user!
-			err := errors.Errorf("parts of %v not found in the repository index; storing the file again", target)
-			arch.error(abstarget, fi, err)
-		}
-
 		fn.isFile = true
 		// Save will close the file, we don't need to do that
 		fn.file = arch.fileSaver.Save(ctx, snPath, file, fi, func() {