-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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(blooms): Ship chunkrefs in task payload #13677
Conversation
message ProtoGapWithBlocks { | ||
ProtoFingerprintBounds bounds = 1 [ | ||
(gogoproto.nullable) = false, | ||
(gogoproto.jsontag) = "bounds" | ||
]; | ||
repeated string blockRef = 2; | ||
repeated ProtoSeries series = 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During a rollout, this may make builders running an older version to fail to deserialize the task. This is fine: task will be retried and eventually the builder will restart.
d07adce
to
92d67f6
Compare
message ProtoSeries { | ||
uint64 fingerprint = 1; | ||
repeated logproto.ShortRef chunks = 2; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This type is essentially the same as GroupedChunkRefs
:
uint64 fingerprint = 1;
string tenant = 2;
repeated ShortRef refs = 3;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might make the planner slower, but let's see.
LGTM
(cherry picked from commit 450bbce)
What this PR does / why we need it:
This PR adds series w/ chunkrefs to the task definition so builders do not need to download TSDBs to load the series and tasks.
We do this since if we are building blooms for the current day, as the compactor updates and deletes outdated TSDBs, builders may fail to load the referenced (already removed) TSDBs from the object store. This results in a task failure.
Notes:
loki/pkg/storage/bloom/v1/builder.go
Lines 264 to 269 in d8cc1ce
Checklist
CONTRIBUTING.md
guide (required)feat
PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.docs/sources/setup/upgrade/_index.md
production/helm/loki/Chart.yaml
and updateproduction/helm/loki/CHANGELOG.md
andproduction/helm/loki/README.md
. Example PRdeprecated-config.yaml
anddeleted-config.yaml
files respectively in thetools/deprecated-config-checker
directory. Example PR