This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 026516a70 [util] improve MaintenanceManagerTest.TestRunningInstances
026516a70 is described below
commit 026516a704d071b5b8e3cc1cd7f7d66a649a0b2d
Author: Alexey Serbin <[email protected]>
AuthorDate: Thu Jan 30 10:01:26 2025 -0800
[util] improve MaintenanceManagerTest.TestRunningInstances
This patch introduces a ScopedCleanup instance to unregister maintenance
operations registered by MaintenanceManagerTest.RunningInstances if
ASSERT_EVENTUALLY triggers. Also, the scenario is now more resilient to
scheduler anomalies. The motivation for this patch was a report
on test failures [1] with the following TSAN warning due to CHECK()
in MaintenanceOp::~MaintenanceOp():
WARNING: ThreadSanitizer: data race on vptr (ctor/dtor vs virtual call)
[1]
http://dist-test.cloudera.org/job?job_id=jenkins-slave.1738131961.3963207
Change-Id: I95f34e7cb8d37a0da4445012de40abfb2fe6f00e
Reviewed-on: http://gerrit.cloudera.org:8080/22427
Reviewed-by: Abhishek Chennaka <[email protected]>
Tested-by: Kudu Jenkins
---
src/kudu/util/maintenance_manager-test.cc | 59 +++++++++++++++++++++++--------
1 file changed, 44 insertions(+), 15 deletions(-)
diff --git a/src/kudu/util/maintenance_manager-test.cc
b/src/kudu/util/maintenance_manager-test.cc
index ca16374b3..a00a7d61a 100644
--- a/src/kudu/util/maintenance_manager-test.cc
+++ b/src/kudu/util/maintenance_manager-test.cc
@@ -18,6 +18,7 @@
#include "kudu/util/maintenance_manager.h"
#include <algorithm>
+#include <array>
#include <atomic>
#include <cmath>
#include <cstddef>
@@ -614,32 +615,60 @@ TEST_F(MaintenanceManagerTest,
TestPrioritizeLogRetentionUnderMemoryPressure) {
}
// Test retrieving a list of an op's running instances
-TEST_F(MaintenanceManagerTest, TestRunningInstances) {
+TEST_F(MaintenanceManagerTest, RunningInstances) {
TestMaintenanceOp op("op", MaintenanceOp::HIGH_IO_USAGE);
op.set_perf_improvement(10);
op.set_remaining_runs(2);
op.set_sleep_time(MonoDelta::FromSeconds(1));
+
manager_->RegisterOp(&op);
+ auto unregistrant = MakeScopedCleanup([&] {
+ manager_->UnregisterOp(&op);
+ });
// Check that running instances are added to the maintenance manager's
collection,
- // and fields are getting filled.
+ // and fields are populated.
+ MaintenanceManagerStatusPB status_pb;
+ std::array<const MaintenanceManagerStatusPB_OpInstancePB*, 2> instances{
nullptr, nullptr };
ASSERT_EVENTUALLY([&]() {
- MaintenanceManagerStatusPB status_pb;
- manager_->GetMaintenanceManagerStatusDump(&status_pb);
- ASSERT_EQ(status_pb.running_operations_size(), 2);
- const MaintenanceManagerStatusPB_OpInstancePB& instance1 =
status_pb.running_operations(0);
- const MaintenanceManagerStatusPB_OpInstancePB& instance2 =
status_pb.running_operations(1);
- ASSERT_EQ(instance1.name(), op.name());
- ASSERT_NE(instance1.thread_id(), instance2.thread_id());
- });
+ // Due to scheduler anomalies and other uncontrollable factors, the main
+ // thread might be put off CPU for some time, so at this point one or even
+ // two operations may be completed already.
+ manager_->GetMaintenanceManagerStatusDump(&status_pb);
+ const auto num_running = status_pb.running_operations_size();
+ ASSERT_GE(num_running, 0);
+ const auto num_completed = status_pb.completed_operations_size();
+ ASSERT_GE(num_completed, 0);
+
+ ASSERT_EQ(2, num_running + num_completed);
+ if (num_running == 2) {
+ instances[0] = &status_pb.running_operations(0);
+ instances[1] = &status_pb.running_operations(1);
+ } else if (num_completed == 2) {
+ instances[0] = &status_pb.completed_operations(0);
+ instances[1] = &status_pb.completed_operations(1);
+ } else {
+ instances[0] = &status_pb.running_operations(0);
+ instances[1] = &status_pb.completed_operations(0);
+ }
+ });
+ ASSERT_NE(nullptr, instances[0]);
+ ASSERT_NE(nullptr, instances[1]);
+ ASSERT_EQ(op.name(), instances[0]->name());
+ ASSERT_EQ(op.name(), instances[1]->name());
+ ASSERT_NE(instances[0]->thread_id(), instances[1]->thread_id());
- // Wait for instances to complete.
- manager_->UnregisterOp(&op);
+ // When unregistering operations, the maintenance manager waits for them
+ // to complete.
+ unregistrant.run();
// Check that running instances are removed from collection after completion.
- MaintenanceManagerStatusPB status_pb;
- manager_->GetMaintenanceManagerStatusDump(&status_pb);
- ASSERT_EQ(status_pb.running_operations_size(), 0);
+ {
+ MaintenanceManagerStatusPB status_pb;
+ manager_->GetMaintenanceManagerStatusDump(&status_pb);
+ ASSERT_EQ(0, status_pb.running_operations_size());
+ ASSERT_EQ(2, status_pb.completed_operations_size());
+ }
}
// Test adding operations and make sure that the history of recently completed
