From 302dabecca46a06d29334a5c8bd8edc95ff2228f Mon Sep 17 00:00:00 2001 From: yibin Date: Fri, 15 Dec 2023 12:40:57 +0800 Subject: [PATCH 1/3] Update tsan suppressions to add protobuf DebugString and ShortDebugString Signed-off-by: yibin --- tests/sanitize/tsan.suppression | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/sanitize/tsan.suppression b/tests/sanitize/tsan.suppression index 29ac8020569..2c5b8be7843 100644 --- a/tests/sanitize/tsan.suppression +++ b/tests/sanitize/tsan.suppression @@ -1,6 +1,7 @@ race:dbms/src/Common/TiFlashMetrics.h race:DB::Context::setCancelTest race:DB::getCurrentExceptionMessage -race:google::protobuf::internal::AssignDescriptors +race:google::protobuf::Message::DebugString +race:google::protobuf::Message::ShortDebugString race:fiu_fail race:dbms/src/DataStreams/BlockStreamProfileInfo.h From a09db2995a73360c0ea1f24980e144ff304ab9a3 Mon Sep 17 00:00:00 2001 From: yibin Date: Tue, 19 Dec 2023 15:42:35 +0800 Subject: [PATCH 2/3] Add mutex to protect executor creation Signed-off-by: yibin --- dbms/src/Flash/tests/gtest_compute_server.cpp | 27 +++++++++++-------- dbms/src/TestUtils/MPPTaskTestUtils.h | 1 + 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/dbms/src/Flash/tests/gtest_compute_server.cpp b/dbms/src/Flash/tests/gtest_compute_server.cpp index 0879bbef7fc..e1ea0ed4d97 100644 --- a/dbms/src/Flash/tests/gtest_compute_server.cpp +++ b/dbms/src/Flash/tests/gtest_compute_server.cpp @@ -147,11 +147,16 @@ class ComputeServerRunner : public DB::tests::MPPTaskTestUtils BlockInputStreamPtr stream; try { - auto tasks = prepareMPPTasks( - context.scan("test_db", "l_table") - .aggregation({Max(col("l_table.s"))}, {col("l_table.s")}) - .project({col("max(l_table.s)"), col("l_table.s")}), - properties); + QueryTasks tasks; + { + std::unique_lock lock(mu); + auto tmp_tasks = prepareMPPTasks( + context.scan("test_db", "l_table") + .aggregation({Max(col("l_table.s"))}, {col("l_table.s")}) + .project({col("max(l_table.s)"), col("l_table.s")}), + properties); + tasks = std::move(tmp_tasks); + } executeProblematicMPPTasks(tasks, properties, stream); } catch (...) @@ -985,7 +990,7 @@ try addOneQuery(i + 10, running_queries, gather_ids); } using namespace std::literals::chrono_literals; - std::this_thread::sleep_for(2s); + std::this_thread::sleep_for(4s); ASSERT_TRUE( TiFlashMetrics::instance() .tiflash_task_scheduler.get(tiflash_task_scheduler_metrics::type_active_queries_count, "") @@ -997,7 +1002,7 @@ try .Value() == 0); addOneQuery(1, running_queries, gather_ids); - std::this_thread::sleep_for(2s); + std::this_thread::sleep_for(4s); ASSERT_TRUE( TiFlashMetrics::instance() .tiflash_task_scheduler.get(tiflash_task_scheduler_metrics::type_active_queries_count, "") @@ -1020,7 +1025,7 @@ try addOneQuery((i + 1) * 20, running_queries, gather_ids); } using namespace std::literals::chrono_literals; - std::this_thread::sleep_for(2s); + std::this_thread::sleep_for(4s); ASSERT_TRUE( TiFlashMetrics::instance() .tiflash_task_scheduler.get(tiflash_task_scheduler_metrics::type_active_queries_count, "") @@ -1032,7 +1037,7 @@ try .Value() == 0); addOneQuery(30, running_queries, gather_ids); - std::this_thread::sleep_for(2s); + std::this_thread::sleep_for(4s); ASSERT_TRUE( TiFlashMetrics::instance() .tiflash_task_scheduler.get(tiflash_task_scheduler_metrics::type_active_queries_count, "") @@ -1046,7 +1051,7 @@ try /// cancel 1 running query MockComputeServerManager::instance().cancelGather(gather_ids[0]); running_queries[0].join(); - std::this_thread::sleep_for(2s); + std::this_thread::sleep_for(4s); ASSERT_TRUE( TiFlashMetrics::instance() .tiflash_task_scheduler.get(tiflash_task_scheduler_metrics::type_active_queries_count, "") @@ -1103,7 +1108,7 @@ try single_gather_properties.gather_id = 1; addOneGather(running_queries, gather_ids, single_gather_properties); using namespace std::literals::chrono_literals; - std::this_thread::sleep_for(2s); + std::this_thread::sleep_for(4s); /// 6 gathers, but two query ASSERT_TRUE( TiFlashMetrics::instance() diff --git a/dbms/src/TestUtils/MPPTaskTestUtils.h b/dbms/src/TestUtils/MPPTaskTestUtils.h index 7f5ae002ec3..3023cf24b85 100644 --- a/dbms/src/TestUtils/MPPTaskTestUtils.h +++ b/dbms/src/TestUtils/MPPTaskTestUtils.h @@ -104,6 +104,7 @@ class MPPTaskTestUtils : public ExecutorTest static LoggerPtr log_ptr; static size_t server_num; static MPPTestMeta test_meta; + std::mutex mu; }; #define ASSERT_MPPTASK_EQUAL(tasks, properties, expected_cols) \ From 2e41c0fea9eb1e70805604fa5dcb0702126ef1e5 Mon Sep 17 00:00:00 2001 From: yibin Date: Tue, 19 Dec 2023 16:04:34 +0800 Subject: [PATCH 3/3] Refact Signed-off-by: yibin --- dbms/src/Flash/tests/gtest_compute_server.cpp | 15 +++++---------- dbms/src/TestUtils/MPPTaskTestUtils.cpp | 1 + 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/dbms/src/Flash/tests/gtest_compute_server.cpp b/dbms/src/Flash/tests/gtest_compute_server.cpp index e1ea0ed4d97..24c5496ab50 100644 --- a/dbms/src/Flash/tests/gtest_compute_server.cpp +++ b/dbms/src/Flash/tests/gtest_compute_server.cpp @@ -147,16 +147,11 @@ class ComputeServerRunner : public DB::tests::MPPTaskTestUtils BlockInputStreamPtr stream; try { - QueryTasks tasks; - { - std::unique_lock lock(mu); - auto tmp_tasks = prepareMPPTasks( - context.scan("test_db", "l_table") - .aggregation({Max(col("l_table.s"))}, {col("l_table.s")}) - .project({col("max(l_table.s)"), col("l_table.s")}), - properties); - tasks = std::move(tmp_tasks); - } + auto tasks = prepareMPPTasks( + context.scan("test_db", "l_table") + .aggregation({Max(col("l_table.s"))}, {col("l_table.s")}) + .project({col("max(l_table.s)"), col("l_table.s")}), + properties); executeProblematicMPPTasks(tasks, properties, stream); } catch (...) diff --git a/dbms/src/TestUtils/MPPTaskTestUtils.cpp b/dbms/src/TestUtils/MPPTaskTestUtils.cpp index c2240c1abdf..adf51f5f5d7 100644 --- a/dbms/src/TestUtils/MPPTaskTestUtils.cpp +++ b/dbms/src/TestUtils/MPPTaskTestUtils.cpp @@ -108,6 +108,7 @@ BlockInputStreamPtr MPPTaskTestUtils::prepareMPPStreams(DAGRequestBuilder builde std::vector MPPTaskTestUtils::prepareMPPTasks(DAGRequestBuilder builder, const DAGProperties & properties) { + std::lock_guard lock(mu); auto tasks = builder.buildMPPTasks(context, properties); for (int i = test_meta.context_idx; i < TiFlashTestEnv::globalContextSize(); ++i) TiFlashTestEnv::getGlobalContext(i).setCancelTest();