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

[TSDB] fingerprint offsets is now correctly 16 byte aligned #6630

Merged
merged 10 commits into from
Jul 8, 2022

Conversation

owen-d
Copy link
Member

@owen-d owen-d commented Jul 8, 2022

This contains a few fixes I've found over the past few days of debugging TSDB operations (sorry for being all over the place), but the important bits are

  1. Correctly calculates storage.SeriesRef offsets in the fingerprintOffsets table in 7a34b15. This resulted in an incredibly hard to find bug in sharding code because it wasn't:
    • Using section offsets to increase the starting offsets for our series references
    • Using 16 byte offsets of the file position not series number

It caused everything but the last shard to be skipped most off the time because the actual references were waaay higher than expected
2) Simplifies the query planning algorithm to consistently prefer smaller units of work.

ref #5428

@owen-d owen-d requested a review from a team as a code owner July 8, 2022 09:46
owen-d added 10 commits July 8, 2022 06:01
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
@owen-d owen-d changed the title [TSDB] [TSDB] fingerprint offsets is now correctly 16 byte aligned Jul 8, 2022
@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0.6%

Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM

@owen-d owen-d merged commit 094f8f3 into grafana:main Jul 8, 2022
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