-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[PLAT-104290] auto detect/remove overlapping blocks to be compacted to avoid compactor panics #23
Conversation
9298d89
to
283cdac
Compare
Signed-off-by: Yi Jin <yi.jin@databricks.com>
e5c7ca0
to
22159f6
Compare
@@ -591,8 +591,6 @@ func (f *BaseFetcher) fetch(ctx context.Context, metrics *FetcherMetrics, filter | |||
if tenant, ok := m.Thanos.Labels[metadata.TenantLabel]; ok { | |||
numBlocksByTenant[tenant]++ | |||
} else { | |||
level.Error(f.logger).Log("msg", "found blocks without label "+metadata.TenantLabel, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is verbose and not useful in testing
Signed-off-by: Yi Jin <yi.jin@databricks.com>
Signed-off-by: Yi Jin <yi.jin@databricks.com>
1141c66
to
7445895
Compare
@@ -284,10 +283,6 @@ func NewDefaultGrouper( | |||
Name: "thanos_compact_group_vertical_compactions_total", | |||
Help: "Total number of group compaction attempts that resulted in a new block based on overlapping blocks.", | |||
}, []string{"resolution"}), | |||
overlappingBlocks: promauto.With(reg).NewCounterVec(prometheus.CounterOpts{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move this metric to its own source file, this will avoid conflicts when merge from upstream.
cmd/thanos/compact.go
Outdated
@@ -786,6 +789,9 @@ func (cc *compactConfig) registerFlag(cmd extkingpin.FlagClause) { | |||
"NOTE: This flag is ignored and (enabled) when --deduplication.replica-label flag is set."). | |||
Hidden().Default("false").BoolVar(&cc.enableVerticalCompaction) | |||
|
|||
cmd.Flag("compact.remove-overlapping", "In house flag to remove overlapping blocks. Turn this on to fix PLAT-104290."). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No point in putting a Jira ticket number in a public repo here. Just briefly describe the issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aha, forgot this is public, let me fix it.
c606168
to
e6dbcf2
Compare
Signed-off-by: Yi Jin <yi.jin@databricks.com>
Adding a flag
enableOverlappingRemoval
to control this.This will replace the
DefaultCompactionLifecycleCallback
as a new callback.This handles overlapping blocks before actually doing the compaction which could potentially causing halting or panic like thanos-io#6775
Changes
Verification
Deployed to dev, no panics, and overlapped blocks got deleted: