From b2e8044fbdb854fa58a9a8c8881b40b66b21a890 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 8 May 2023 21:40:23 +0200 Subject: [PATCH 1/3] Fix inverted condition for fsync warning The warning should only be printed if fsync is _not_ supported and not the other way around. --- repo/repo.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/repo/repo.go b/repo/repo.go index 3162980..0cae450 100644 --- a/repo/repo.go +++ b/repo/repo.go @@ -638,15 +638,16 @@ func (h *Handler) saveBlob(w http.ResponseWriter, r *http.Request) { return } - if !syncNotSup { + if syncNotSup { + h.fsyncWarning.Do(func() { + log.Print("WARNING: fsync is not supported by the data storage. This can lead to data loss, if the system crashes or the storage is unexpectedly disconnected.") + }) + } else { if err := syncDir(filepath.Dir(path)); err != nil { // Don't call os.Remove(path) as this is prone to race conditions with parallel upload retries h.internalServerError(w, err) return } - h.fsyncWarning.Do(func() { - log.Print("WARNING: fsync is not supported by the data storage. This can lead to data loss, if the system crashes or the storage is unexpectedly disconnected.") - }) } h.sendMetric(objectType, BlobWrite, uint64(written)) From be14687a9c862065ffcbd88eae250d9adde0a1fd Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 13 May 2023 21:50:39 +0200 Subject: [PATCH 2/3] Print fsync warning only once The repo.Handler is freshly instantiated for every request such that it forget that the fsync warning was already printed. Use a single instance in the Server instead. --- handlers.go | 3 +++ repo/repo.go | 5 ++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/handlers.go b/handlers.go index f1f3954..8163ccf 100644 --- a/handlers.go +++ b/handlers.go @@ -7,6 +7,7 @@ import ( "path" "path/filepath" "strings" + "sync" "github.com/restic/rest-server/quota" "github.com/restic/rest-server/repo" @@ -34,6 +35,7 @@ type Server struct { htpasswdFile *HtpasswdFile quotaManager *quota.Manager + fsyncWarning sync.Once } // MaxFolderDepth is the maxDepth param passed to splitURLPath. @@ -91,6 +93,7 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { QuotaManager: s.quotaManager, // may be nil PanicOnError: s.PanicOnError, NoVerifyUpload: s.NoVerifyUpload, + FsyncWarning: &s.fsyncWarning, } if s.Prometheus { opt.BlobMetricFunc = makeBlobMetricFunc(username, folderPath) diff --git a/repo/repo.go b/repo/repo.go index 0cae450..a1ad642 100644 --- a/repo/repo.go +++ b/repo/repo.go @@ -39,6 +39,7 @@ type Options struct { BlobMetricFunc BlobMetricFunc QuotaManager *quota.Manager + FsyncWarning *sync.Once } // DefaultDirMode is the file mode used for directory creation if not @@ -74,8 +75,6 @@ func New(path string, opt Options) (*Handler, error) { type Handler struct { path string // filesystem path of repo opt Options - - fsyncWarning sync.Once } // httpDefaultError write a HTTP error with the default description @@ -639,7 +638,7 @@ func (h *Handler) saveBlob(w http.ResponseWriter, r *http.Request) { } if syncNotSup { - h.fsyncWarning.Do(func() { + h.opt.FsyncWarning.Do(func() { log.Print("WARNING: fsync is not supported by the data storage. This can lead to data loss, if the system crashes or the storage is unexpectedly disconnected.") }) } else { From 420c4c6683ea1e2dfaddf2d17fa9d37098452fdc Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 13 May 2023 21:59:48 +0200 Subject: [PATCH 3/3] Add changelog for fsync warning --- changelog/unreleased/issue-230 | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 changelog/unreleased/issue-230 diff --git a/changelog/unreleased/issue-230 b/changelog/unreleased/issue-230 new file mode 100644 index 0000000..6f9704d --- /dev/null +++ b/changelog/unreleased/issue-230 @@ -0,0 +1,9 @@ +Bugfix: Fix erroneous warnings about unsupported fsync + +Due to a regression in rest-server 0.12.0, it continuously printed +`WARNING: fsync is not supported by the data storage. This can lead to data loss, +if the system crashes or the storage is unexpectedly disconnected.` for systems +that support fsync. We have fixed the warning. + +https://github.com/restic/rest-server/issues/230 +https://github.com/restic/rest-server/pull/231