FelixYBW commented on code in PR #11249:
URL: 
https://github.com/apache/incubator-gluten/pull/11249#discussion_r2589391954


##########
cpp/velox/memory/VeloxMemoryManager.cc:
##########
@@ -445,27 +445,11 @@ bool VeloxMemoryManager::tryDestructSafe() {
 }
 
 VeloxMemoryManager::~VeloxMemoryManager() {
-  static const uint32_t kWaitTimeoutMs = 
FLAGS_gluten_velox_async_timeout_on_task_stopping; // 30s by default
-  uint32_t accumulatedWaitMs = 0UL;
   bool destructed = false;
-  for (int32_t tryCount = 0; accumulatedWaitMs < kWaitTimeoutMs; tryCount++) {

Review Comment:
   request cancel should be used for different case. In this case, the task 
finishes normally, not is cancelled. 
   8025's close() is the most elegant way to close the pending I/O request but 
it changes too much velox code and even hard to get merged. In theory velox has 
a close() for each operator which should free all resources the operator holds. 
   
   14722 relys on the class descrution to close the resource. It's easier for 
us to pick.



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