From 4e50d6eed360bba101a9592a5b60cc577985c94c Mon Sep 17 00:00:00 2001 From: Zakhar Bessarab Date: Sun, 27 Oct 2024 16:40:13 -0300 Subject: [PATCH] lib/storage/partition: prevent panic in case resulting in-memory part is empty after merge (#7329) It is possible for in-memory part to be empty if ingested samples are removed by retention filters. In this case, data will not be discarded due to retention before creating in memory part. After in-memory parts merge samples will be removed resulting in creating completely empty part at destination. This commit checks for resulting part and skips it, if it's empty. --------- Signed-off-by: Zakhar Bessarab --- docs/changelog/CHANGELOG.md | 1 + lib/storage/partition.go | 8 ++++++++ lib/storage/partition_test.go | 36 +++++++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+) diff --git a/docs/changelog/CHANGELOG.md b/docs/changelog/CHANGELOG.md index 8b5b569bf..374a88eca 100644 --- a/docs/changelog/CHANGELOG.md +++ b/docs/changelog/CHANGELOG.md @@ -24,6 +24,7 @@ See also [LTS releases](https://docs.victoriametrics.com/lts-releases/). * BUGFIX: [dashboards](https://github.com/VictoriaMetrics/VictoriaMetrics/blob/master/dashboards) for Single-node VictoriaMetrics, cluster: The free disk space calculation now will subtract the size of the `-storage.minFreeDiskSpaceBytes` flag to correctly display the remaining available space of Single-node VictoriaMetrics/vmstorage rather than the actual available disk space, as well as the full ETA. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/7334) for the details. * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert): properly set `group_name` and `file` fields for recording rules in `/api/v1/rules`. +* BUGFIX: [Single-node VictoriaMetrics](https://docs.victoriametrics.com/) and `vmstorage` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/cluster-victoriametrics/): prevent panic when ingesting samples which are outisde of configured [retention filters](https://docs.victoriametrics.com/#retention-filters). This could happen when backfilling data with retention filters which exclude samples from the backfill range. * BUGFIX: [vmctl](https://docs.victoriametrics.com/vmctl/): fix issue with series matching for `vmctl vm-native` with `--vm-native-disable-per-metric-migration` flag enabled. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/7309). ## [v1.105.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.105.0) diff --git a/lib/storage/partition.go b/lib/storage/partition.go index ab70c3791..3d9f01879 100644 --- a/lib/storage/partition.go +++ b/lib/storage/partition.go @@ -745,6 +745,9 @@ func (pt *partition) mustMergeInmemoryParts(pws []*partWrapper) []*partWrapper { }() pw := pt.mustMergeInmemoryPartsFinal(pwsChunk) + if pw == nil { + return + } pwsResultLock.Lock() pwsResult = append(pwsResult, pw) @@ -806,6 +809,11 @@ func (pt *partition) mustMergeInmemoryPartsFinal(pws []*partWrapper) *partWrappe } mpDst.ph = *ph + // resulting part is empty, no need to create a part wrapper + if ph.BlocksCount == 0 { + return nil + } + return newPartWrapperFromInmemoryPart(mpDst, flushToDiskDeadline) } diff --git a/lib/storage/partition_test.go b/lib/storage/partition_test.go index 2643347a5..ea234a50c 100644 --- a/lib/storage/partition_test.go +++ b/lib/storage/partition_test.go @@ -150,3 +150,39 @@ func newTestPartWrappersForSizes(sizes []uint64) []*partWrapper { } return pws } + +func TestMergeInMemoryPartsEmptyResult(t *testing.T) { + pt := &partition{} + s := newTestStorage() + s.retentionMsecs = 1000 + defer stopTestStorage(s) + pt.s = s + var pws []*partWrapper + + const ( + inMemoryPartsCount = 5 + rowsCount = 10 + ) + + for range inMemoryPartsCount { + rows := make([]rawRow, rowsCount) + for i := range rowsCount { + rows[i].TSID = TSID{ + MetricID: uint64(i), + } + rows[i].Value = float64(i) + rows[i].Timestamp = int64(i) + rows[i].PrecisionBits = 64 + } + + pws = append(pws, &partWrapper{ + mp: newTestInmemoryPart(rows), + p: &part{}, + }) + } + + pwsNew := pt.mustMergeInmemoryParts(pws) + if len(pwsNew) != 0 { + t.Fatalf("unexpected non-empty pwsNew: %d", len(pwsNew)) + } +}