Repository: kudu Updated Branches: refs/heads/master 000791109 -> 93be1310d
maintenance_manager: simplify test This simplifies maintenance_manager-test a bit to avoid a condition variable, and instead just poll from the main thread. This reduces the complexity to understand the test a bit, and the test still runs quickly (<100ms). This also changes the test operation in preparation for a later patch which will allow an operation to run multiple times in parallel and auto-disable itself after a prescribed number of runs. Change-Id: I265318f64771803714f48a6e96fd3260b6eafae4 Reviewed-on: http://gerrit.cloudera.org:8080/4294 Reviewed-by: Adar Dembo <[email protected]> Tested-by: Kudu Jenkins Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/d5a1f2d4 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/d5a1f2d4 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/d5a1f2d4 Branch: refs/heads/master Commit: d5a1f2d42b48bb5e852de3a57221ad25e178b169 Parents: 0007911 Author: Todd Lipcon <[email protected]> Authored: Thu Sep 1 16:57:19 2016 -0700 Committer: Todd Lipcon <[email protected]> Committed: Fri Sep 2 01:16:59 2016 +0000 ---------------------------------------------------------------------- src/kudu/util/maintenance_manager-test.cc | 98 ++++++++++---------------- 1 file changed, 38 insertions(+), 60 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/d5a1f2d4/src/kudu/util/maintenance_manager-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/maintenance_manager-test.cc b/src/kudu/util/maintenance_manager-test.cc index 87023dc..60a9d51 100644 --- a/src/kudu/util/maintenance_manager-test.cc +++ b/src/kudu/util/maintenance_manager-test.cc @@ -73,39 +73,28 @@ class MaintenanceManagerTest : public KuduTest { TEST_F(MaintenanceManagerTest, TestCreateAndShutdown) { } -enum TestMaintenanceOpState { - OP_DISABLED, - OP_RUNNABLE, - OP_RUNNING, - OP_FINISHED, -}; - class TestMaintenanceOp : public MaintenanceOp { public: TestMaintenanceOp(const std::string& name, IOUsage io_usage, - TestMaintenanceOpState state, const shared_ptr<MemTracker>& tracker) : MaintenanceOp(name, io_usage), - state_change_cond_(&lock_), - state_(state), consumption_(tracker, 500), logs_retained_bytes_(0), perf_improvement_(0), metric_entity_(METRIC_ENTITY_test.Instantiate(&metric_registry_, "test")), maintenance_op_duration_(METRIC_maintenance_op_duration.Instantiate(metric_entity_)), - maintenance_ops_running_(METRIC_maintenance_ops_running.Instantiate(metric_entity_, 0)) { + maintenance_ops_running_(METRIC_maintenance_ops_running.Instantiate(metric_entity_, 0)), + remaining_runs_(1) { } virtual ~TestMaintenanceOp() {} virtual bool Prepare() OVERRIDE { std::lock_guard<Mutex> guard(lock_); - if (state_ != OP_RUNNABLE) { + if (remaining_runs_ == 0) { return false; } - state_ = OP_RUNNING; - state_change_cond_.Broadcast(); DLOG(INFO) << "Prepared op " << name(); return true; } @@ -113,47 +102,21 @@ class TestMaintenanceOp : public MaintenanceOp { virtual void Perform() OVERRIDE { DLOG(INFO) << "Performing op " << name(); std::lock_guard<Mutex> guard(lock_); - CHECK_EQ(OP_RUNNING, state_); - state_ = OP_FINISHED; - state_change_cond_.Broadcast(); + CHECK_GE(remaining_runs_, 1); + remaining_runs_--; } virtual void UpdateStats(MaintenanceOpStats* stats) OVERRIDE { std::lock_guard<Mutex> guard(lock_); - stats->set_runnable(state_ == OP_RUNNABLE); + stats->set_runnable(remaining_runs_ > 0); stats->set_ram_anchored(consumption_.consumption()); stats->set_logs_retained_bytes(logs_retained_bytes_); stats->set_perf_improvement(perf_improvement_); } - void Enable() { + void set_remaining_runs(int runs) { std::lock_guard<Mutex> guard(lock_); - DCHECK((state_ == OP_DISABLED) || (state_ == OP_FINISHED)); - state_ = OP_RUNNABLE; - state_change_cond_.Broadcast(); - } - - void WaitForState(TestMaintenanceOpState state) { - std::lock_guard<Mutex> guard(lock_); - while (true) { - if (state_ == state) { - return; - } - state_change_cond_.Wait(); - } - } - - bool WaitForStateWithTimeout(TestMaintenanceOpState state, int ms) { - MonoDelta to_wait = MonoDelta::FromMilliseconds(ms); - std::lock_guard<Mutex> guard(lock_); - while (true) { - if (state_ == state) { - return true; - } - if (!state_change_cond_.TimedWait(to_wait)) { - return false; - } - } + remaining_runs_ = runs; } void set_ram_anchored(uint64_t ram_anchored) { @@ -181,8 +144,7 @@ class TestMaintenanceOp : public MaintenanceOp { private: Mutex lock_; - ConditionVariable state_change_cond_; - enum TestMaintenanceOpState state_; + ScopedTrackedConsumption consumption_; uint64_t logs_retained_bytes_; uint64_t perf_improvement_; @@ -190,19 +152,29 @@ class TestMaintenanceOp : public MaintenanceOp { scoped_refptr<MetricEntity> metric_entity_; scoped_refptr<Histogram> maintenance_op_duration_; scoped_refptr<AtomicGauge<uint32_t> > maintenance_ops_running_; + + // The number of remaining times this operation will run before disabling + // itself. + int remaining_runs_; }; // Create an op and wait for it to start running. Unregister it while it is // running and verify that UnregisterOp waits for it to finish before // proceeding. TEST_F(MaintenanceManagerTest, TestRegisterUnregister) { - TestMaintenanceOp op1("1", MaintenanceOp::HIGH_IO_USAGE, OP_DISABLED, test_tracker_); + TestMaintenanceOp op1("1", MaintenanceOp::HIGH_IO_USAGE, test_tracker_); op1.set_ram_anchored(1001); + // Register initially with no remaining runs. We'll later enable it once it's + // already registered. + op1.set_remaining_runs(0); manager_->RegisterOp(&op1); scoped_refptr<kudu::Thread> thread; - CHECK_OK(Thread::Create("TestThread", "TestRegisterUnregister", - boost::bind(&TestMaintenanceOp::Enable, &op1), &thread)); - op1.WaitForState(OP_FINISHED); + CHECK_OK(Thread::Create( + "TestThread", "TestRegisterUnregister", + boost::bind(&TestMaintenanceOp::set_remaining_runs, &op1, 1), &thread)); + AssertEventually([&]() { + ASSERT_EQ(op1.DurationHistogram()->TotalCount(), 1); + }); manager_->UnregisterOp(&op1); ThreadJoiner(thread.get()).Join(); } @@ -210,18 +182,22 @@ TEST_F(MaintenanceManagerTest, TestRegisterUnregister) { // Test that we'll run an operation that doesn't improve performance when memory // pressure gets high. TEST_F(MaintenanceManagerTest, TestMemoryPressure) { - TestMaintenanceOp op("op", MaintenanceOp::HIGH_IO_USAGE, OP_RUNNABLE, test_tracker_); + TestMaintenanceOp op("op", MaintenanceOp::HIGH_IO_USAGE, test_tracker_); op.set_ram_anchored(100); manager_->RegisterOp(&op); // At first, we don't want to run this, since there is no perf_improvement. - CHECK_EQ(false, op.WaitForStateWithTimeout(OP_FINISHED, 20)); + SleepFor(MonoDelta::FromMilliseconds(20)); + ASSERT_EQ(0, op.DurationHistogram()->TotalCount()); // set the ram_anchored by the high mem op so high that we'll have to run it. scoped_refptr<kudu::Thread> thread; - CHECK_OK(Thread::Create("TestThread", "MaintenanceManagerTest", + ASSERT_OK(Thread::Create("TestThread", "MaintenanceManagerTest", boost::bind(&TestMaintenanceOp::set_ram_anchored, &op, 1100), &thread)); - op.WaitForState(OP_FINISHED); + + AssertEventually([&]() { + ASSERT_EQ(op.DurationHistogram()->TotalCount(), 1); + }); manager_->UnregisterOp(&op); ThreadJoiner(thread.get()).Join(); } @@ -230,15 +206,15 @@ TEST_F(MaintenanceManagerTest, TestMemoryPressure) { TEST_F(MaintenanceManagerTest, TestLogRetentionPrioritization) { manager_->Shutdown(); - TestMaintenanceOp op1("op1", MaintenanceOp::LOW_IO_USAGE, OP_RUNNABLE, test_tracker_); + TestMaintenanceOp op1("op1", MaintenanceOp::LOW_IO_USAGE, test_tracker_); op1.set_ram_anchored(0); op1.set_logs_retained_bytes(100); - TestMaintenanceOp op2("op2", MaintenanceOp::HIGH_IO_USAGE, OP_RUNNABLE, test_tracker_); + TestMaintenanceOp op2("op2", MaintenanceOp::HIGH_IO_USAGE, test_tracker_); op2.set_ram_anchored(100); op2.set_logs_retained_bytes(100); - TestMaintenanceOp op3("op3", MaintenanceOp::HIGH_IO_USAGE, OP_RUNNABLE, test_tracker_); + TestMaintenanceOp op3("op3", MaintenanceOp::HIGH_IO_USAGE, test_tracker_); op3.set_ram_anchored(200); op3.set_logs_retained_bytes(100); @@ -266,12 +242,14 @@ TEST_F(MaintenanceManagerTest, TestLogRetentionPrioritization) { TEST_F(MaintenanceManagerTest, TestCompletedOpsHistory) { for (int i = 0; i < 5; i++) { string name = Substitute("op$0", i); - TestMaintenanceOp op(name, MaintenanceOp::HIGH_IO_USAGE, OP_RUNNABLE, test_tracker_); + TestMaintenanceOp op(name, MaintenanceOp::HIGH_IO_USAGE, test_tracker_); op.set_perf_improvement(1); op.set_ram_anchored(100); manager_->RegisterOp(&op); - CHECK_EQ(true, op.WaitForStateWithTimeout(OP_FINISHED, 200)); + AssertEventually([&]() { + ASSERT_EQ(op.DurationHistogram()->TotalCount(), 1); + }); manager_->UnregisterOp(&op); MaintenanceManagerStatusPB status_pb;
