Copilot commented on code in PR #13029:
URL: https://github.com/apache/gluten/pull/13029#discussion_r4017683024
##########
cpp/velox/memory/GlutenDirectBufferedInput.h:
##########
@@ -81,25 +34,21 @@ class GlutenDirectBufferedInput : private
detail::ExecutorBarrierHolder,
folly::Executor* executor,
const facebook::velox::io::ReaderOptions& readerOptions,
folly::F14FastMap<std::string, std::string> fileReadOps = {})
- : ExecutorBarrierHolder(executor),
- DirectBufferedInput(
+ : DirectBufferedInput(
std::move(readFile),
metricsLog,
std::move(fileNum),
std::move(tracker),
std::move(groupId),
std::move(ioStatistics),
std::move(ioStats),
- barrier(),
+ executor,
Review Comment:
Passing the raw executor here removes the only per-input drain for queued
async loads. The destructor cancels `kPlanned` loads and clears them, but a
not-yet-scheduled `AsyncLoadHolder` can remain queued on the executor and
retain the task's `MemoryPool` until after this input/runtime is torn down;
this is the `pools_.size() != 0` failure tracked by #12911. Please retain an
equivalent barrier/drain (or include the alternative fix) rather than reverting
to the raw executor.
--
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]
