paleolimbot commented on PR #13706: URL: https://github.com/apache/arrow/pull/13706#issuecomment-1232964386
@westonpace Whenever you get a chance, the C++ code here needs a set of eyes, particularly because it involves the lifecycle of the ExecPlan (notably, getting multiple ExecPlans to run under the same R event loop so that we can evaluate user-defined functions). I was hoping that this change would also make it more clear when we `StartProducing()` and `StopProducing()`...previously we *did* call `StopProducing()` when the result `RecordBatchReader` was garbage collected but since we don't have any control of when that happens, we couldn't guarantee a prompt stop request for something like `head()` (I know that a prompt stop request doesn't stop the plan immediately, but it sounds like at some point it will). I was also hoping this change would fix the sporadic memory leaks observed, which as you'll see from the valgrind crossbow jobs above, it does not. The checks above also confirm that those leaks are not a result of the IO thread pool shutting down (although occasionally one can observe a direct leak of an ExecPlan). -- 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: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
