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

Don't interrupt the query path if deletes aren't available #6368

Conversation

MasslessParticle
Copy link
Contributor

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.

@MasslessParticle MasslessParticle requested a review from a team as a code owner June 10, 2022 21:24
@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%

Copy link
Contributor

@kavirajk kavirajk left a 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)
Copy link
Contributor

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?

Copy link
Contributor Author

@MasslessParticle MasslessParticle Jun 13, 2022

Choose a reason for hiding this comment

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

@owen-d owen-d merged commit efda5b2 into grafana:main Jun 13, 2022
grafanabot pushed a commit that referenced this pull request Jun 13, 2022
MasslessParticle added a commit that referenced this pull request Jun 16, 2022
)

(cherry picked from commit efda5b2)

Co-authored-by: Travis Patterson <travis.patterson@grafana.com>
grafanabot pushed a commit that referenced this pull request Jun 23, 2022
MichelHollands pushed a commit that referenced this pull request Jun 23, 2022
)

(cherry picked from commit efda5b2)

Co-authored-by: Travis Patterson <travis.patterson@grafana.com>
grafanabot added a commit that referenced this pull request Jun 24, 2022
)

(cherry picked from commit efda5b2)

Co-authored-by: Travis Patterson <travis.patterson@grafana.com>
(cherry picked from commit f79ff95)
vlad-diachenko pushed a commit that referenced this pull request Jun 24, 2022
) (#6493)

(cherry picked from commit efda5b2)

Co-authored-by: Travis Patterson <travis.patterson@grafana.com>
(cherry picked from commit f79ff95)
chaudum added a commit that referenced this pull request Jun 30, 2022
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>
MasslessParticle pushed a commit that referenced this pull request Jun 30, 2022
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>
grafanabot pushed a commit that referenced this pull request Jun 30, 2022
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)
chaudum added a commit that referenced this pull request Jun 30, 2022
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)
grafanabot pushed a commit that referenced this pull request Jul 14, 2022
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)
MasslessParticle pushed a commit that referenced this pull request Jul 14, 2022
… (#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>
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.

6 participants