Skip to content

[Backport] ScanQueryFrameProcessor: Close CursorHolders as we go along. (#17152) (#17168)#17245

Merged
kfaraz merged 2 commits intoapache:31.0.0from
kfaraz:backport_17152
Oct 4, 2024
Merged

[Backport] ScanQueryFrameProcessor: Close CursorHolders as we go along. (#17152) (#17168)#17245
kfaraz merged 2 commits intoapache:31.0.0from
kfaraz:backport_17152

Conversation

@kfaraz
Copy link
Copy Markdown
Contributor

@kfaraz kfaraz commented Oct 4, 2024

Backport the following:
#17152
#17168

…17152)

* ScanQueryFrameProcessor: Close CursorHolders as we go along.

The change in apache#16533 added CursorHolders to the processor-level Closer.
This is problematic when running on an input channel: it means we started
keeping around all CursorHolders for all frames we process and closing them
when the channel is complete, rather than closing them as we go along.

This patch restores the prior behavior, where resources are closed as we go.

* Fix other call sites.

* Fix reference.

* Improvements.
@kfaraz kfaraz added the Backport label Oct 4, 2024
@kfaraz kfaraz added this to the 31.0.0 milestone Oct 4, 2024
@github-actions github-actions Bot added Area - Batch Ingestion Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 labels Oct 4, 2024
…ervals (apache#17168)

* fix issue with ScanQueryFrameProcessor cursor build not adjusting intervals

* all hail the robot overlords

* style bot
@kfaraz kfaraz changed the title [Backport] ScanQueryFrameProcessor: Close CursorHolders as we go along. (#17152) [Backport] ScanQueryFrameProcessor: Close CursorHolders as we go along. (#17152) (#17168) Oct 4, 2024
Copy link
Copy Markdown
Contributor

@LakshSingla LakshSingla left a comment

Choose a reason for hiding this comment

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

Please "Rebase and Merge" the PR instead of "Squash and Merge", otherwise these commits will mess up with the find-missing-backports script afair.

@kfaraz
Copy link
Copy Markdown
Contributor Author

kfaraz commented Oct 4, 2024

Please "Rebase and Merge" the PR instead of "Squash and Merge", otherwise these commits will mess up with the find-missing-backports script afair.

Yeah, I used to think so too. But checked the script and also verified with some other backports.
It works as long as each PR is called out in the commit message in the format (#<pr-number>).
e.g. The message Backport multiple commits (#1234) (#5678) would mark both #1234 and #5678 as backported.

@kfaraz kfaraz merged commit c1622be into apache:31.0.0 Oct 4, 2024
@kfaraz kfaraz deleted the backport_17152 branch October 4, 2024 11:43
@LakshSingla
Copy link
Copy Markdown
Contributor

@kfaraz I see. Thanks for the clarification

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area - Batch Ingestion Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 Backport

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants