From f99c95c7661f8a45d0ee68be23a8f18c75e7a22b Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Fri, 26 Jan 2018 19:49:43 +0100 Subject: [PATCH 1/2] archiver: Fix intermediate index upload A user discovered[1] that when the backup finishes during the upload of an intermediate index, the upload is cancelled and the index never fully saved, but the snapshot is saved and the backup finalizes without an error. This lead to a situation where a snapshot references data that is contained in the repo, but not referenced in any index, leading to strange error messages. This commit uses a dedicated context to signal the intermediate index uploading routine to terminate after the last index has been uploaded. This way, an upload running when the backup finishes is completed before the routine terminates and the snapshot is saved. [1] https://forum.restic.net/t/error-loading-tree-check-prune-and-forget-gives-error-b2-backend/406 --- internal/archiver/archiver.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index 50a7a7488..9f2e029fb 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -620,7 +620,7 @@ func (j archiveJob) Copy() pipe.Job { const saveIndexTime = 30 * time.Second // saveIndexes regularly queries the master index for full indexes and saves them. -func (arch *Archiver) saveIndexes(ctx context.Context, wg *sync.WaitGroup) { +func (arch *Archiver) saveIndexes(saveCtx, shutdownCtx context.Context, wg *sync.WaitGroup) { defer wg.Done() ticker := time.NewTicker(saveIndexTime) @@ -628,11 +628,13 @@ func (arch *Archiver) saveIndexes(ctx context.Context, wg *sync.WaitGroup) { for { select { - case <-ctx.Done(): + case <-saveCtx.Done(): + return + case <-shutdownCtx.Done(): return case <-ticker.C: debug.Log("saving full indexes") - err := arch.repo.SaveFullIndex(ctx) + err := arch.repo.SaveFullIndex(saveCtx) if err != nil { debug.Log("save indexes returned an error: %v", err) fmt.Fprintf(os.Stderr, "error saving preliminary index: %v\n", err) @@ -748,16 +750,16 @@ func (arch *Archiver) Snapshot(ctx context.Context, p *restic.Progress, paths, t // run index saver var wgIndexSaver sync.WaitGroup - indexCtx, indexCancel := context.WithCancel(ctx) + shutdownCtx, indexShutdown := context.WithCancel(ctx) wgIndexSaver.Add(1) - go arch.saveIndexes(indexCtx, &wgIndexSaver) + go arch.saveIndexes(ctx, shutdownCtx, &wgIndexSaver) // wait for all workers to terminate debug.Log("wait for workers") wg.Wait() // stop index saver - indexCancel() + indexShutdown() wgIndexSaver.Wait() debug.Log("workers terminated") From 4219bfbcc9613732121d0edee0b13a51d7bc5798 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Fri, 26 Jan 2018 22:05:53 +0100 Subject: [PATCH 2/2] Add entry to changelog --- changelog/0.8.2/pull-1589 | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 changelog/0.8.2/pull-1589 diff --git a/changelog/0.8.2/pull-1589 b/changelog/0.8.2/pull-1589 new file mode 100644 index 000000000..33d013a5b --- /dev/null +++ b/changelog/0.8.2/pull-1589 @@ -0,0 +1,17 @@ +Bugfix: Complete intermediate index upload + +After a user posted a comprehensive report of what he observed, we were able to +find a bug and correct it: During backup, restic uploads so-called +"intermediate" index files. When the backup finishes during a transfer of such +an intermediate index, the upload is cancelled, but the backup is finished +without an error. This leads to an inconsistent state, where the snapshot +references data that is contained in the repo, but is not referenced in any +index. + +The situation can be resolved by building a new index with `rebuild-index`, but +looks very confusing at first. Since all the data got uploaded to the repo +successfully, there was no risk of data loss, just minor inconvenience for our +users. + +https://github.com/restic/restic/pull/1589 +https://forum.restic.net/t/error-loading-tree-check-prune-and-forget-gives-error-b2-backend/406