Skip to content

Commit

Permalink
Fix MockSharedArbitrationTest.ensureMemoryPoolMaxCapacity (#11470)
Browse files Browse the repository at this point in the history
Summary:
global arbitration now runs in a separate thread. The stats updating is async to the main testing thread. So the check might be flaky in this test. Making sure global arbitration finishes before checking stats eliminates the flakiness.

Pull Request resolved: #11470

Reviewed By: xiaoxmeng

Differential Revision: D65613023

Pulled By: tanjialiang

fbshipit-source-id: 4a41fffc524b014424931f605a68f13f3375389a
  • Loading branch information
tanjialiang authored and facebook-github-bot committed Nov 11, 2024
1 parent c8c4707 commit cc48ac9
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 1 deletion.
1 change: 0 additions & 1 deletion velox/common/memory/SharedArbitrator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1384,7 +1384,6 @@ SharedArbitrator::GlobalArbitrationSection::GlobalArbitrationSection(
SharedArbitrator::GlobalArbitrationSection::~GlobalArbitrationSection() {
VELOX_CHECK(arbitrator_->globalArbitrationRunning_);
arbitrator_->globalArbitrationRunning_ = false;
;
}

std::string SharedArbitrator::kind() const {
Expand Down
3 changes: 3 additions & 0 deletions velox/common/memory/tests/MockSharedArbitratorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2287,6 +2287,7 @@ TEST_F(MockSharedArbitrationTest, ensureMemoryPoolMaxCapacity) {
memCapacity - memCapacity / 2,
false,
false}};

for (const auto& testData : testSettings) {
SCOPED_TRACE(testData.debugString());
setupMemory(
Expand Down Expand Up @@ -2322,6 +2323,8 @@ TEST_F(MockSharedArbitrationTest, ensureMemoryPoolMaxCapacity) {
} else if (testData.hasOtherTask) {
ASSERT_EQ(otherOp->reclaimer()->stats().numReclaims, 0);
}
test::SharedArbitratorTestHelper arbitratorHelper(arbitrator_);
arbitratorHelper.waitForGlobalArbitrationToFinish();
if (testData.expectedSuccess &&
(((testData.allocatedBytes + testData.requestBytes) >
testData.poolMaxCapacity) ||
Expand Down

0 comments on commit cc48ac9

Please sign in to comment.