Skip to content

Add properties to update memory in TableFinishOperator#16095

Merged
arhimondr merged 1 commit intoprestodb:masterfrom
viczhang861:track_stats_memory
May 14, 2021
Merged

Add properties to update memory in TableFinishOperator#16095
arhimondr merged 1 commit intoprestodb:masterfrom
viczhang861:track_stats_memory

Conversation

@viczhang861
Copy link
Contributor

@viczhang861 viczhang861 commented May 13, 2021

When system memory used by TableFinishOperator is tracked
and included in query total memory, query may hit memory
limit and fail.

== RELEASE NOTES ==

General Changes
* Memory tracking in TableFinishOperator can be enabled by setting the `table-finish-operator-memory-tracking-enabled` configuration property to `true`.

@viczhang861 viczhang861 requested a review from a team May 13, 2021 21:19
Copy link
Member

@arhimondr arhimondr left a comment

Choose a reason for hiding this comment

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

LGTM % comments

Copy link
Member

Choose a reason for hiding this comment

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

nit: tableFinishOperatorMemoryTrackingEnabled (also get/set and the property name)

Copy link
Member

Choose a reason for hiding this comment

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

Ideally we shouldn't allow users disable memory tracking. Let's have it as a configuration property only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, removed

When system memory used by TableFinishOperator is tracked
and included in query total memory, query may hit memory
limit and fail.
@viczhang861 viczhang861 force-pushed the track_stats_memory branch from 2293480 to cc8ac49 Compare May 14, 2021 02:10
@viczhang861
Copy link
Contributor Author

@arhimondr arhimondr merged commit 779afcc into prestodb:master May 14, 2021
@viczhang861 viczhang861 deleted the track_stats_memory branch May 14, 2021 20:39
@sujay-jain sujay-jain mentioned this pull request May 21, 2021
10 tasks
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.

2 participants