Skip to content

Enable querier execution from logical plan #6908

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

Closed

Conversation

rubywtl
Copy link
Contributor

@rubywtl rubywtl commented Jul 24, 2025

What this PR does:
This PR enables the queriers to execute a logical plan derived from the serialized logical plan in the request body, under the distributed execution feature flag. It includes a fallback to the PromQL engine when necessary.

Unit tests have been added to verify that queries are correctly executed from the logical plan in the request body when expected, as opposed to the query string in the request URL

Which issue(s) this PR fixes:
Addresses the problem outlined in proposal 6789

Checklist

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

@rubywtl rubywtl force-pushed the querier/logical-plan-execution branch from fe31a41 to 2f83e12 Compare July 24, 2025 01:37
@rubywtl
Copy link
Contributor Author

rubywtl commented Jul 24, 2025

An open question: when we expect to execute from a logical plan but receive an empty plan parameter, should we
(1) treat this as an error (since the frontend in this case should provide a plan) and return a error: 500,
or (2) handle it gracefully by falling back to the PromQL query string?

My current implementation does the fallback, but I'm wondering if this masks potential frontend issues we should catch instead

@rubywtl rubywtl force-pushed the querier/logical-plan-execution branch from 2f83e12 to 422b1f2 Compare July 24, 2025 17:32
@rubywtl rubywtl force-pushed the querier/logical-plan-execution branch from a7a4faf to d984196 Compare July 24, 2025 18:53
Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

Can you also add a new integration test for this?

alexqyle and others added 8 commits July 28, 2025 11:18
* add zone label to ring_members metric

Signed-off-by: Alex Le <[email protected]>

* update changelog

Signed-off-by: Alex Le <[email protected]>

* fix nil pointer

Signed-off-by: Alex Le <[email protected]>

---------

Signed-off-by: Alex Le <[email protected]>
…#6873)

* Querier: Support configuring optimizers and XFunctions

Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>

* Address comments

Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>

---------

Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
### Main fixes applied:

1. **Line 5**: Fixed "long term storage" → "long-term storage" (consistency with hyphenation)

2. **Line 12**: Fixed "Long term storage" → "Long-term storage" (consistency with hyphenation)

3. **Line 126**: Fixed "the AMP" → "AMP" (removed unnecessary article - "the" before abbreviation when referring to the service directly)

4. **Line 126**: Fixed "managed monitoring for your containers" → "managed monitoring service for your containers" (added missing word "service" for clarity)

### Minor grammar improvements:

- **Hyphenation consistency**: Made sure "long-term" is consistently hyphenated throughout
- **Article usage**: Corrected the use of "the" before abbreviations where it wasn't needed
- **Completeness**: Added missing words to make sentences grammatically complete

The document now has consistent terminology, proper grammar, and professional language throughout. All the technical content, links, and formatting remain intact while the language is now more polished and consistent.

Signed-off-by: Charlie Le <[email protected]>
@rubywtl rubywtl marked this pull request as draft July 28, 2025 18:50
@rubywtl rubywtl closed this Jul 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants