From 5c39b19acd7d2515a617f03af864ba40a81e5967 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Fri, 24 Mar 2023 22:03:11 -0700 Subject: [PATCH] app/vmbackup: simplify code a bit after 5ba347bd2cd7f84afa07ac424630c2c23262b500 Unconditionally call deleteSnapshot() func just after making the snapshot, either successful or unsuccessful Related issue: https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2055 --- app/vmbackup/main.go | 48 ++++++++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/app/vmbackup/main.go b/app/vmbackup/main.go index edb93e4e1..c4facc8e6 100644 --- a/app/vmbackup/main.go +++ b/app/vmbackup/main.go @@ -52,7 +52,8 @@ func main() { // Storing snapshot delete function to be able to call it in case // of error since logger.Fatal will exit the program without // calling deferred functions. - var deleteSnapshot func() + // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2055 + deleteSnapshot := func() {} if len(*snapshotCreateURL) > 0 { // create net/url object @@ -91,31 +92,42 @@ func main() { logger.Fatalf("cannot delete snapshot: %s", err) } } - - defer deleteSnapshot() } else if len(*snapshotName) == 0 { logger.Fatalf("`-snapshotName` or `-snapshot.createURL` must be provided") } - if err := snapshot.Validate(*snapshotName); err != nil { - logger.Fatalf("invalid -snapshotName=%q: %s", *snapshotName, err) - } go httpserver.Serve(*httpListenAddr, false, nil) + err := makeBackup() + deleteSnapshot() + if err != nil { + logger.Fatalf("cannot create backup: %s", err) + } + + startTime := time.Now() + logger.Infof("gracefully shutting down http server for metrics at %q", *httpListenAddr) + if err := httpserver.Stop(*httpListenAddr); err != nil { + logger.Fatalf("cannot stop http server for metrics: %s", err) + } + logger.Infof("successfully shut down http server for metrics in %.3f seconds", time.Since(startTime).Seconds()) +} + +func makeBackup() error { + if err := snapshot.Validate(*snapshotName); err != nil { + return fmt.Errorf("invalid -snapshotName=%q: %s", *snapshotName, err) + } + srcFS, err := newSrcFS() if err != nil { - deleteSnapshot() - logger.Fatalf("%s", err) + return err } dstFS, err := newDstFS() if err != nil { - deleteSnapshot() - logger.Fatalf("%s", err) + return err } originFS, err := newOriginFS() if err != nil { - deleteSnapshot() - logger.Fatalf("%s", err) + return err } a := &actions.Backup{ Concurrency: *concurrency, @@ -124,20 +136,12 @@ func main() { Origin: originFS, } if err := a.Run(); err != nil { - deleteSnapshot() - logger.Fatalf("cannot create backup: %s", err) + return err } srcFS.MustStop() dstFS.MustStop() originFS.MustStop() - - startTime := time.Now() - logger.Infof("gracefully shutting down http server for metrics at %q", *httpListenAddr) - if err := httpserver.Stop(*httpListenAddr); err != nil { - deleteSnapshot() - logger.Fatalf("cannot stop http server for metrics: %s", err) - } - logger.Infof("successfully shut down http server for metrics in %.3f seconds", time.Since(startTime).Seconds()) + return nil } func usage() {