From 9bad52b68737130b46710c6b860935820e28e8a9 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Fri, 23 Feb 2024 04:46:11 +0200 Subject: [PATCH] app/vmstorage: deprecate -snapshotCreateTimeout command-line flag Creating snapshot shouldn't time out under normal conditions. The timeout was related to the bug, which has been fixed in https://github.com/VictoriaMetrics/VictoriaMetrics/commit/6460475e3bb488086046cb5fdce3915206d731ef . Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3551 --- app/vmstorage/main.go | 8 ++------ lib/mergeset/table.go | 8 +------- lib/mergeset/table_test.go | 4 ++-- lib/storage/storage.go | 12 +++++------- lib/storage/storage_test.go | 4 ++-- lib/storage/table.go | 14 +++----------- 6 files changed, 15 insertions(+), 35 deletions(-) diff --git a/app/vmstorage/main.go b/app/vmstorage/main.go index 5af4768d8d..5b273d215f 100644 --- a/app/vmstorage/main.go +++ b/app/vmstorage/main.go @@ -31,7 +31,7 @@ var ( forceMergeAuthKey = flagutil.NewPassword("forceMergeAuthKey", "authKey, which must be passed in query string to /internal/force_merge pages") forceFlushAuthKey = flagutil.NewPassword("forceFlushAuthKey", "authKey, which must be passed in query string to /internal/force_flush pages") snapshotsMaxAge = flagutil.NewDuration("snapshotsMaxAge", "0", "Automatically delete snapshots older than -snapshotsMaxAge if it is set to non-zero duration. Make sure that backup process has enough time to finish the backup before the corresponding snapshot is automatically deleted") - snapshotCreateTimeout = flag.Duration("snapshotCreateTimeout", 0, "The timeout for creating new snapshot. If set, make sure that timeout is lower than backup period") + snapshotCreateTimeout = flag.Duration("snapshotCreateTimeout", 0, "Deprecated: this flag does nothing") precisionBits = flag.Int("precisionBits", 64, "The number of precision bits to store per each value. Lower precision bits improves data compression at the cost of precision loss") @@ -299,11 +299,7 @@ func RequestHandler(w http.ResponseWriter, r *http.Request) bool { case "/create": snapshotsCreateTotal.Inc() w.Header().Set("Content-Type", "application/json") - deadline := uint64(0) - if *snapshotCreateTimeout > 0 { - deadline = fasttime.UnixTimestamp() + uint64(snapshotCreateTimeout.Seconds()) - } - snapshotPath, err := Storage.CreateSnapshot(deadline) + snapshotPath, err := Storage.CreateSnapshot() if err != nil { err = fmt.Errorf("cannot create snapshot: %w", err) jsonResponseError(w, err) diff --git a/lib/mergeset/table.go b/lib/mergeset/table.go index b76a244e4b..898c5175a2 100644 --- a/lib/mergeset/table.go +++ b/lib/mergeset/table.go @@ -15,7 +15,6 @@ import ( "unsafe" "github.com/VictoriaMetrics/VictoriaMetrics/lib/cgroup" - "github.com/VictoriaMetrics/VictoriaMetrics/lib/fasttime" "github.com/VictoriaMetrics/VictoriaMetrics/lib/fs" "github.com/VictoriaMetrics/VictoriaMetrics/lib/logger" "github.com/VictoriaMetrics/VictoriaMetrics/lib/memory" @@ -1536,10 +1535,8 @@ func mustOpenParts(path string) []*partWrapper { // // Snapshot is created using linux hard links, so it is usually created very quickly. // -// If deadline is reached before snapshot is created error is returned. -// // The caller is responsible for data removal at dstDir on unsuccessful snapshot creation. -func (tb *Table) CreateSnapshotAt(dstDir string, deadline uint64) error { +func (tb *Table) CreateSnapshotAt(dstDir string) error { logger.Infof("creating Table snapshot of %q...", tb.path) startTime := time.Now() @@ -1574,9 +1571,6 @@ func (tb *Table) CreateSnapshotAt(dstDir string, deadline uint64) error { // Skip in-memory parts continue } - if deadline > 0 && fasttime.UnixTimestamp() > deadline { - return fmt.Errorf("cannot create snapshot for %q: timeout exceeded", tb.path) - } srcPartPath := pw.p.path dstPartPath := filepath.Join(dstDir, filepath.Base(srcPartPath)) fs.MustHardLinkFiles(srcPartPath, dstPartPath) diff --git a/lib/mergeset/table_test.go b/lib/mergeset/table_test.go index a71c39a51d..7b7376681c 100644 --- a/lib/mergeset/table_test.go +++ b/lib/mergeset/table_test.go @@ -126,11 +126,11 @@ func TestTableCreateSnapshotAt(t *testing.T) { // Create multiple snapshots. snapshot1 := path + "-test-snapshot1" - if err := tb.CreateSnapshotAt(snapshot1, 0); err != nil { + if err := tb.CreateSnapshotAt(snapshot1); err != nil { t.Fatalf("cannot create snapshot1: %s", err) } snapshot2 := path + "-test-snapshot2" - if err := tb.CreateSnapshotAt(snapshot2, 0); err != nil { + if err := tb.CreateSnapshotAt(snapshot2); err != nil { t.Fatalf("cannot create snapshot2: %s", err) } diff --git a/lib/storage/storage.go b/lib/storage/storage.go index 808a089452..16c9ffa286 100644 --- a/lib/storage/storage.go +++ b/lib/storage/storage.go @@ -328,7 +328,7 @@ func (s *Storage) DebugFlush() { } // CreateSnapshot creates snapshot for s and returns the snapshot name. -func (s *Storage) CreateSnapshot(deadline uint64) (string, error) { +func (s *Storage) CreateSnapshot() (string, error) { logger.Infof("creating Storage snapshot for %q...", s.path) startTime := time.Now() @@ -348,10 +348,7 @@ func (s *Storage) CreateSnapshot(deadline uint64) (string, error) { fs.MustMkdirFailIfExist(dstDir) dirsToRemoveOnError = append(dirsToRemoveOnError, dstDir) - smallDir, bigDir, err := s.tb.CreateSnapshot(snapshotName, deadline) - if err != nil { - return "", fmt.Errorf("cannot create table snapshot: %w", err) - } + smallDir, bigDir := s.tb.MustCreateSnapshot(snapshotName) dirsToRemoveOnError = append(dirsToRemoveOnError, smallDir, bigDir) dstDataDir := filepath.Join(dstDir, dataDirname) @@ -372,14 +369,15 @@ func (s *Storage) CreateSnapshot(deadline uint64) (string, error) { idbSnapshot := filepath.Join(srcDir, indexdbDirname, snapshotsDirname, snapshotName) idb := s.idb() currSnapshot := filepath.Join(idbSnapshot, idb.name) - if err := idb.tb.CreateSnapshotAt(currSnapshot, deadline); err != nil { + if err := idb.tb.CreateSnapshotAt(currSnapshot); err != nil { return "", fmt.Errorf("cannot create curr indexDB snapshot: %w", err) } dirsToRemoveOnError = append(dirsToRemoveOnError, idbSnapshot) + var err error ok := idb.doExtDB(func(extDB *indexDB) { prevSnapshot := filepath.Join(idbSnapshot, extDB.name) - err = extDB.tb.CreateSnapshotAt(prevSnapshot, deadline) + err = extDB.tb.CreateSnapshotAt(prevSnapshot) }) if ok && err != nil { return "", fmt.Errorf("cannot create prev indexDB snapshot: %w", err) diff --git a/lib/storage/storage_test.go b/lib/storage/storage_test.go index 09fe564860..6763ba9d4f 100644 --- a/lib/storage/storage_test.go +++ b/lib/storage/storage_test.go @@ -1020,7 +1020,7 @@ func testStorageAddRows(rng *rand.Rand, s *Storage) error { } // Try creating a snapshot from the storage. - snapshotName, err := s.CreateSnapshot(0) + snapshotName, err := s.CreateSnapshot() if err != nil { return fmt.Errorf("cannot create snapshot from the storage: %w", err) } @@ -1178,7 +1178,7 @@ func TestStorageDeleteStaleSnapshots(t *testing.T) { } } // Try creating a snapshot from the storage. - snapshotName, err := s.CreateSnapshot(0) + snapshotName, err := s.CreateSnapshot() if err != nil { t.Fatalf("cannot create snapshot from the storage: %s", err) } diff --git a/lib/storage/table.go b/lib/storage/table.go index 6ead99d61c..e76d65f38c 100644 --- a/lib/storage/table.go +++ b/lib/storage/table.go @@ -119,10 +119,8 @@ func mustOpenTable(path string, s *Storage) *table { return tb } -// CreateSnapshot creates tb snapshot and returns paths to small and big parts of it. -// If deadline is reached before snapshot is created error is returned. -// If any error occurs during snapshot created data is not removed. -func (tb *table) CreateSnapshot(snapshotName string, deadline uint64) (string, string, error) { +// MustCreateSnapshot creates tb snapshot and returns paths to small and big parts of it. +func (tb *table) MustCreateSnapshot(snapshotName string) (string, string) { logger.Infof("creating table snapshot of %q...", tb.path) startTime := time.Now() @@ -136,12 +134,6 @@ func (tb *table) CreateSnapshot(snapshotName string, deadline uint64) (string, s fs.MustMkdirFailIfExist(dstBigDir) for _, ptw := range ptws { - if deadline > 0 && fasttime.UnixTimestamp() > deadline { - fs.MustRemoveAll(dstSmallDir) - fs.MustRemoveAll(dstBigDir) - return "", "", fmt.Errorf("cannot create snapshot for %q: timeout exceeded", tb.path) - } - smallPath := filepath.Join(dstSmallDir, ptw.pt.name) bigPath := filepath.Join(dstBigDir, ptw.pt.name) ptw.pt.MustCreateSnapshotAt(smallPath, bigPath) @@ -153,7 +145,7 @@ func (tb *table) CreateSnapshot(snapshotName string, deadline uint64) (string, s fs.MustSyncPath(filepath.Dir(dstBigDir)) logger.Infof("created table snapshot for %q at (%q, %q) in %.3f seconds", tb.path, dstSmallDir, dstBigDir, time.Since(startTime).Seconds()) - return dstSmallDir, dstBigDir, nil + return dstSmallDir, dstBigDir } // MustDeleteSnapshot deletes snapshot with the given snapshotName.