Hook up partition compaction end to end implementation #6510
Conversation
…fecycle Signed-off-by: Alex Le <leqiyue@amazon.com>
Signed-off-by: Alex Le <leqiyue@amazon.com>
Signed-off-by: Alex Le <leqiyue@amazon.com>
Signed-off-by: Alex Le <leqiyue@amazon.com>
| @@ -698,15 +754,26 @@ func (c *Compactor) stopping(_ error) error { | |||
| } | |||
|
|
|||
| func (c *Compactor) running(ctx context.Context) error { | |||
| // Ensure an initial cleanup occurred as first thing when running compactor. | |||
| if err := services.StartAndAwaitRunning(ctx, c.blocksCleaner); err != nil { | |||
There was a problem hiding this comment.
Is there a specific reason why we have to move cleaning here?
There was a problem hiding this comment.
Because cleaner cycle might be running for a while depending on how many tenants and how big each tenants are. We don't want to compactor got into unhealthy state in the ring because of long running cleaner process.
|
|
||
| func (f *DisabledDeduplicateFilter) DuplicateIDs() []ulid.ULID { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Can we make these types private?
Can you add some comment to DisabledDeduplicateFilter. We want to disable duplicate filter because it makes no sense for partitioning compactor as we always have duplicates?
There was a problem hiding this comment.
The DefaultDeduplicateFilter from Thanos would mark duplicate blocks if those blocks are from same group and having same source blocks. In partitioning compactor, partitions from same time range would always or eventually have same source as it is the natural of partitioning compactor. We don't want those blocks got filtered out when doing grouping for next level compaction.
|
|
||
| globalMaxt := blocks[0].Meta().MaxTime | ||
| g, _ := errgroup.WithContext(ctx) | ||
| g.SetLimit(8) |
There was a problem hiding this comment.
Is this a sane default to set in Cortex?
There was a problem hiding this comment.
With my test 8 is good enough to keep CPU busy during compaction. I am wondering if this number is too high for end user, would this just cause CPU usage peaked at 100% for longer time?
| } | ||
| if b.Meta().MaxTime > globalMaxt { | ||
| globalMaxt = b.Meta().MaxTime | ||
| } |
There was a problem hiding this comment.
Although it is part of the original tsdb implementation and we also pass tsdb metrics in the function. I feel it is kind of unecessary to check block overlapping for partitioning compactor. Especially the info log above seems always there
There was a problem hiding this comment.
Make sense. I will remove that part.
Signed-off-by: Alex Le <leqiyue@amazon.com>
Signed-off-by: Alex Le <leqiyue@amazon.com>
What this PR does:
Implement partitioning compaction related lifecycle functions to make partitioning compaction end to end working.
PartitionCompactionBlockDeletableCheckermakes sure no parent blocks got deleted after each compaction. Cleaner would handle parent blocks clean up for partitioning compaction.ShardedBlockPopulatorwould useShardedPostingto including particular series in the result block.ShardedCompactionLifecycleCallbackis used to emit partitioning compaction metrics at beginning and end of compaction. It also initializeShardedBlockPopulatorfor each compaction.Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]