-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(memory-tracking): implement arrow_buffer::MemoryPool for MemoryPool #18928
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
base: main
Are you sure you want to change the base?
feat(memory-tracking): implement arrow_buffer::MemoryPool for MemoryPool #18928
Conversation
| parquet_encryption = [ | ||
| "parquet/encryption", | ||
| ] | ||
| arrow_buffer_pool = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The feature-flag name is fairly descriptive. Maybe we could change it to something better. I'm open to suggestions!
| fn available(&self) -> isize { | ||
| // The pool may be overfilled, so this method might return a negative value. | ||
| (self.capacity() as i128 - self.used() as i128) | ||
| .try_into() | ||
| .unwrap_or(isize::MIN) | ||
| } | ||
|
|
||
| fn used(&self) -> usize { | ||
| self.inner.reserved() | ||
| } | ||
|
|
||
| fn capacity(&self) -> usize { | ||
| match self.inner.memory_limit() { | ||
| MemoryLimit::Infinite | MemoryLimit::Unknown => usize::MAX, | ||
| MemoryLimit::Finite(capacity) => capacity, | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really sure these methods have a use-case... I'm open to removing them in the upstream trait.
Which issue does this PR close?
Rationale for this change
Related to #16841. The ability to correctly account for memory usage of arrow buffers in execution nodes is crucial to maximise resource usage while preventing OOMs.
What changes are included in this PR?
arrow_buffer_poolfeature-flagAre these changes tested?
Yes!
Are there any user-facing changes?
Introduced new API.