Skip to content

Commit

Permalink
Fix NPE caused by testingRunArbitration (facebookincubator#9137)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: facebookincubator#9137

testingRunArbitration is called from within an operator to force a call to shrinkPools to simulate what would happen if
arbitration was triggered by a call to growCapacity.

It does not check if the pool was aborted due to an error during arbitration which leads to all sorts of memory issues as the
Task will be terminated as part of aborting the pool causing operators to get destructed.

Adding a check to simulate what already happens in growCapacity.

Reviewed By: xiaoxmeng

Differential Revision: D55038530

fbshipit-source-id: be443619235956a43bd08158dca276126cca4789
  • Loading branch information
Kevin Wilfong authored and facebook-github-bot committed Mar 20, 2024
1 parent 1d95a47 commit 21491d7
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 0 deletions.
4 changes: 4 additions & 0 deletions velox/common/memory/MemoryArbitrator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -478,5 +478,9 @@ void testingRunArbitration(
static_cast<MemoryPoolImpl*>(pool)->testingManager()->shrinkPools(
targetBytes, allowSpill);
pool->leaveArbitration();

// This function is simulating an arbitration triggered by growCapacity, which
// would check this.
static_cast<MemoryPoolImpl*>(pool)->testingCheckIfAborted();
}
} // namespace facebook::velox::memory
4 changes: 4 additions & 0 deletions velox/common/memory/MemoryPool.h
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,10 @@ class MemoryPoolImpl : public MemoryPool {
return allocator_;
}

void testingCheckIfAborted() const {
checkIfAborted();
}

uint64_t testingMinReservationBytes() const {
return minReservationBytes_;
}
Expand Down

0 comments on commit 21491d7

Please sign in to comment.