From 483da4366012a1216e7f92e482af9bee95a6d9c5 Mon Sep 17 00:00:00 2001 From: Bartek Plotka Date: Tue, 13 Mar 2018 12:30:27 +0000 Subject: [PATCH] compact: Exclude last block with shortest range instead of newest one by ULID. Fixes https://github.com/prometheus/prometheus/issues/3943 issue. Added tests. Signed-off-by: Bartek Plotka --- compact.go | 23 ++++++++++--- compact_test.go | 86 +++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 94 insertions(+), 15 deletions(-) diff --git a/compact.go b/compact.go index 3668f0831..e028e5ec4 100644 --- a/compact.go +++ b/compact.go @@ -151,16 +151,11 @@ func (c *LeveledCompactor) Plan(dir string) ([]string, error) { if err != nil { return nil, err } - // We do not include the most recently created block. This gives users a window - // of a full block size to piece-wise backup new data without having to care - // about data overlap. if len(dirs) < 1 { return nil, nil } - dirs = dirs[:len(dirs)-1] var dms []dirMeta - for _, dir := range dirs { meta, err := readMetaFile(dir) if err != nil { @@ -172,6 +167,10 @@ func (c *LeveledCompactor) Plan(dir string) ([]string, error) { } func (c *LeveledCompactor) plan(dms []dirMeta) ([]string, error) { + // We do not include the most recently created block with the shortest range (the block which was just created from WAL). + // This gives users a window of a full block size to piece-wise backup new data without having to care about data overlap. + dms = excludeFreshMeta(dms) + sort.Slice(dms, func(i, j int) bool { return dms[i].meta.MinTime < dms[j].meta.MinTime }) @@ -199,6 +198,20 @@ func (c *LeveledCompactor) plan(dms []dirMeta) ([]string, error) { return nil, nil } +// excludeFreshMeta removes newest block with the shortest range from given meta files. +func excludeFreshMeta(dms []dirMeta) []dirMeta { + sort.Slice(dms, func(i, j int) bool { + leftRange := dms[i].meta.MaxTime - dms[i].meta.MinTime + rightRange := dms[j].meta.MaxTime - dms[j].meta.MinTime + if leftRange == rightRange { + return dms[i].meta.MinTime < dms[j].meta.MinTime + } + + return leftRange > rightRange + }) + return dms[:len(dms)-1] +} + // selectDirs returns the dir metas that should be compacted into a single new block. // If only a single block range is configured, the result is always nil. func (c *LeveledCompactor) selectDirs(ds []dirMeta) []dirMeta { diff --git a/compact_test.go b/compact_test.go index 116bb3d1a..e4ec376ce 100644 --- a/compact_test.go +++ b/compact_test.go @@ -172,8 +172,7 @@ func TestLeveledCompactor_plan(t *testing.T) { }, expected: nil, }, - // We should wait for a third block of size 20 to appear before compacting - // the existing ones. + // We should wait for four blocks of size 20 to appear before compacting. { metas: []dirMeta{ metaRange("1", 0, 20, nil), @@ -181,26 +180,49 @@ func TestLeveledCompactor_plan(t *testing.T) { }, expected: nil, }, + // We should wait for a next block of size 20 to appear before compacting + // the existing ones. We have three, but we ignore the fresh one with shortest range. + { + metas: []dirMeta{ + metaRange("1", 0, 20, nil), + metaRange("2", 20, 40, nil), + metaRange("3", 40, 60, nil), + }, + expected: nil, + }, // Block to fill the entire parent range appeared – should be compacted. { metas: []dirMeta{ metaRange("1", 0, 20, nil), metaRange("2", 20, 40, nil), metaRange("3", 40, 60, nil), + metaRange("4", 60, 80, nil), }, expected: []string{"1", "2", "3"}, }, - // Block for the next parent range appeared. Nothing will happen in the first one - // anymore and we should compact it. + // Block for the next parent range appeared with gap with size 20. Nothing will happen in the first one + // anymore but we ignore fresh one still, so no compaction. { metas: []dirMeta{ metaRange("1", 0, 20, nil), metaRange("2", 20, 40, nil), metaRange("3", 60, 80, nil), }, + expected: nil, + }, + // Block for the next parent range appeared, and we have a gap with size 20 between second and third block. + // We will not get this missed gap anymore and we should compact just these two. + { + metas: []dirMeta{ + metaRange("1", 0, 20, nil), + metaRange("2", 20, 40, nil), + metaRange("3", 60, 80, nil), + metaRange("4", 80, 100, nil), + }, expected: []string{"1", "2"}, }, { + // We have 20, 20, 20, 60, 60 range blocks. "3" is marked as fresh one, so we ignore it during compaction. metas: []dirMeta{ metaRange("1", 0, 20, nil), metaRange("2", 20, 40, nil), @@ -208,27 +230,54 @@ func TestLeveledCompactor_plan(t *testing.T) { metaRange("4", 60, 120, nil), metaRange("5", 120, 180, nil), }, - expected: []string{"1", "2", "3"}, + expected: []string{"1", "2"}, }, { + // We have 20, 60, 60, 240 range blocks. We can compact 60 + 60 only, because the newest one with the + // shortest range is not included in planning (4). metas: []dirMeta{ metaRange("2", 20, 40, nil), metaRange("4", 60, 120, nil), metaRange("5", 120, 180, nil), metaRange("6", 720, 960, nil), }, + expected: []string{"4", "5"}, + }, + { + // We got fresh block if size 20. We can compact 20 + 60 + 60. + metas: []dirMeta{ + metaRange("2", 20, 40, nil), + metaRange("4", 60, 120, nil), + metaRange("5", 120, 180, nil), + metaRange("6", 720, 960, nil), + metaRange("7", 960, 980, nil), + }, expected: []string{"2", "4", "5"}, }, { + // We have 60, 20, 20, 20 range blocks. + // Fresh block with shortest range block is ignored (6), so nothing can be compacted. metas: []dirMeta{ metaRange("1", 0, 60, nil), metaRange("4", 60, 80, nil), metaRange("5", 80, 100, nil), metaRange("6", 100, 120, nil), }, + expected: nil, + }, + { + // We have 60, 20, 20, 20, 20 range blocks. + // Fresh block with shortest range block is ignored but compaction 20 + 20 + 20 is possible. + metas: []dirMeta{ + metaRange("1", 0, 60, nil), + metaRange("4", 60, 80, nil), + metaRange("5", 80, 100, nil), + metaRange("6", 100, 120, nil), + metaRange("6", 120, 140, nil), + }, expected: []string{"4", "5", "6"}, }, - // Select large blocks that have many tombstones. + // Do not select large blocks that have many tombstones when there is no fresh block. { metas: []dirMeta{ metaRange("1", 0, 720, &BlockStats{ @@ -236,15 +285,27 @@ func TestLeveledCompactor_plan(t *testing.T) { NumTombstones: 3, }), }, + expected: nil, + }, + // Select large blocks that have many tombstones when fresh appears. + { + metas: []dirMeta{ + metaRange("1", 0, 720, &BlockStats{ + NumSeries: 10, + NumTombstones: 3, + }), + metaRange("2", 720, 740, nil), + }, expected: []string{"1"}, }, - // For small blocks, do not compact tombstones. + // For small blocks, do not compact tombstones, even when fresh appears. { metas: []dirMeta{ metaRange("1", 0, 30, &BlockStats{ NumSeries: 10, NumTombstones: 3, }), + metaRange("2", 720, 740, nil), }, expected: nil, }, @@ -256,16 +317,21 @@ func TestLeveledCompactor_plan(t *testing.T) { NumSeries: 0, NumTombstones: 0, }), + metaRange("2", 720, 740, nil), }, expected: nil, }, } for _, c := range cases { - res, err := compactor.plan(c.metas) - testutil.Ok(t, err) + if !t.Run("", func(t *testing.T) { + res, err := compactor.plan(c.metas) + testutil.Ok(t, err) - testutil.Equals(t, c.expected, res) + testutil.Equals(t, c.expected, res) + }) { + return + } } }