Skip to content

Commit

Permalink
Fix test MockSharedArbitratorTest.arbitrationFailure (#11450)
Browse files Browse the repository at this point in the history
Summary:
Previous fix on global arbitration abort reverted the condition of abort (prev wrong behavior vs curr correct behavior). But some tests were not expecting that and still wait for a long time before abort, causing test timeout.

Pull Request resolved: #11450

Reviewed By: xiaoxmeng, bikramSingh91

Differential Revision: D65513731

Pulled By: tanjialiang

fbshipit-source-id: 663da937cf3dc7d691ce79cfbb3b14e452e56b43
  • Loading branch information
tanjialiang authored and facebook-github-bot committed Nov 6, 2024
1 parent 3ca03d8 commit 7f3588e
Showing 1 changed file with 64 additions and 5 deletions.
69 changes: 64 additions & 5 deletions velox/common/memory/tests/MockSharedArbitratorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,9 @@ class MockSharedArbitrationTest : public testing::Test {
{std::string(ExtraConfig::kGlobalArbitrationEnabled),
folly::to<std::string>(globalArtbitrationEnabled)},
{std::string(ExtraConfig::kGlobalArbitrationWithoutSpill),
folly::to<std::string>(globalArbitrationWithoutSpill)}};
folly::to<std::string>(globalArbitrationWithoutSpill)},
{std::string(ExtraConfig::kGlobalArbitrationAbortTimeRatio),
folly::to<std::string>(globalArbitrationAbortTimeRatio)}};
options.arbitrationStateCheckCb = std::move(arbitrationStateCheckCb);
options.checkUsageLeak = true;
manager_ = std::make_unique<MemoryManager>(options);
Expand Down Expand Up @@ -858,6 +860,26 @@ TEST_F(MockSharedArbitrationTest, asyncArbitrationWork) {

// Test different kinds of arbitraton failures.
TEST_F(MockSharedArbitrationTest, arbitrationFailures) {
// Set the globalArbitrationAbortTimeRatio to be very small so that the query
// can be aborted sooner and the test would not timeout.
setupMemory(
kMemoryCapacity,
0,
0,
0,
0,
0,
0,
0,
0,
0,
0,
kMemoryReclaimThreadsHwMultiplier,
nullptr,
true,
5 * 60 * 1'000'000'000UL,
false,
0.005);
// Local arbitration failure with exceeded capacity limit.
{
auto task = addTask(64 * MB);
Expand Down Expand Up @@ -3027,8 +3049,27 @@ TEST_F(MockSharedArbitrationTest, minReclaimBytes) {
for (const auto& testData : testSettings) {
SCOPED_TRACE(testData.debugString());

// Make simple settings to focus shrink capacity logic testing.
setupMemory(memoryCapacity, 0, 0, 0, 0, 0, 0, 0, testData.minReclaimBytes);
// Make simple settings to focus shrink capacity logic testing. Set the
// globalArbitrationAbortTimeRatio to be very small so that the query
// can be aborted sooner and the test would not timeout.
setupMemory(
memoryCapacity,
0,
0,
0,
0,
0,
0,
0,
testData.minReclaimBytes,
0,
0,
kMemoryReclaimThreadsHwMultiplier,
nullptr,
true,
5 * 60 * 1'000'000'000UL,
false,
0.005);
std::vector<TestTaskContainer> taskContainers;
for (const auto& testTask : testData.testTasks) {
auto task = addTask();
Expand Down Expand Up @@ -3903,8 +3944,26 @@ TEST_F(MockSharedArbitrationTest, arbitrationFailure) {

for (const auto& testData : testSettings) {
SCOPED_TRACE(testData.debugString());

setupMemory(maxCapacity, 0, initialCapacity, 0);
// Set the globalArbitrationAbortTimeRatio to be very small so that the
// query can be aborted sooner and the test would not timeout.
setupMemory(
maxCapacity,
0,
initialCapacity,
0,
0,
0,
0,
0,
0,
0,
0,
kMemoryReclaimThreadsHwMultiplier,
nullptr,
true,
5 * 60 * 1'000'000'000UL,
false,
0.005);
std::shared_ptr<MockTask> requestorTask = addTask();
MockMemoryOperator* requestorOp = addMemoryOp(requestorTask, false);
requestorOp->allocate(testData.requestorCapacity);
Expand Down

0 comments on commit 7f3588e

Please sign in to comment.