FelixYBW commented on PR #11249:
URL: 
https://github.com/apache/incubator-gluten/pull/11249#issuecomment-3611229018

   Thank you, @rui-mo . It's a good summary of the memory leak issue we 
analyzed so far. We noted the issue in this 
https://github.com/facebookincubator/velox/pull/8205 but it's not merged and we 
work around it in Gluten by adding the wait time during memory manager close. 
   
   With current upstream velox, there are two issues:
   1. The risk of segfault leading by 4) in Rui's comment.
   2. Then pending I/O is executed any way even we doesn't need the data at all.
   
   What we should do and PR14722 does is to wait the current I/O request to 
finish and close the pending I/O request. But it may cause Prestoc++ deadlock. 
So PR14722 can hardly be merged into upstream. Instead we will add a factory to 
create customized input in Gluten which use PR14722.
   
   We temporarily picked PR14722 into gluten/velox now. With this PR we should 
solve all the I/O thread memleak issue totally. Let's know if you noted more 
failure.
   
   In theory with PR14722 we needn't the wait in the PR, we still use it for 
other potential memleaks.


-- 
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]

Reply via email to