nandorKollar commented on PR #13976: URL: https://github.com/apache/iceberg/pull/13976#issuecomment-3285753183
> > Here we check that root allocator has no unallocated memory left after each test case. I'm not sure that this actually covers child allocators too, I suppose it does (though it is easy to add a verification for child allocators here, if it doesn't). What do you thing? > > We can't actually do that for another reason, it will break parallel test runs since multiple tests could be using the root allocator so we can legitimately have allocators open after a test. I think your original intuition that we need to not use the root allocator directly for these special columns is probably right. We need to refactor such that no one touches an allocator they don't have access to for closing. 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. -- 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]
