Improve compaction of sporadic blocks #7329
Conversation
jhalterman
force-pushed
the
sporadic-block-compaction
branch
2 times, most recently
from
189a256
to
1f2317b
Compare
| @@ -142,10 +142,6 @@ func TestMultitenantCompactor_ShouldSupportSplitAndMergeCompactor(t *testing.T) | |||
| block1 := createTSDBBlock(t, bkt, userID, 0, (5 * time.Minute).Milliseconds(), numSeries, externalLabels("")) | |||
| block2 := createTSDBBlock(t, bkt, userID, time.Minute.Milliseconds(), (7 * time.Minute).Milliseconds(), numSeries, externalLabels("")) | |||
|
|
|||
| // Add another block as "most recent one" otherwise the previous blocks are not compacted | |||
There was a problem hiding this comment.
These additional blocks are no longer needed since the new split_merge_grouper conditions allow compaction to proceed based on the job's time being < now.
jhalterman
force-pushed
the
sporadic-block-compaction
branch
5 times, most recently
from
1f671e4
to
4dfe3c1
Compare
| @@ -549,7 +550,6 @@ func testMetadataQueriesWithBlocksStorage( | |||
| resp []prompb.Label | |||
| } | |||
| type labelValuesTest struct { | |||
| label string | |||
There was a problem hiding this comment.
Removed this field since it had the same value in all test cases.
There was a problem hiding this comment.
Nice job! LGTM. I think this is a nice improvement and safe change.
I'd like to hear what @pracucci thinks, as he introduced the check in the first place and may better remember the reasons
The check was an old one. It was originally inherited by Prometheus (which does something similar), and was introduce in a era where blocks backfilling, OOO ingestion and -compactor.first-level-compaction-wait-period were not a thing.
| @@ -42,6 +42,7 @@ func Test_MaxSeriesAndChunksPerQueryLimitHit(t *testing.T) { | |||
| "-blocks-storage.tsdb.retention-period": blockRangePeriod.String(), // We want blocks to be immediately deleted from ingesters. | |||
| "-blocks-storage.tsdb.ship-interval": "1s", | |||
| "-blocks-storage.tsdb.head-compaction-interval": "500ms", | |||
| "-compactor.first-level-compaction-wait-period": "1m", // Do not compact aggressively | |||
There was a problem hiding this comment.
If this is the default you want for integration tests, then it's better to do the change in BlocksStorageFlags (defined in integration/configs.go).
There was a problem hiding this comment.
Making this the default causes some other ITs, which expect immediate L1 compaction, to fail. I'll leave the current default for now and just override for these 2 tests.
This PR improves compaction of blocks that are created sporadically, by relaxing the highestMaxTime check to exclude jobs where the max compaction level is 1, or where the job is at least 1 job range in the past.
jhalterman
force-pushed
the
sporadic-block-compaction
branch
2 times, most recently
from
2288406
to
8d17aca
Compare
What this PR does
This PR changes the split_merge_grouper's check that's meant to filter recent blocks based on the
highestMaxTime, to allow jobs where either the:Together, these conditions address users who upload blocks sporadically, where currently compaction does not occur as expected (as described in #7265).
Which issue(s) this PR fixes or relates to
Fixes #7265
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX].about-versioning.mdupdated with experimental features.