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

Fixed compact failed due to duplicate labels #542

Conversation

jojohappy
Copy link
Member

Signed-off-by: jojohappy sarahdj0917@gmail.com

Changes

fixes #497

Just print out an error log to prompt the series have duplicate labels. I think it is an unusual case, so I do nothing for the that.

Verification

Signed-off-by: jojohappy <sarahdj0917@gmail.com>
@jojohappy jojohappy force-pushed the fix_compactor_err_with_duplicate_labels branch from 67b402f to 4d8f023 Compare September 28, 2018 10:37
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Not sure about all of this, sure it unblock peope temporarily until the problem will not grow to enormous sizes (label will be duplicated 1000 times, because of 1000 compactions the does not handle this gracefully).

The questions is again.. why this happens.. who creates that broken blocks. (: I think we need to rethink the whole block sanity problem and how we handle this.

@@ -282,7 +283,9 @@ func GatherIndexIssueStats(logger log.Logger, fn string, minTime int64, maxTime
}
l0 := lset[0]
for _, l := range lset[1:] {
if l.Name <= l0.Name {
if l.Name == l0.Name {
Copy link
Member

Choose a reason for hiding this comment

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

It only makes sense if l.Value == l0.Value as well I guess.

Copy link
Member Author

Choose a reason for hiding this comment

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

But we would hit the case that with same label name and different value. I think it has been not make sense with duplicate label name.

@jojohappy
Copy link
Member Author

Totally agree with you @bwplotka. So should we open a ticket in upstream (Prometheus)?

@bwplotka
Copy link
Member

bwplotka commented Oct 1, 2018

Yes, if you can repro the duplicated label problem.

Let's close this for now until we have a common way of handling those issues.

@bwplotka bwplotka closed this Oct 1, 2018
@jojohappy jojohappy deleted the fix_compactor_err_with_duplicate_labels branch January 18, 2019 15:12
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.

compact: consider handling duplicate labels, or continuing on error
2 participants