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

boundscheck & partitioning fingerprints between blocks #11237

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

owen-d
Copy link
Member

@owen-d owen-d commented Nov 15, 2023

Adds a few utilities for comparing fingerprints to blocks that cover a specific fingerprint range. Will likely need to be refactored with more comprehensive types, but the logic still applies.

Comment on lines +158 to +160
bounded := boundedRequests{
bounds: block,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the slice reqs not need to be allocated?


// reqs models a set of requests covering many fingerprints.
// consumers models a set of blocks covering different fingerprint ranges
func partitionFingerprintRange(reqs [][]model.Fingerprint, blocks []FingerprintBounds) (res []boundedRequests) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You have a named return value, but you return the value also explicitly.
Personally I'm not a big fan of named return values, because they can hide allocation problems.

Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
Copy link
Contributor

Trivy scan found the following vulnerabilities:

@owen-d owen-d merged commit 58eaad9 into grafana:main Nov 17, 2023
7 checks passed
jeschkies pushed a commit that referenced this pull request Nov 21, 2023
Adds a few utilities for comparing fingerprints to blocks that cover a
specific fingerprint range. Will likely need to be refactored with more
comprehensive types, but the logic still applies.
rhnasc pushed a commit to inloco/loki that referenced this pull request Apr 12, 2024
Adds a few utilities for comparing fingerprints to blocks that cover a
specific fingerprint range. Will likely need to be refactored with more
comprehensive types, but the logic still applies.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants