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

Fix fingerprint construction and comparison bugs #121

Merged
merged 2 commits into from
Apr 9, 2013

Conversation

juliusv
Copy link
Member

@juliusv juliusv commented Apr 9, 2013

No description provided.

}

return false
return f.String() < o.String()
Copy link
Member

Choose a reason for hiding this comment

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

One caveat:

What about the stringified LENGTH in the format HASH-F-LENGTH-L? Will this handle said case appropriately?

Copy link
Member Author

Choose a reason for hiding this comment

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

Question: String() is an alias for ToRowKey(), so even if "1000" < "2" by that definition, that ordering is still consistent with what's on disk, right?

Copy link
Member

Choose a reason for hiding this comment

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

labelMatterLength -> labelMatterModulus?

labelMatterModulus := labelMatterLength % 10

Format string takes precision up to two digits? Then you can merely f.String() < o.String()?

Copy link
Member

Choose a reason for hiding this comment

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

I believe that is correct. I would want to verify it, though.

2013/4/9 juliusv notifications@github.com

In model/fingerprinting.go:

@@ -151,20 +152,7 @@ func (f fingerprint) LastCharacterOfLastLabelValue() string {
}

func (f fingerprint) Less(o Fingerprint) bool {

  • if f.Hash() < o.Hash() {
  •   return true
    
  • }
  • if f.FirstCharacterOfFirstLabelName() < o.FirstCharacterOfFirstLabelName() {
  •   return true
    
  • }
  • if f.LabelMatterLength() < o.LabelMatterLength() {
  •   return true
    
  • }
  • if f.LastCharacterOfLastLabelValue() < o.LastCharacterOfLastLabelValue() {
  •   return true
    

- }

  • return false
  • return f.String() < o.String()

Question: String() is an alias for ToRowKey(), so even if "1000" < "2" by
that definition, that ordering is still consistent with what's on disk,
right?


Reply to this email directly or view it on GitHubhttps://github.com//pull/121/files#r3710121
.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, the DTO fingerprint is just a proto version of String() / ToRowKey():

func (f fingerprint) ToDTO() *dto.Fingerprint {
  return &dto.Fingerprint{
    Signature: proto.String(f.ToRowKey()),
  }
}

and then the LevelDB key is built from that via:

targetKey = &dto.SampleKey{
  Fingerprint: fingerprint.ToDTO(),
  Timestamp: fooTime,
}

So my theory should hold. Ok to just keep my code as it is now then?

@matttproud
Copy link
Member

Aside from that small nit, 👍

Seems like just using String() is the easiest way of doing this.
juliusv added a commit that referenced this pull request Apr 9, 2013
…ison

Fix fingerprint construction and comparison bugs
@juliusv juliusv merged commit e254c0b into master Apr 9, 2013
@juliusv juliusv deleted the julius-fix-fingerprint-comparison branch April 9, 2013 09:44
simonpasquier pushed a commit to simonpasquier/prometheus that referenced this pull request Oct 12, 2017
Add a blog post on detecting and dealing with outliers.
arajkumar pushed a commit to arajkumar/prometheus that referenced this pull request Apr 7, 2022
Bug 2056802: scrape: Fix label_limits cache usage
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.

None yet

2 participants