-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Don't interrupt the query path if deletes aren't available #6368
Don't interrupt the query path if deletes aren't available #6368
Conversation
./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% |
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.
LGTM. one minor nit.
@@ -177,7 +177,7 @@ func (q *SingleTenantQuerier) SelectSamples(ctx context.Context, params logql.Se | |||
|
|||
params.SampleQueryRequest.Deletes, err = q.deletesForUser(ctx, params.Start, params.End) | |||
if err != nil { | |||
return nil, err | |||
level.Error(spanlogger.FromContext(ctx)).Log("msg", "failed loading deletes for user", "err", err) |
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: since you saying for user
, nice to include tenat-id
as well in the error log?
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.
I think that should be covered by the spanlogger: https://github.com/grafana/dskit/blob/ebb5c6de233d26951aef41eada3cab3295ecb368/spanlogger/spanlogger.go#L99-L111
(cherry picked from commit efda5b2)
(cherry picked from commit efda5b2)
To avoid showing deleted data, the querier originally failed when deletes were unavailable. If the compactor becomes unavailable this can cause Loki to stop responding to queries altogether. This PR changes the querier to log an error but continue servicing the request when it fails to get deletes. Extends PR #6368 Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
To avoid showing deleted data, the querier originally failed when deletes were unavailable. If the compactor becomes unavailable this can cause Loki to stop responding to queries altogether. This PR changes the querier to log an error but continue servicing the request when it fails to get deletes. Extends PR #6368 Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
To avoid showing deleted data, the querier originally failed when deletes were unavailable. If the compactor becomes unavailable this can cause Loki to stop responding to queries altogether. This PR changes the querier to log an error but continue servicing the request when it fails to get deletes. Extends PR #6368 Signed-off-by: Christian Haudum <christian.haudum@gmail.com> (cherry picked from commit a1745c7)
To avoid showing deleted data, the querier originally failed when deletes were unavailable. If the compactor becomes unavailable this can cause Loki to stop responding to queries altogether. This PR changes the querier to log an error but continue servicing the request when it fails to get deletes. Extends PR #6368 Signed-off-by: Christian Haudum <christian.haudum@gmail.com> (cherry picked from commit a1745c7)
To avoid showing deleted data, the querier originally failed when deletes were unavailable. If the compactor becomes unavailable this can cause Loki to stop responding to queries altogether. This PR changes the querier to log an error but continue servicing the request when it fails to get deletes. Extends PR #6368 Signed-off-by: Christian Haudum <christian.haudum@gmail.com> (cherry picked from commit a1745c7)
… (#6683) To avoid showing deleted data, the querier originally failed when deletes were unavailable. If the compactor becomes unavailable this can cause Loki to stop responding to queries altogether. This PR changes the querier to log an error but continue servicing the request when it fails to get deletes. Extends PR #6368 Signed-off-by: Christian Haudum <christian.haudum@gmail.com> (cherry picked from commit a1745c7) Co-authored-by: Christian Haudum <christian.haudum@gmail.com>
To avoid showing deleted data, the querier originally failed when deletes were unavailable. If the compactor becomes unavailable this can cause loki to stop responding to queries altogether. This PR changes the querier to log an error but continue servicing the request when it fails to get deletes.
It also adds metrics to the
delete_requests_client
so we can alert when there is a chronic failure to get delete requests.