zhztheplayer commented on pull request #7030: URL: https://github.com/apache/arrow/pull/7030#issuecomment-766616108
Hi @emkornfield @pitrou I think I have addressed existing comments so would you like to take another look now? Thanks a lot. Since this was submitted long time ago (before release 1.0.0 IIRC) and I've perform big rebase for times, If we could make progress I would like to help with it in any way. I know to review a PR creating a new module is usually not trivial work, and current shape of the implementation might be not that perfect for everybody so I may want to suggest we can break up current/future goal to minor patches like we have already done on [the topic of filters](https://github.com/apache/arrow/pull/7030#discussion_r426990234) for this PR. Based on some recent comments I think we may don't have to make 100% consensus on the current design of `ReservationListenableMemoryPool` or relevant things instantly, if this part is in a way controversial I could split out the part from this PR then create a new one for it after this get merged (should I? would you suggest that). Everything will be working fine enough without it based on our experience (yes we are relying on this feature in our own project for long time). That doesn't mean it is not a "problem" but we can put discussions in separate places to avoid this from being hard to review and merge. The review history is already too long to give every reviewer information he need yet. And if you have other suggestions on the whole design or architecture of the basic functionality please be free to add comments and I am happy to make further refactors accordingly. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
