nandorKollar commented on PR #13976: URL: https://github.com/apache/iceberg/pull/13976#issuecomment-3287765532
> > This make sense, the recommendation I made will break parallel tests for sure. Do you happen to have a recommendation how we make sure that no one touches an allocator they don't have access to for closing? Would Kevin's previous idea help there? We won't expose the root allocator, but instead arrowAllocation adds a new method which will create a new child allocator for the readers. It is always the readers responsibility to close it. Just some thoughts, hope it make some sense. We can't really change rootAllocator method though, as it is already public. > > I don't think even Kevin's idea will really help since the if we had a child allocator coming out of the ArrowAllocator it would also just get abandoned. We need to figure out how to get the vectors we need without touching the root and instead by touching the Allocator that the ParquetBuilder is using I think that's what I try to achieve with this PR. In this PR, the only place, where we use the root allocator is when we create a child allocator for the `VectorizedReaderBuilder`. The builder will then use this child allocator to create new `VectorizedArrowReader` instances. Isn't this what we try to achieve? No doubt, the variable name `rootAllocator` in `VectorizedReaderBuilder` is no longer accurate, also, we might add an extra validation in the constructor to verify that the `bufferAllocator` parameter is not the root allocator (`bufferAllocator.getRoot() != bufferAllocator`). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
