-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
refactor(blooms): Forward created metas from builder to planner #13133
refactor(blooms): Forward created metas from builder to planner #13133
Conversation
aca3280
to
8d89a53
Compare
} | ||
|
||
func (r *TaskResult) ToProtoTaskResult() *ProtoTaskResult { | ||
if r.Error != nil { |
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.
nit: return nil
if r == nil
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.
Done
pkg/bloombuild/planner/planner.go
Outdated
}() | ||
|
||
taskTimeout := p.limits.BuilderResponseTimeout(task.Tenant) | ||
if taskTimeout == 0 { | ||
// If the timeout is 0 (disabled), just wait for the builder to respond | ||
return <-errCh | ||
result := <-resultsCh | ||
return result.Error |
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.
nit: add TODO: process result
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.
Refactored the function a bit for better readability
What this PR does / why we need it:
We forward the list of created metas from the builder to the planner so we can later compute the outdated metas and blocks for deletion.
Special notes for your reviewer:
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