Enable arbitrator in OperatorTestBase by default

tanjialiang · Mar 18, 2024 · a43e329 · a43e329
1 parent 9b8d95e
commit a43e329
Show file tree

Hide file tree

Showing 8 changed files with 4 additions and 60 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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::shared_ptr<exec::Task>, std::vector<RowVectorPtr>>
   executeSingleThreaded(
       core::PlanFragment plan,

diff --git 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
@@ -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
@@ -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<RowVectorPtr>& data,
     const std::vector<std::string>& groupingKeys,