From 39e0007e1467287aacebd5c5d9af1ada8ab3e220 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Fri, 9 Feb 2024 04:03:20 +0200 Subject: [PATCH] lib/snapshot: move Time, Validate and NewName into lib/snapshot/snapshotutil package This allows removing importing unneeded command-line flags into binaries, which import lib/storage, which, in turn, was importing lib/snapshot in order to use Time, Validate and NewName functions. This is a follow-up for 83e55456e212267e8c55151a541790bd4f820187 Updates https://github.com/VictoriaMetrics/VictoriaMetrics/pull/5738 --- app/vmbackup/main.go | 3 +- lib/backup/actions/backup.go | 4 +- lib/snapshot/snapshot.go | 40 ------------- lib/snapshot/snapshot_test.go | 51 ----------------- lib/snapshot/snapshotutil/snapshotutil.go | 47 ++++++++++++++++ .../snapshotutil/snapshotutil_test.go | 56 +++++++++++++++++++ lib/storage/storage.go | 10 ++-- 7 files changed, 112 insertions(+), 99 deletions(-) create mode 100644 lib/snapshot/snapshotutil/snapshotutil.go create mode 100644 lib/snapshot/snapshotutil/snapshotutil_test.go diff --git a/app/vmbackup/main.go b/app/vmbackup/main.go index e6c92d0e9..a6c6089e5 100644 --- a/app/vmbackup/main.go +++ b/app/vmbackup/main.go @@ -20,6 +20,7 @@ import ( "github.com/VictoriaMetrics/VictoriaMetrics/lib/logger" "github.com/VictoriaMetrics/VictoriaMetrics/lib/pushmetrics" "github.com/VictoriaMetrics/VictoriaMetrics/lib/snapshot" + "github.com/VictoriaMetrics/VictoriaMetrics/lib/snapshot/snapshotutil" ) var ( @@ -169,7 +170,7 @@ See the docs at https://docs.victoriametrics.com/vmbackup.html . } func newSrcFS() (*fslocal.FS, error) { - if err := snapshot.Validate(*snapshotName); err != nil { + if err := snapshotutil.Validate(*snapshotName); err != nil { return nil, fmt.Errorf("invalid -snapshotName=%q: %w", *snapshotName, err) } snapshotPath := filepath.Join(*storageDataPath, "snapshots", *snapshotName) diff --git a/lib/backup/actions/backup.go b/lib/backup/actions/backup.go index e870fd899..b06d873e9 100644 --- a/lib/backup/actions/backup.go +++ b/lib/backup/actions/backup.go @@ -13,7 +13,7 @@ import ( "github.com/VictoriaMetrics/VictoriaMetrics/lib/backup/fslocal" "github.com/VictoriaMetrics/VictoriaMetrics/lib/backup/fsnil" "github.com/VictoriaMetrics/VictoriaMetrics/lib/logger" - "github.com/VictoriaMetrics/VictoriaMetrics/lib/snapshot" + "github.com/VictoriaMetrics/VictoriaMetrics/lib/snapshot/snapshotutil" "github.com/VictoriaMetrics/metrics" ) @@ -87,7 +87,7 @@ func (b *Backup) Run() error { func storeMetadata(src *fslocal.FS, dst common.RemoteFS) error { snapshotName := filepath.Base(src.Dir) - snapshotTime, err := snapshot.Time(snapshotName) + snapshotTime, err := snapshotutil.Time(snapshotName) if err != nil { return fmt.Errorf("cannot decode snapshot name %q: %w", snapshotName, err) } diff --git a/lib/snapshot/snapshot.go b/lib/snapshot/snapshot.go index deb79777f..87bf8d09f 100644 --- a/lib/snapshot/snapshot.go +++ b/lib/snapshot/snapshot.go @@ -8,17 +8,11 @@ import ( "io" "net/http" "net/url" - "regexp" - "strings" - "sync/atomic" - "time" "github.com/VictoriaMetrics/VictoriaMetrics/lib/httputils" "github.com/VictoriaMetrics/VictoriaMetrics/lib/logger" ) -var snapshotNameRegexp = regexp.MustCompile(`^[0-9]{14}-[0-9A-Fa-f]+$`) - var ( tlsInsecureSkipVerify = flag.Bool("snapshot.tlsInsecureSkipVerify", false, "Whether to skip tls verification when connecting to -snapshotCreateURL") tlsCertFile = flag.String("snapshot.tlsCertFile", "", "Optional path to client-side TLS certificate file to use when connecting to -snapshotCreateURL") @@ -119,37 +113,3 @@ func Delete(deleteSnapshotURL string, snapshotName string) error { } return fmt.Errorf("Unkown status: %v", snap.Status) } - -// Validate validates the snapshotName -func Validate(snapshotName string) error { - _, err := Time(snapshotName) - return err -} - -// Time returns snapshot creation time from the given snapshotName -func Time(snapshotName string) (time.Time, error) { - if !snapshotNameRegexp.MatchString(snapshotName) { - return time.Time{}, fmt.Errorf("unexpected snapshot name=%q; it must match %q regexp", snapshotName, snapshotNameRegexp.String()) - } - n := strings.IndexByte(snapshotName, '-') - if n < 0 { - logger.Panicf("BUG: cannot find `-` in snapshotName=%q", snapshotName) - } - s := snapshotName[:n] - t, err := time.Parse("20060102150405", s) - if err != nil { - return time.Time{}, fmt.Errorf("unexpected timestamp=%q in snapshot name: %w; it must match YYYYMMDDhhmmss pattern", s, err) - } - return t, nil -} - -// NewName returns new name for new snapshot -func NewName() string { - return fmt.Sprintf("%s-%08X", time.Now().UTC().Format("20060102150405"), nextSnapshotIdx()) -} - -func nextSnapshotIdx() uint64 { - return atomic.AddUint64(&snapshotIdx, 1) -} - -var snapshotIdx = uint64(time.Now().UnixNano()) diff --git a/lib/snapshot/snapshot_test.go b/lib/snapshot/snapshot_test.go index 42712e0f1..6d4266de6 100644 --- a/lib/snapshot/snapshot_test.go +++ b/lib/snapshot/snapshot_test.go @@ -104,54 +104,3 @@ func TestDeleteSnapshotFailed(t *testing.T) { t.Fatalf("Snapshot should have failed, got: %v", err) } } - -func Test_Validate(t *testing.T) { - tests := []struct { - name string - snapshotName string - want bool - }{ - { - name: "empty snapshot name", - snapshotName: "", - want: false, - }, - { - name: "short snapshot name", - snapshotName: "", - want: false, - }, - { - name: "short first part of the snapshot name", - snapshotName: "2022050312163-16EB56ADB4110CF2", - want: false, - }, - { - name: "short second part of the snapshot name", - snapshotName: "20220503121638-16EB56ADB4110CF", - want: true, - }, - { - name: "correct snapshot name", - snapshotName: "20220503121638-16EB56ADB4110CF2", - want: true, - }, - { - name: "invalid time part snapshot name", - snapshotName: "00000000000000-16EB56ADB4110CF2", - want: false, - }, - { - name: "not enough parts of the snapshot name", - snapshotName: "2022050312163816EB56ADB4110CF2", - want: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if err := Validate(tt.snapshotName); (err == nil) != tt.want { - t.Errorf("checkSnapshotName() = %v, want %v", err, tt.want) - } - }) - } -} diff --git a/lib/snapshot/snapshotutil/snapshotutil.go b/lib/snapshot/snapshotutil/snapshotutil.go new file mode 100644 index 000000000..9c55fe4ac --- /dev/null +++ b/lib/snapshot/snapshotutil/snapshotutil.go @@ -0,0 +1,47 @@ +package snapshotutil + +import ( + "fmt" + "regexp" + "strings" + "sync/atomic" + "time" + + "github.com/VictoriaMetrics/VictoriaMetrics/lib/logger" +) + +var snapshotNameRegexp = regexp.MustCompile(`^[0-9]{14}-[0-9A-Fa-f]+$`) + +// Validate validates the snapshotName +func Validate(snapshotName string) error { + _, err := Time(snapshotName) + return err +} + +// Time returns snapshot creation time from the given snapshotName +func Time(snapshotName string) (time.Time, error) { + if !snapshotNameRegexp.MatchString(snapshotName) { + return time.Time{}, fmt.Errorf("unexpected snapshot name=%q; it must match %q regexp", snapshotName, snapshotNameRegexp.String()) + } + n := strings.IndexByte(snapshotName, '-') + if n < 0 { + logger.Panicf("BUG: cannot find `-` in snapshotName=%q", snapshotName) + } + s := snapshotName[:n] + t, err := time.Parse("20060102150405", s) + if err != nil { + return time.Time{}, fmt.Errorf("unexpected timestamp=%q in snapshot name: %w; it must match YYYYMMDDhhmmss pattern", s, err) + } + return t, nil +} + +// NewName returns new name for new snapshot +func NewName() string { + return fmt.Sprintf("%s-%08X", time.Now().UTC().Format("20060102150405"), nextSnapshotIdx()) +} + +func nextSnapshotIdx() uint64 { + return atomic.AddUint64(&snapshotIdx, 1) +} + +var snapshotIdx = uint64(time.Now().UnixNano()) diff --git a/lib/snapshot/snapshotutil/snapshotutil_test.go b/lib/snapshot/snapshotutil/snapshotutil_test.go new file mode 100644 index 000000000..5c2a3d5a0 --- /dev/null +++ b/lib/snapshot/snapshotutil/snapshotutil_test.go @@ -0,0 +1,56 @@ +package snapshotutil + +import ( + "testing" +) + +func Test_Validate(t *testing.T) { + tests := []struct { + name string + snapshotName string + want bool + }{ + { + name: "empty snapshot name", + snapshotName: "", + want: false, + }, + { + name: "short snapshot name", + snapshotName: "", + want: false, + }, + { + name: "short first part of the snapshot name", + snapshotName: "2022050312163-16EB56ADB4110CF2", + want: false, + }, + { + name: "short second part of the snapshot name", + snapshotName: "20220503121638-16EB56ADB4110CF", + want: true, + }, + { + name: "correct snapshot name", + snapshotName: "20220503121638-16EB56ADB4110CF2", + want: true, + }, + { + name: "invalid time part snapshot name", + snapshotName: "00000000000000-16EB56ADB4110CF2", + want: false, + }, + { + name: "not enough parts of the snapshot name", + snapshotName: "2022050312163816EB56ADB4110CF2", + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := Validate(tt.snapshotName); (err == nil) != tt.want { + t.Errorf("checkSnapshotName() = %v, want %v", err, tt.want) + } + }) + } +} diff --git a/lib/storage/storage.go b/lib/storage/storage.go index ec3a7ce3a..cc7f87734 100644 --- a/lib/storage/storage.go +++ b/lib/storage/storage.go @@ -25,7 +25,7 @@ import ( "github.com/VictoriaMetrics/VictoriaMetrics/lib/logger" "github.com/VictoriaMetrics/VictoriaMetrics/lib/memory" "github.com/VictoriaMetrics/VictoriaMetrics/lib/querytracer" - "github.com/VictoriaMetrics/VictoriaMetrics/lib/snapshot" + "github.com/VictoriaMetrics/VictoriaMetrics/lib/snapshot/snapshotutil" "github.com/VictoriaMetrics/VictoriaMetrics/lib/timeutil" "github.com/VictoriaMetrics/VictoriaMetrics/lib/uint64set" "github.com/VictoriaMetrics/VictoriaMetrics/lib/workingsetcache" @@ -342,7 +342,7 @@ func (s *Storage) CreateSnapshot(deadline uint64) (string, error) { } }() - snapshotName := snapshot.NewName() + snapshotName := snapshotutil.NewName() srcDir := s.path dstDir := filepath.Join(srcDir, snapshotsDirname, snapshotName) fs.MustMkdirFailIfExist(dstDir) @@ -409,7 +409,7 @@ func (s *Storage) ListSnapshots() ([]string, error) { } snapshotNames := make([]string, 0, len(fnames)) for _, fname := range fnames { - if err := snapshot.Validate(fname); err != nil { + if err := snapshotutil.Validate(fname); err != nil { continue } snapshotNames = append(snapshotNames, fname) @@ -420,7 +420,7 @@ func (s *Storage) ListSnapshots() ([]string, error) { // DeleteSnapshot deletes the given snapshot. func (s *Storage) DeleteSnapshot(snapshotName string) error { - if err := snapshot.Validate(snapshotName); err != nil { + if err := snapshotutil.Validate(snapshotName); err != nil { return fmt.Errorf("invalid snapshotName %q: %w", snapshotName, err) } snapshotPath := filepath.Join(s.path, snapshotsDirname, snapshotName) @@ -446,7 +446,7 @@ func (s *Storage) DeleteStaleSnapshots(maxAge time.Duration) error { } expireDeadline := time.Now().UTC().Add(-maxAge) for _, snapshotName := range list { - t, err := snapshot.Time(snapshotName) + t, err := snapshotutil.Time(snapshotName) if err != nil { return fmt.Errorf("cannot parse snapshot date from %q: %w", snapshotName, err) }