Skip to content

Conversation

@sopel39
Copy link
Member

@sopel39 sopel39 commented Jul 7, 2025

Operator#finish might be called even if operator is blocked (e.g. waiting for memory). Therefore, memory available condition needs to be re-checked every time finish is called.

Description

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

## General
* Reduce number of worker OOMs when running join queries. ({issue}`26142`)

sopel39 added 2 commits June 25, 2025 17:37
Operator#finish might be called even if operator is blocked (e.g. waiting
for memory). Therefore, memory available condition needs to be re-checked
every time finish is called.
@sopel39 sopel39 requested a review from raunaqmorarka July 7, 2025 18:25
@cla-bot cla-bot bot added the cla-signed label Jul 7, 2025
@sopel39
Copy link
Member Author

sopel39 commented Jul 7, 2025

cc @osscm

@raunaqmorarka raunaqmorarka requested a review from pettyjamesm July 7, 2025 21:17
Copy link
Member

@pettyjamesm pettyjamesm left a comment

Choose a reason for hiding this comment

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

LGTM. Seems like the previous behavior could result in unnecessary OOMs? If so, this PR probably merits a release note mention.

@sopel39
Copy link
Member Author

sopel39 commented Jul 9, 2025

LGTM. Seems like the previous behavior could result in unnecessary OOMs? If so, this PR probably merits a release note mention.

It's not guaranteed you will hit OOM without this PR. I will suggest some RN

@sopel39 sopel39 merged commit 24a9369 into trinodb:master Jul 9, 2025
279 of 281 checks passed
@sopel39 sopel39 deleted the oss/fix_waiting_for_mem branch July 9, 2025 10:02
@github-actions github-actions bot added this to the 477 milestone Jul 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants