Support Ruler to query Query Frontend #6151

SungJin1212 · 2024-08-12T11:51:06Z

What this PR does:
Add ruler.frontend-address to allow query to query frontends instead of ingesters. If -ruler.frontend-address is set, rulers query to query frontends for evaluating rules. It can support query frontend features like vertical query sharding.

Which issue(s) this PR fixes:
Fixes #5105

Checklist

Tests updated
Documentation added
CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

yeya24

Instead of making Query Frontend a queryable, I think I prefer changing the engine query function https://github.com/cortexproject/cortex/blob/master/pkg/ruler/compat.go#L206.

Instead of using a local engine, we use a remote query frontend to evaluate the query results.

pkg/querier/query_frontend_queryable.go

SungJin1212 · 2024-08-13T12:51:52Z

@yeya24
I remake the pr. Could you please take a look?

yeya24 · 2024-08-13T16:50:09Z

pkg/ruler/compat.go

@@ -226,6 +226,33 @@ func EngineQueryFunc(engine promql.QueryEngine, q storage.Queryable, overrides R
 	}
 }

+func QueryFrontendQueryFunc(promClient *promClient, overrides RulesLimits, userID string, lookbackDelta time.Duration) rules.QueryFunc {


Can we reuse the existing EngineQueryFunc function?

pkg/ruler/compat.go

pkg/ruler/prom_client.go

SungJin1212 · 2024-08-14T02:44:04Z

@yeya24
I have reflected the review you left. Thank you for the review!

pkg/ruler/prom_client.go

pkg/ruler/ruler.go

pkg/ruler/prom_client.go

SungJin1212 · 2024-08-16T08:17:19Z

@yeya24
I have fixed the pr. Could you please take a look?

danielblando · 2024-08-16T18:53:13Z

We are in the process of releasing 1.18
Please rebase PR and change the changelog to master

SungJin1212 · 2024-08-20T01:40:07Z

@yeya24
I think we need to quey to QFE via gRPC.
I'm thinking the step as follows:

Define query request, response (including floating histogram) proto in cortex.pb
Ruler has grpc client and query to QFE via the client.

yeya24 · 2024-08-20T02:21:27Z

Make sense. I am thinking about the same thing. Query frontend already supports grpc as long as you wrap your request with httpgrpc. We don't need to define a new rpc in cortex.pb

For the protobuf change, we don't need to do it in this PR.

SungJin1212 · 2024-08-20T02:24:44Z

@yeya24
Thanks, I try implementing it.

yeya24 · 2024-08-20T02:33:19Z

@SungJin1212 This is how query frontend converts a http request to httpgrpc request https://github.com/cortexproject/cortex/blob/master/pkg/frontend/transport/roundtripper.go#L39.

We can do something similar. But from Ruler to Query Frontend.

SungJin1212 · 2024-09-01T02:06:07Z

@yeya24
Could you please take a look?

yeya24

@SungJin1212 Thanks for the great work! I think this feature looks really solid now.
We can give it a try.
I think we also lack of some documentation but it is fine to do it in another PR.

CHANGELOG.md

friedrichg

I have no concerns. Only small nits:

Mark it as experimental on:
https://cortexmetrics.io/docs/configuration/v1guarantees/#experimental-features
Architecture diagram needs update
https://cortexmetrics.io/docs/architecture/
Why is it better? I can see frontend caching is one is reason.
You could add some a guide page similar to
https://cortexmetrics.io/docs/guides/ruler-sharding/

yeya24 · 2024-09-04T15:42:13Z

@SungJin1212 As @friedrichg said, let's mark this feature as experimental in cortexmetrics.io/docs/configuration/v1guarantees#experimental-features.

Architecture diagram needs update
cortexmetrics.io/docs/architecture

I think we can skip this in this pr. We need to update it anyway.

Why is it better? I can see frontend caching is one is reason.
You could add some a guide page similar to
cortexmetrics.io/docs/guides/ruler-sharding

We will add a doc but will create another issue to track that. This item doesn't need to be owned by @SungJin1212

SungJin1212 · 2024-09-05T02:42:01Z

@yeya24 @friedrichg
I have added experimental flags and got the rebase. I think caching and using vertical sharding for evaluating rules is a main contribution of this pr.

yeya24 · 2024-09-05T05:28:32Z

docs/configuration/config-file-reference.md

+
+  # Timeout for downstream rulers.
+  # CLI flag: -ruler.frontendClient.remote-timeout
+  [remote_timeout: <duration> | default = 2m]


What's this flag for? This seems not used. I wonder if we should consolidate this flag with frontend timeout as they seem to do the same thing.
However, the flag description seems hardcoded to Timeout for downstream rulers..

yeya24 · 2024-09-05T05:32:55Z

pkg/ruler/frontend_client_pool.go

+type frontendPool struct {
+	timeout              time.Duration
+	prometheusHTTPPrefix string
+	grpcConfig           ClientConfig


I wonder if we should use grpcclient.Config instead?
ClientConfig seems used for Ruler to talk to each other only

SungJin1212 · 2024-09-05T05:45:48Z

@yeya24
There were some mistakes when rebasing. I have fixed it!

Signed-off-by: SungJin1212 <[email protected]>

SungJin1212 · 2024-09-06T23:56:32Z

@friedrichg
Could you please take a look?

friedrichg

Lgtm

yeya24 · 2024-09-07T03:46:44Z

Thanks for the contribution!

pull-request-size bot added the size/L label Aug 12, 2024

SungJin1212 marked this pull request as draft August 12, 2024 11:51

SungJin1212 force-pushed the add-query-frontend-queryable branch 7 times, most recently from cec96af to 4d42fcf Compare August 12, 2024 12:42

yeya24 reviewed Aug 13, 2024

View reviewed changes

pkg/querier/query_frontend_queryable.go Outdated Show resolved Hide resolved

pkg/querier/query_frontend_queryable.go Outdated Show resolved Hide resolved

SungJin1212 force-pushed the add-query-frontend-queryable branch 3 times, most recently from 7ff9818 to 84d16b4 Compare August 13, 2024 12:42

yeya24 reviewed Aug 13, 2024

View reviewed changes

SungJin1212 force-pushed the add-query-frontend-queryable branch 2 times, most recently from d579b4b to 062885e Compare August 14, 2024 02:41

SungJin1212 changed the title ~~Add query frontend queryable~~ Support Ruler to query Query Frontend Aug 14, 2024

SungJin1212 marked this pull request as ready for review August 14, 2024 04:13

yeya24 reviewed Aug 15, 2024

View reviewed changes

pkg/ruler/prom_client.go Outdated Show resolved Hide resolved

pkg/ruler/ruler.go Show resolved Hide resolved

pkg/ruler/prom_client.go Outdated Show resolved Hide resolved

pkg/ruler/prom_client.go Outdated Show resolved Hide resolved

SungJin1212 force-pushed the add-query-frontend-queryable branch from 062885e to aba8591 Compare August 16, 2024 08:13

SungJin1212 force-pushed the add-query-frontend-queryable branch 2 times, most recently from c78a066 to 35f8879 Compare August 17, 2024 05:31

SungJin1212 requested a review from yeya24 August 28, 2024 10:45

SungJin1212 force-pushed the add-query-frontend-queryable branch 4 times, most recently from 248c350 to f1e2a3d Compare August 31, 2024 06:05

SungJin1212 force-pushed the add-query-frontend-queryable branch 2 times, most recently from 006b710 to 0324695 Compare September 2, 2024 06:54

yeya24 approved these changes Sep 3, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

SungJin1212 force-pushed the add-query-frontend-queryable branch from 0324695 to bb300a5 Compare September 3, 2024 06:33

yeya24 requested review from friedrichg and CharlieTLe September 4, 2024 00:47

friedrichg requested changes Sep 4, 2024

View reviewed changes

SungJin1212 force-pushed the add-query-frontend-queryable branch from bb300a5 to 2d88c97 Compare September 5, 2024 02:36

SungJin1212 requested a review from friedrichg September 5, 2024 02:56

yeya24 reviewed Sep 5, 2024

View reviewed changes

SungJin1212 force-pushed the add-query-frontend-queryable branch from 2d88c97 to 3a74a24 Compare September 5, 2024 05:45

Support Ruler to query Query Frontend

83db99d

Signed-off-by: SungJin1212 <[email protected]>

SungJin1212 force-pushed the add-query-frontend-queryable branch from 3a74a24 to 83db99d Compare September 5, 2024 05:52

SungJin1212 requested a review from yeya24 September 6, 2024 01:10

friedrichg approved these changes Sep 7, 2024

View reviewed changes

yeya24 merged commit 1523080 into cortexproject:master Sep 7, 2024
16 checks passed

This was referenced Sep 9, 2024

Add documentation for Ruler remote evaluation via Query Frontend #6198

Closed

Query Frontend: add new field for dense native histogram format #6199

Merged

SungJin1212 mentioned this pull request Nov 18, 2024

Support ruler to retrieve proto format query response #6345

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Ruler to query Query Frontend #6151

Support Ruler to query Query Frontend #6151

SungJin1212 commented Aug 12, 2024 •

edited

Loading

yeya24 left a comment

SungJin1212 commented Aug 13, 2024

yeya24 Aug 13, 2024

SungJin1212 commented Aug 14, 2024

SungJin1212 commented Aug 16, 2024

danielblando commented Aug 16, 2024

SungJin1212 commented Aug 20, 2024 •

edited

Loading

yeya24 commented Aug 20, 2024 •

edited

Loading

SungJin1212 commented Aug 20, 2024

yeya24 commented Aug 20, 2024

SungJin1212 commented Sep 1, 2024

yeya24 left a comment

friedrichg left a comment

yeya24 commented Sep 4, 2024

SungJin1212 commented Sep 5, 2024 •

edited

Loading

yeya24 Sep 5, 2024

yeya24 Sep 5, 2024

SungJin1212 commented Sep 5, 2024

SungJin1212 commented Sep 6, 2024

friedrichg left a comment

yeya24 commented Sep 7, 2024

Support Ruler to query Query Frontend #6151

Support Ruler to query Query Frontend #6151

Conversation

SungJin1212 commented Aug 12, 2024 • edited Loading

yeya24 left a comment

Choose a reason for hiding this comment

SungJin1212 commented Aug 13, 2024

yeya24 Aug 13, 2024

Choose a reason for hiding this comment

SungJin1212 commented Aug 14, 2024

SungJin1212 commented Aug 16, 2024

danielblando commented Aug 16, 2024

SungJin1212 commented Aug 20, 2024 • edited Loading

yeya24 commented Aug 20, 2024 • edited Loading

SungJin1212 commented Aug 20, 2024

yeya24 commented Aug 20, 2024

SungJin1212 commented Sep 1, 2024

yeya24 left a comment

Choose a reason for hiding this comment

friedrichg left a comment

Choose a reason for hiding this comment

yeya24 commented Sep 4, 2024

SungJin1212 commented Sep 5, 2024 • edited Loading

yeya24 Sep 5, 2024

Choose a reason for hiding this comment

yeya24 Sep 5, 2024

Choose a reason for hiding this comment

SungJin1212 commented Sep 5, 2024

SungJin1212 commented Sep 6, 2024

friedrichg left a comment

Choose a reason for hiding this comment

yeya24 commented Sep 7, 2024

SungJin1212 commented Aug 12, 2024 •

edited

Loading

SungJin1212 commented Aug 20, 2024 •

edited

Loading

yeya24 commented Aug 20, 2024 •

edited

Loading

SungJin1212 commented Sep 5, 2024 •

edited

Loading