From 2a3b19e1d21ffee9b74fbb92fa5e435f94d433ef Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Fri, 14 Apr 2023 19:49:54 -0700 Subject: [PATCH] lib/fs: convert CreateFlockFile to MustCreateFlockFile Callers of CreateFlockFile log the returned err and exit. It is better to log the error inside the MustCreateFlockFile together with the path to the specified directory and the call stack. This simplifies the code at the callers' side while leaving the debuggability at the same level. --- lib/backup/actions/restore.go | 5 +---- lib/fs/fs.go | 14 +++++++++----- lib/mergeset/table.go | 6 +----- lib/persistentqueue/persistentqueue.go | 10 +--------- lib/storage/storage.go | 7 +------ lib/storage/table.go | 6 +----- 6 files changed, 14 insertions(+), 34 deletions(-) diff --git a/lib/backup/actions/restore.go b/lib/backup/actions/restore.go index 155b2d8ea..2d81bf3d2 100644 --- a/lib/backup/actions/restore.go +++ b/lib/backup/actions/restore.go @@ -46,10 +46,7 @@ func (r *Restore) Run() error { // Make sure VictoriaMetrics doesn't run during the restore process. fs.MustMkdirIfNotExist(r.Dst.Dir) - flockF, err := fs.CreateFlockFile(r.Dst.Dir) - if err != nil { - return fmt.Errorf("cannot create lock file in %q; make sure VictoriaMetrics doesn't use the dir; error: %w", r.Dst.Dir, err) - } + flockF := fs.MustCreateFlockFile(r.Dst.Dir) defer fs.MustClose(flockF) if err := createRestoreLock(r.Dst.Dir); err != nil { diff --git a/lib/fs/fs.go b/lib/fs/fs.go index f8d585b7c..b4b7aacab 100644 --- a/lib/fs/fs.go +++ b/lib/fs/fs.go @@ -362,14 +362,18 @@ func MustWriteData(w filestream.WriteCloser, data []byte) { } } -// CreateFlockFile creates FlockFilename file in the directory dir +// MustCreateFlockFile creates FlockFilename file in the directory dir // and returns the handler to the file. -func CreateFlockFile(dir string) (*os.File, error) { - flockFile := filepath.Join(dir, FlockFilename) - return createFlockFile(flockFile) +func MustCreateFlockFile(dir string) *os.File { + flockFilepath := filepath.Join(dir, FlockFilename) + f, err := createFlockFile(flockFilepath) + if err != nil { + logger.Panicf("FATAL: cannot create lock file: %s; make sure a single process has exclusive access to %q", err, dir) + } + return f } -// FlockFilename is the filename for the file created by CreateFlockFile(). +// FlockFilename is the filename for the file created by MustCreateFlockFile(). const FlockFilename = "flock.lock" // MustGetFreeSpace returns free space for the given directory path. diff --git a/lib/mergeset/table.go b/lib/mergeset/table.go index 96153d9a8..81a1da24e 100644 --- a/lib/mergeset/table.go +++ b/lib/mergeset/table.go @@ -331,11 +331,7 @@ func OpenTable(path string, flushCallback func(), prepareBlock PrepareBlockCallb fs.MustMkdirIfNotExist(path) // Protect from concurrent opens. - flockF, err := fs.CreateFlockFile(path) - if err != nil { - return nil, fmt.Errorf("cannot create lock file in %q; "+ - "make sure the dir isn't used by other processes or manually delete the file if you recover from abrupt VictoriaMetrics crash; error: %w", path, err) - } + flockF := fs.MustCreateFlockFile(path) // Open table parts. pws, err := openParts(path) diff --git a/lib/persistentqueue/persistentqueue.go b/lib/persistentqueue/persistentqueue.go index ffedb9435..27874dde8 100644 --- a/lib/persistentqueue/persistentqueue.go +++ b/lib/persistentqueue/persistentqueue.go @@ -144,14 +144,6 @@ func mustOpenInternal(path, name string, chunkFileSize, maxBlockSize, maxPending return q } -func mustCreateFlockFile(path string) *os.File { - f, err := fs.CreateFlockFile(path) - if err != nil { - logger.Panicf("FATAL: %s", err) - } - return f -} - func tryOpeningQueue(path, name string, chunkFileSize, maxBlockSize, maxPendingBytes uint64) (*queue, error) { // Protect from concurrent opens. var q queue @@ -178,7 +170,7 @@ func tryOpeningQueue(path, name string, chunkFileSize, maxBlockSize, maxPendingB } fs.MustMkdirIfNotExist(path) - q.flockF = mustCreateFlockFile(path) + q.flockF = fs.MustCreateFlockFile(path) mustCloseFlockF := true defer func() { if mustCloseFlockF { diff --git a/lib/storage/storage.go b/lib/storage/storage.go index 98f4025ce..e1a4e415f 100644 --- a/lib/storage/storage.go +++ b/lib/storage/storage.go @@ -167,12 +167,7 @@ func OpenStorage(path string, retentionMsecs int64, maxHourlySeries, maxDailySer } // Protect from concurrent opens. - flockF, err := fs.CreateFlockFile(path) - if err != nil { - return nil, fmt.Errorf("cannot create lock file in %q; "+ - "make sure the dir isn't used by other processes or manually delete the file if you recover from abrupt VictoriaMetrics crash; error: %w", path, err) - } - s.flockF = flockF + s.flockF = fs.MustCreateFlockFile(path) // Check whether restore process finished successfully restoreLockF := filepath.Join(path, backupnames.RestoreInProgressFilename) diff --git a/lib/storage/table.go b/lib/storage/table.go index 37799347f..cd78ae68d 100644 --- a/lib/storage/table.go +++ b/lib/storage/table.go @@ -86,11 +86,7 @@ func openTable(path string, s *Storage) (*table, error) { fs.MustMkdirIfNotExist(path) // Protect from concurrent opens. - flockF, err := fs.CreateFlockFile(path) - if err != nil { - return nil, fmt.Errorf("cannot create lock file in %q; "+ - "make sure the dir isn't used by other processes or manually delete the file if you recover from abrupt VictoriaMetrics crash; error: %w", path, err) - } + flockF := fs.MustCreateFlockFile(path) // Create directories for small and big partitions if they don't exist yet. smallPartitionsPath := filepath.Join(path, smallDirname)