Skip to content

Conversation

charleskorn
Copy link
Contributor

@charleskorn charleskorn commented Apr 9, 2025

What this PR does

This PR modifies MQE's behaviour when evaluating binary operations to close their inner operators as soon as they're no longer needed.

This allows those inner operators to then free resources that might otherwise remain until query evaluation has finished. This is particularly important for cases where only a subset of series from one side match the other side, and so some intermediate state might be unnecessarily held by inner operators until the query finishes.

This PR also clarifies that it is expected that calling Close on an operator multiple times should be safe, and extends pedantic mode to call Close on the query's root operator a second time to ensure this is true.

Which issue(s) this PR fixes or relates to

(none)

Checklist

  • Tests updated.
  • [n/a] Documentation added.
  • [covered by Mimir Query Engine #10067] CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • [n/a] about-versioning.md updated with experimental features.

Copy link
Contributor

@56quarters 56quarters left a comment

Choose a reason for hiding this comment

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

LGTM but I'm not super familiar with MQE yet.

@charleskorn charleskorn marked this pull request as ready for review April 10, 2025 01:17
@charleskorn charleskorn requested a review from a team as a code owner April 10, 2025 01:17
@charleskorn charleskorn enabled auto-merge (squash) April 11, 2025 06:33
@charleskorn charleskorn merged commit 1efc9a3 into main Apr 11, 2025
26 checks passed
@charleskorn charleskorn deleted the charleskorn/mqe-close-binary-operation-sides branch April 11, 2025 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants