lib/backup: force copying of parts.json (#5006)

* lib/backup: force copying of parts.json

Copying of parts.json is required because `part.key()` comparison can create same key value for files with different contents. This will result in inconsistent backup being created or restored.

See: https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5005
Signed-off-by: Zakhar Bessarab <z.bessarab@victoriametrics.com>

* lib/backup: ensure parts.json is only copied once

Signed-off-by: Zakhar Bessarab <z.bessarab@victoriametrics.com>

---------

Signed-off-by: Zakhar Bessarab <z.bessarab@victoriametrics.com>
Co-authored-by: Nikolay <nik@victoriametrics.com>

docs/CHANGELOG.md

@@ -47,6 +47,7 @@ ssue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4825) and [these
 
 * BUGFIX: [Official Grafana dashboards for VictoriaMetrics](https://grafana.com/orgs/victoriametrics): fix display of ingested rows rate for `Samples ingested/s` and `Samples rate` panels for vmagent's dasbhoard. Previously, not all ingested protocols were accounted in these panels. An extra panel `Rows rate` was added to `Ingestion` section to display the split for rows ingested rate by protocol.
 * BUGFIX: [vmui](https://docs.victoriametrics.com/#vmui): fix the bug causing render looping when switching to heatmap.
+* BUGFIX: [vmbackup](https://docs.victoriametrics.com/vmbackup.html): force copying of `data/small/.../parts.json` in order to ensure backup consistency. Previously, `parts.json` could be skipped during copying, which could lead to data loss during restore. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5005).
 * BUGFIX: [VictoriaMetrics enterprise](https://docs.victoriametrics.com/enterprise.html) validate `-dedup.minScrapeInterval` value and `-downsampling.period` intervals are multiples of each other. See [these docs](https://docs.victoriametrics.com/#downsampling).
 
 ## [v1.93.4](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.93.4)

lib/backup/actions/backup.go

@@ -147,6 +147,7 @@ func runBackup(src *fslocal.FS, dst common.RemoteFS, origin common.OriginFS, con
 	}
 
 	srcCopyParts := common.PartsDifference(partsToCopy, originParts)
+	srcCopyParts = common.EnforceSpecialsCopy(srcParts, srcCopyParts)
 	uploadSize := getPartsSize(srcCopyParts)
 	if len(srcCopyParts) > 0 {
 		logger.Infof("uploading %d parts from %s to %s", len(srcCopyParts), src, dst)

lib/backup/actions/copy.go

@@ -84,6 +84,7 @@ func runCopy(src common.OriginFS, dst common.RemoteFS, concurrency int) error {
 	}
 
 	partsToCopy := common.PartsDifference(srcParts, dstParts)
+	partsToCopy = common.EnforceSpecialsCopy(srcParts, partsToCopy)
 	copySize := getPartsSize(partsToCopy)
 	if err := copySrcParts(src, dst, partsToCopy, concurrency); err != nil {
 		return fmt.Errorf("cannot server-side copy parts from src to dst: %w", err)

lib/backup/actions/restore.go

@@ -144,6 +144,7 @@ func (r *Restore) Run() error {
 	}
 
 	partsToCopy := common.PartsDifference(srcParts, dstParts)
+	partsToCopy = common.EnforceSpecialsCopy(srcParts, partsToCopy)
 	downloadSize := getPartsSize(partsToCopy)
 	if len(partsToCopy) > 0 {
 		perPath := make(map[string][]common.Part)

lib/backup/common/part.go

@@ -135,3 +135,31 @@ func PartsIntersect(a, b []Part) []Part {
 	}
 	return d
 }
+
+// EnforceSpecialsCopy enforces copying of special parts from src to toCopy without checking whether
+// part is already present in dst.
+func EnforceSpecialsCopy(src, toCopy []Part) []Part {
+	// `parts.json` files must be copied from src to dst without checking whether they already exist in dst.
+	// This is needed because size and paths for those files can be the same even if the contents differ.
+	// See: https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5005
+	filtered := make(map[Part]bool)
+	for _, pt := range src {
+		if strings.HasPrefix(pt.Path, "data") && strings.HasSuffix(pt.Path, "parts.json") {
+			filtered[pt] = false
+		}
+	}
+
+	for _, pt := range toCopy {
+		if _, ok := filtered[pt]; ok {
+			filtered[pt] = true
+		}
+	}
+
+	for pt, ok := range filtered {
+		if !ok {
+			toCopy = append(toCopy, pt)
+		}
+	}
+
+	return toCopy
+}