From a43e329c36dceb445eac53c729db20d74b8ab8a6 Mon Sep 17 00:00:00 2001 From: Jialiang Tan Date: Mon, 18 Mar 2024 16:31:43 -0700 Subject: [PATCH] Enable arbitrator in OperatorTestBase by default --- velox/exec/tests/AggregationTest.cpp | 6 ------ velox/exec/tests/GroupedExecutionTest.cpp | 1 - velox/exec/tests/HashJoinTest.cpp | 10 ---------- velox/exec/tests/RowNumberTest.cpp | 10 ---------- velox/exec/tests/TaskTest.cpp | 10 ---------- velox/exec/tests/utils/OperatorTestBase.cpp | 15 ++++----------- velox/exec/tests/utils/OperatorTestBase.h | 2 -- .../tests/utils/AggregationTestBase.cpp | 10 ---------- 8 files changed, 4 insertions(+), 60 deletions(-) diff --git a/velox/exec/tests/AggregationTest.cpp b/velox/exec/tests/AggregationTest.cpp index 107cc59099dbc..968d768951023 100644 --- a/velox/exec/tests/AggregationTest.cpp +++ b/velox/exec/tests/AggregationTest.cpp @@ -138,16 +138,10 @@ void checkSpillStats(PlanNodeStats& stats, bool expectedSpill) { class AggregationTest : public OperatorTestBase { protected: static void SetUpTestCase() { - FLAGS_velox_testing_enable_arbitration = true; OperatorTestBase::SetUpTestCase(); TestValue::enable(); } - static void TearDownTestCase() { - FLAGS_velox_testing_enable_arbitration = false; - OperatorTestBase::TearDownTestCase(); - } - void SetUp() override { OperatorTestBase::SetUp(); filesystems::registerLocalFileSystem(); diff --git a/velox/exec/tests/GroupedExecutionTest.cpp b/velox/exec/tests/GroupedExecutionTest.cpp index 209401f6aa2af..6d53ef982f87a 100644 --- a/velox/exec/tests/GroupedExecutionTest.cpp +++ b/velox/exec/tests/GroupedExecutionTest.cpp @@ -34,7 +34,6 @@ class GroupedExecutionTest : public virtual HiveConnectorTestBase { } static void SetUpTestCase() { - FLAGS_velox_testing_enable_arbitration = true; HiveConnectorTestBase::SetUpTestCase(); } diff --git a/velox/exec/tests/HashJoinTest.cpp b/velox/exec/tests/HashJoinTest.cpp index eec0053650d30..abca47ad7f22b 100644 --- a/velox/exec/tests/HashJoinTest.cpp +++ b/velox/exec/tests/HashJoinTest.cpp @@ -749,16 +749,6 @@ class HashJoinTest : public HiveConnectorTestBase { explicit HashJoinTest(const TestParam& param) : numDrivers_(param.numDrivers) {} - static void SetUpTestCase() { - FLAGS_velox_testing_enable_arbitration = true; - OperatorTestBase::SetUpTestCase(); - } - - static void TearDownTestCase() { - FLAGS_velox_testing_enable_arbitration = false; - OperatorTestBase::TearDownTestCase(); - } - void SetUp() override { HiveConnectorTestBase::SetUp(); diff --git a/velox/exec/tests/RowNumberTest.cpp b/velox/exec/tests/RowNumberTest.cpp index 6ec773d2c4774..39c1c8d90b207 100644 --- a/velox/exec/tests/RowNumberTest.cpp +++ b/velox/exec/tests/RowNumberTest.cpp @@ -26,16 +26,6 @@ namespace facebook::velox::exec::test { class RowNumberTest : public OperatorTestBase { protected: - static void SetUpTestCase() { - FLAGS_velox_testing_enable_arbitration = true; - OperatorTestBase::SetUpTestCase(); - } - - static void TearDownTestCase() { - FLAGS_velox_testing_enable_arbitration = false; - OperatorTestBase::TearDownTestCase(); - } - RowNumberTest() { filesystems::registerLocalFileSystem(); rowType_ = ROW( diff --git a/velox/exec/tests/TaskTest.cpp b/velox/exec/tests/TaskTest.cpp index 2c32216ebe3f4..a580e4f1f8816 100644 --- a/velox/exec/tests/TaskTest.cpp +++ b/velox/exec/tests/TaskTest.cpp @@ -457,16 +457,6 @@ class TestBadMemoryTranslator : public exec::Operator::PlanNodeTranslator { } // namespace class TaskTest : public HiveConnectorTestBase { protected: - static void SetUpTestCase() { - FLAGS_velox_testing_enable_arbitration = true; - OperatorTestBase::SetUpTestCase(); - } - - static void TearDownTestCase() { - FLAGS_velox_testing_enable_arbitration = false; - OperatorTestBase::TearDownTestCase(); - } - static std::pair, std::vector> executeSingleThreaded( core::PlanFragment plan, diff --git a/velox/exec/tests/utils/OperatorTestBase.cpp b/velox/exec/tests/utils/OperatorTestBase.cpp index cf09e11e394a8..8c2ffbf50a17a 100644 --- a/velox/exec/tests/utils/OperatorTestBase.cpp +++ b/velox/exec/tests/utils/OperatorTestBase.cpp @@ -32,10 +32,6 @@ DECLARE_bool(velox_memory_leak_check_enabled); DECLARE_bool(velox_enable_memory_usage_track_in_default_memory_pool); -DEFINE_bool( - velox_testing_enable_arbitration, - false, - "Enable to turn on arbitration for tests by default"); using namespace facebook::velox::common::testutil; using namespace facebook::velox::memory; @@ -56,17 +52,14 @@ OperatorTestBase::~OperatorTestBase() { } void OperatorTestBase::SetUpTestCase() { - FLAGS_velox_enable_memory_usage_track_in_default_memory_pool = true; FLAGS_velox_memory_leak_check_enabled = true; memory::SharedArbitrator::registerFactory(); MemoryManagerOptions options; options.allocatorCapacity = 8L << 30; - if (FLAGS_velox_testing_enable_arbitration) { - options.arbitratorCapacity = 6L << 30; - options.arbitratorKind = "SHARED"; - options.checkUsageLeak = true; - options.arbitrationStateCheckCb = memoryArbitrationStateCheck; - } + options.arbitratorCapacity = 6L << 30; + options.arbitratorKind = "SHARED"; + options.checkUsageLeak = true; + options.arbitrationStateCheckCb = memoryArbitrationStateCheck; memory::MemoryManager::testingSetInstance(options); asyncDataCache_ = cache::AsyncDataCache::create(memory::memoryManager()->allocator()); diff --git a/velox/exec/tests/utils/OperatorTestBase.h b/velox/exec/tests/utils/OperatorTestBase.h index a10b316c173f8..54eb0d6b97eb4 100644 --- a/velox/exec/tests/utils/OperatorTestBase.h +++ b/velox/exec/tests/utils/OperatorTestBase.h @@ -28,8 +28,6 @@ #include "velox/vector/tests/utils/VectorMaker.h" #include "velox/vector/tests/utils/VectorTestBase.h" -DECLARE_bool(velox_testing_enable_arbitration); - namespace facebook::velox::exec::test { class OperatorTestBase : public testing::Test, public velox::test::VectorTestBase { diff --git a/velox/functions/lib/aggregates/tests/utils/AggregationTestBase.cpp b/velox/functions/lib/aggregates/tests/utils/AggregationTestBase.cpp index 49a5691b1b0f8..b0ef8ca0aa8c4 100644 --- a/velox/functions/lib/aggregates/tests/utils/AggregationTestBase.cpp +++ b/velox/functions/lib/aggregates/tests/utils/AggregationTestBase.cpp @@ -79,16 +79,6 @@ void AggregationTestBase::TearDown() { OperatorTestBase::TearDown(); } -void AggregationTestBase::SetUpTestCase() { - FLAGS_velox_testing_enable_arbitration = true; - OperatorTestBase::SetUpTestCase(); -} - -void AggregationTestBase::TearDownTestCase() { - FLAGS_velox_testing_enable_arbitration = false; - OperatorTestBase::TearDownTestCase(); -} - void AggregationTestBase::testAggregations( const std::vector& data, const std::vector& groupingKeys,