Skip to content
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

verify: Add separated blocks issue and fix. #280

Closed
wants to merge 2 commits into from
Closed

Conversation

bwplotka
Copy link
Member

Signed-off-by: Bartek Plotka [email protected]

// - another ovelapped block with range 20-50
// - bigger block is missing exactly these sources from smaller block to fill its range.
// Various bugs could introduce these blocks, including broken compaction for Prometheus 2.2.0.
func SeparatedBlocks(ctx context.Context, logger log.Logger, bkt objstore.Bucket, backupBkt objstore.Bucket, repair bool) error {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mh, can you elaborate a bit more what motivated this one. It seems like the starting position the example describes is actually valid, i.e. not an issue.

If the compactor does not pick that input up for compaction, that's probably just: prometheus-junkyard/tsdb#90

Though it can of course involve risk if the input blocks aren't truly disjoint in their actual data.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example states the broken state (overlapping), why you say it is not an issue?

Thanks for referencing this issue. It all makes sense, so maybe we want to add something like that for TSDB? I mean at least explicit VerticalCompact for now not used internally yet. To compact overlapped blocks.

Wonder however if compactor should do that automatically. For now any overlap case is invalid state. It can be introduced only by bug. It should not happen. Of course we could make it differently -> allow or don't care about overlaps and just "heal" them by something like vertical compact, true, but not sure if that's the right path. Not for now for sure.

For now I would really merge this for now here.

@bwplotka bwplotka changed the base branch from debug-dir to master April 17, 2018 13:41
@bwplotka bwplotka closed this Mar 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants