Skip to content

Commit

Permalink
Fix flaky test MockSharedArbitrationTest.arbitrateBySelfMemoryReclaim (
Browse files Browse the repository at this point in the history
…facebookincubator#11397)

Summary:
Pull Request resolved: facebookincubator#11397

The test originally intended to test local arbitration. But after refactoring, this test is by default relying on global arbitration for reclaiming. Because global arbitration runs in a different thread, there is a race condition between the actual reclaim and the reclaimed bytes check in the test. Making the test disable global arbitration solves the flakiness.

Reviewed By: xiaoxmeng

Differential Revision: D65259718

fbshipit-source-id: 86a2937c620d1ac58bd9b76c9948f9d9b0aaa0d5
  • Loading branch information
Jialiang Tan authored and facebook-github-bot committed Oct 31, 2024
1 parent 1e7a82f commit c89d8d6
Showing 1 changed file with 15 additions and 4 deletions.
19 changes: 15 additions & 4 deletions velox/common/memory/tests/MockSharedArbitratorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3283,14 +3283,26 @@ DEBUG_ONLY_TEST_F(MockSharedArbitrationTest, reclaimWithNoCandidate) {
}

TEST_F(MockSharedArbitrationTest, arbitrateBySelfMemoryReclaim) {
const std::vector<bool> isLeafReclaimables = {true, false};
for (const auto isLeafReclaimable : isLeafReclaimables) {
for (const auto isLeafReclaimable : {true, false}) {
SCOPED_TRACE(fmt::format("isLeafReclaimable {}", isLeafReclaimable));
const uint64_t memCapacity = 128 * MB;
const uint64_t reservedCapacity = 8 * MB;
const uint64_t poolReservedCapacity = 4 * MB;
setupMemory(
memCapacity, reservedCapacity, reservedCapacity, poolReservedCapacity);
memCapacity,
reservedCapacity,
reservedCapacity,
poolReservedCapacity,
0,
0,
0,
0,
0,
0,
0,
kMemoryReclaimThreadsHwMultiplier,
nullptr,
false);
std::shared_ptr<MockTask> task = addTask(kMemoryCapacity);
auto* memOp = addMemoryOp(task, isLeafReclaimable);
const int allocateSize = 8 * MB;
Expand All @@ -3305,7 +3317,6 @@ TEST_F(MockSharedArbitrationTest, arbitrateBySelfMemoryReclaim) {
memOp->allocate(memCapacity), "Exceeded memory pool cap");
ASSERT_EQ(oldNumRequests + 1, arbitrator_->stats().numRequests);
ASSERT_EQ(arbitrator_->stats().numFailures, 1);
continue;
} else {
memOp->allocate(memCapacity / 2);
ASSERT_EQ(oldNumRequests + 1, arbitrator_->stats().numRequests);
Expand Down

0 comments on commit c89d8d6

Please sign in to comment.