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 test utility function MkBasicSeriesWithBlooms #11909

Merged
merged 4 commits into from
Feb 9, 2024

Conversation

chaudum
Copy link
Contributor

@chaudum chaudum commented Feb 9, 2024

What this PR does / why we need it:

The utility correctly populated the blooms, but returned incorrect values for keys, which are the indexed keys of a bloom.

The values need to be copied when appending to the result slice.

https://github.com/grafana/loki/pull/11909/files#diff-8aa1bba39beb779ea35f85fa9309b10d08637a2f6527f9fd4b26f59db79802e8L80

Additionally, the source for the tokenizer changed to be avoid duplicate keys that are present in all blooms.

@chaudum chaudum requested a review from a team as a code owner February 9, 2024 13:57
The keys returned by this function were all the same because the
byteslice was not copied, but reused in the for loop.

Also fixed the line that is tokenized to not yield duplicate tokens.

Signed-off-by: Christian Haudum <[email protected]>
@chaudum chaudum force-pushed the chaudum/fix-mkbasicserieswithblooms branch from ad54457 to 0006450 Compare February 9, 2024 13:57
@chaudum chaudum marked this pull request as draft February 9, 2024 14:14
@chaudum chaudum marked this pull request as ready for review February 9, 2024 16:29
@chaudum chaudum requested review from owen-d and salvacorts February 9, 2024 16:30
@chaudum chaudum merged commit 3efd201 into main Feb 9, 2024
8 checks passed
@chaudum chaudum deleted the chaudum/fix-mkbasicserieswithblooms branch February 9, 2024 18:28
rhnasc pushed a commit to inloco/loki that referenced this pull request Apr 12, 2024
The utility correctly populated the blooms, but returned incorrect
values for `keys`, which are the indexed keys of a bloom.

The values need to be copied when appending to the result slice.

Additionally, the source for the tokenizer changed to be avoid duplicate
keys that are present in all blooms.

---------

Signed-off-by: Christian Haudum <[email protected]>
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.

3 participants