feat: Modify optimized compaction to cover edge cases #25594

devanbenz · 2024-11-27T21:21:47Z

This PR changes the algorithm for compaction to account for the following
cases that were not previously accounted for:

Many generations with a groupsize over 2 GB
Single generation with many files and a groupsize under 2 GB

Where groupsize is the total size of the TSM files in said shard directory.

tsdb/engine/tsm1/compact.go

tsdb/engine/tsm1/engine.go

tsdb/engine/tsm1/compact.go

gwossum

First pass, mostly about comments.

This PR changes the algorithm for compaction to account for the following cases that were not previously accounted for: - Many generations with a groupsize over 2 GB - Single generation with many files and a groupsize under 2 GB Where groupsize is the total size of the TSM files in said shard directory. closes #25666

davidby-influx

I still need to review the tests more closely, as well.

tsdb/engine/tsm1/compact.go

tsdb/engine/tsm1/compact_test.go

for shards that may have over a 2 GB group size but many fragmented files (under 2 GB and under aggressive point per block count)

davidby-influx

Lots of good changes and more tests! Thanks for the effort.
I still have a few things that you may want to change. Happy to discuss things in a teleconference.

tsdb/config.go

tsdb/engine/tsm1/compact.go

tsdb/engine/tsm1/compact_test.go

davidby-influx

Great changes, really improving this code. A few more in the endless cycle...

tsdb/engine/tsm1/compact.go

tsdb/engine/tsm1/compact_test.go

davidby-influx · 2024-12-18T18:21:58Z

tsdb/config.go

@@ -77,6 +81,9 @@ const (
 	// partition snapshot compactions that can run at one time.
 	// A value of 0 results in runtime.GOMAXPROCS(0).
 	DefaultSeriesFileMaxConcurrentSnapshotCompactions = 0
+
+	// MaxTSMFileSize is the maximum size of TSM files.


tsdb/engine/tsm1/compact_test.go

devanbenz · 2024-12-18T21:00:44Z

@davidby-influx I've added checks to verify that I'm not planning PlanOptimize() or FullyCompacted() specific TSM layouts 👍

tsdb/config.go

gwossum · 2024-12-18T23:11:22Z

tsdb/engine/tsm1/compact.go

+	// PlanOptimize will return the groups for compaction, the compaction group length,
+	// and the amount of generations within the compaction group.
+	// generationCount needs to be set to decide how many points per block during compaction.
+	// This value is mostly ignored in normal compaction code paths, but,
+	// for the edge case where there is a single generation with many
+	// files under 2 GB this value is an important indicator.


Love this comment!

var called SingleGenerationReasonText var

Many tsm generations over level 4 compaction single tsm generation under level 4 compaction all in same shard. Group size is over 2 GB for each generation.

Add a check to ensure that "orphaned" levels are compacted further with the rest of the shard.

davidby-influx

Please change all require.Equal(t, 0, ... calls to require.Zero(t ... calls.

I think some of the conditionals need to change in the planner./ The scenario I am imagining is a shard that was compacted with the aggressive block count taking new writes and need to be recompacted. We need to ignore files with the aggressive block count, not just those which are exactly the default. This may also require more test to verify correctness.

I will go over the tests in more detail later this week.

davidby-influx · 2024-12-23T23:46:49Z

tsdb/engine/tsm1/compact.go

@@ -397,7 +397,7 @@ func (c *DefaultPlanner) PlanOptimize() (compactGroup []CompactionGroup, compact
 			}
 		}

-		if len(currentGen) == 0 || currentGen.level() == cur.level() {
+		if len(currentGen) == 0 || currentGen.level() >= cur.level() {


Is this the halting issue for the recent customer situation?

davidby-influx · 2024-12-23T23:49:56Z

tsdb/engine/tsm1/compact_test.go

-		t.Fatalf("tsm file length mismatch: got %v, exp %v", got, exp)
-	}
+	_, cgLen := cp.PlanLevel(1)
+	require.Equal(t, int64(0), cgLen, "compaction group length; PlanLevel(1)")


Does require.Zero work here?

davidby-influx · 2024-12-24T00:36:43Z

tsdb/engine/tsm1/compact.go

@@ -449,7 +469,7 @@ func (c *DefaultPlanner) Plan(lastWrite time.Time) ([]CompactionGroup, int64) {
 			var skip bool

 			// Skip the file if it's over the max size and contains a full block and it does not have any tombstones
-			if len(generations) > 2 && group.size() > uint64(maxTSMFileSize) && c.FileStore.BlockCount(group.files[0].Path, 1) == tsdb.DefaultMaxPointsPerBlock && !group.hasTombstones() {
+			if len(generations) > 2 && group.size() > uint64(tsdb.MaxTSMFileSize) && c.FileStore.BlockCount(group.files[0].Path, 1) == tsdb.DefaultMaxPointsPerBlock && !group.hasTombstones() {


I think the c.FileStore.BlockCount(group.files[0].Path, 1) == tsdb.DefaultMaxPointsPerBlock has to become a >= because you may produce files with the aggressive block count in an earlier compaction that you don't want to compact again.

And a test for this?

davidby-influx · 2024-12-24T00:37:24Z

tsdb/engine/tsm1/compact.go

@@ -525,7 +545,7 @@ func (c *DefaultPlanner) Plan(lastWrite time.Time) ([]CompactionGroup, int64) {
 		// Skip the file if it's over the max size and contains a full block or the generation is split
 		// over multiple files.  In the latter case, that would mean the data in the file spilled over
 		// the 2GB limit.
-		if g.size() > uint64(maxTSMFileSize) && c.FileStore.BlockCount(g.files[0].Path, 1) == tsdb.DefaultMaxPointsPerBlock {
+		if g.size() > uint64(tsdb.MaxTSMFileSize) && c.FileStore.BlockCount(g.files[0].Path, 1) == tsdb.DefaultMaxPointsPerBlock {


Same as above:
c.FileStore.BlockCount(g.files[0].Path, 1) == tsdb.DefaultMaxPointsPerBlock might need to be >=

Also, may need a test for this.

davidby-influx · 2024-12-24T00:38:20Z

tsdb/engine/tsm1/compact.go

@@ -569,7 +589,7 @@ func (c *DefaultPlanner) Plan(lastWrite time.Time) ([]CompactionGroup, int64) {
 			}

 			// Skip the file if it's over the max size and it contains a full block
-			if gen.size() >= uint64(maxTSMFileSize) && c.FileStore.BlockCount(gen.files[0].Path, 1) == tsdb.DefaultMaxPointsPerBlock && !gen.hasTombstones() {
+			if gen.size() >= uint64(tsdb.MaxTSMFileSize) && c.FileStore.BlockCount(gen.files[0].Path, 1) == tsdb.DefaultMaxPointsPerBlock && !gen.hasTombstones() {


c.FileStore.BlockCount(gen.files[0].Path, 1) == tsdb.DefaultMaxPointsPerBlock might need to be >=

Also, may need a test for this

davidby-influx · 2024-12-24T00:43:44Z

tsdb/engine/tsm1/compact_test.go

-		}
-	}
+	tsmP, pLenP := cp.Plan(time.Now().Add(-time.Second))
+	require.Equal(t, 0, len(tsmP), "compaction group; Plan()")


require.Zero ?

davidby-influx · 2024-12-24T00:44:04Z

tsdb/engine/tsm1/compact_test.go

-		t.Fatalf("tsm file length mismatch: got %v, exp %v", got, exp)
-	}
+	_, cgLen := cp.PlanLevel(1)
+	require.Equal(t, int64(0), cgLen, "compaction group length; PlanLevel(1)")


require.Zero

devanbenz force-pushed the db/4201/compaction-bugs branch from 6e9db1b to cab638c Compare December 13, 2024 17:20

devanbenz commented Dec 16, 2024

View reviewed changes

tsdb/engine/tsm1/compact.go Outdated Show resolved Hide resolved

devanbenz force-pushed the db/4201/compaction-bugs branch from 9f5098b to 8c9d7e7 Compare December 16, 2024 18:49

devanbenz changed the title ~~feat(wip): WIP modifying compaction tests~~ feat: Modify optimized compaction to cover edge cases Dec 16, 2024

devanbenz marked this pull request as ready for review December 16, 2024 19:32

gwossum reviewed Dec 16, 2024

View reviewed changes

tsdb/engine/tsm1/compact.go Outdated Show resolved Hide resolved

gwossum reviewed Dec 16, 2024

View reviewed changes

tsdb/engine/tsm1/engine.go Outdated Show resolved Hide resolved

gwossum reviewed Dec 16, 2024

View reviewed changes

tsdb/engine/tsm1/compact.go Outdated Show resolved Hide resolved

gwossum reviewed Dec 16, 2024

View reviewed changes

tsdb/engine/tsm1/compact.go Outdated Show resolved Hide resolved

gwossum reviewed Dec 16, 2024

View reviewed changes

devanbenz force-pushed the db/4201/compaction-bugs branch from 7de2cf2 to d631314 Compare December 16, 2024 23:32

devanbenz self-assigned this Dec 16, 2024

davidby-influx requested changes Dec 17, 2024

View reviewed changes

devanbenz added 4 commits December 17, 2024 15:03

feat: Modify the PR to include optimized compaction

67849ae

for shards that may have over a 2 GB group size but many fragmented files (under 2 GB and under aggressive point per block count)

feat: Use named variables for PlanOptimize

827e859

feat: adjust test comments

5387ca3

feat: code removal from debugging

3153596

devanbenz requested review from davidby-influx and gwossum December 17, 2024 21:22

feat: setting BlockCount idx value to 1

83d28ec

davidby-influx requested changes Dec 17, 2024

View reviewed changes

devanbenz added 3 commits December 18, 2024 09:41

feat: Adjust testing and add sprintf for magic vars

f896a01

feat: need to use int64 instead of int

f15d9be

feat: touch

54c8e1c

devanbenz requested a review from davidby-influx December 18, 2024 17:08

davidby-influx requested changes Dec 18, 2024

View reviewed changes

devanbenz added 3 commits December 18, 2024 14:25

feat: Adjust tests to include lower level planning function calls

403d888

feat: Fix up some tests that I forgot to adjust

23d12e1

feat: fix typo

d3afb03

feat: touch

cf657a8

devanbenz requested a review from davidby-influx December 18, 2024 20:59

gwossum reviewed Dec 18, 2024

View reviewed changes

tsdb/config.go Outdated Show resolved Hide resolved

gwossum reviewed Dec 18, 2024

View reviewed changes

devanbenz added 6 commits December 19, 2024 12:04

feat: Call SingleGenerationReason() once by initializing a

fc6ca13

var called SingleGenerationReasonText var

feat: clarify file counts for reason we are not fully compacted

4fc4d55

feat: grammar typo

c93bdfb

feat: missed a test when updating the variable! whoops!

2dd5ef4

feat: Add test for another edge case found;

479de96

Many tsm generations over level 4 compaction single tsm generation under level 4 compaction all in same shard. Group size is over 2 GB for each generation.

feat: Remove some overlapping tests

c392906

Add a check to ensure that "orphaned" levels are compacted further with the rest of the shard.

devanbenz requested a review from gwossum December 23, 2024 15:53

davidby-influx requested changes Dec 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Modify optimized compaction to cover edge cases #25594

feat: Modify optimized compaction to cover edge cases #25594

devanbenz commented Nov 27, 2024 •

edited

Loading

gwossum left a comment

davidby-influx left a comment

davidby-influx left a comment

davidby-influx left a comment

davidby-influx Dec 18, 2024

devanbenz commented Dec 18, 2024

gwossum Dec 18, 2024

davidby-influx left a comment

davidby-influx Dec 23, 2024

davidby-influx Dec 23, 2024

davidby-influx Dec 24, 2024

davidby-influx Dec 24, 2024

davidby-influx Dec 24, 2024

davidby-influx Dec 24, 2024

davidby-influx Dec 24, 2024

davidby-influx Dec 24, 2024

davidby-influx Dec 24, 2024

davidby-influx Dec 24, 2024

feat: Modify optimized compaction to cover edge cases #25594

Are you sure you want to change the base?

feat: Modify optimized compaction to cover edge cases #25594

Conversation

devanbenz commented Nov 27, 2024 • edited Loading

gwossum left a comment

Choose a reason for hiding this comment

davidby-influx left a comment

Choose a reason for hiding this comment

davidby-influx left a comment

Choose a reason for hiding this comment

davidby-influx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devanbenz commented Dec 18, 2024

Choose a reason for hiding this comment

davidby-influx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devanbenz commented Nov 27, 2024 •

edited

Loading