github-actions[bot] commented on code in PR #65620:
URL: https://github.com/apache/doris/pull/65620#discussion_r3595900024
##########
be/test/util/debug_points_test.cpp:
##########
@@ -96,6 +97,61 @@ TEST(DebugPointsTest, AddTest) {
DebugPoints::instance()->get_debug_param_or_default<std::string>("dbug4", ""));
}
+TEST(DebugPointsTest, PredicateDoesNotConsumeExecuteLimit) {
+ config::enable_debug_points = true;
+ DebugPoints::instance()->clear();
+
+ auto debug_point = std::make_shared<DebugPoint>();
+ debug_point->execute_limit = 1;
+ debug_point->params["partition_id"] = "123";
+ DebugPoints::instance()->add("conditional", debug_point);
+
+ auto does_not_match = [](const DebugPoint& point) {
+ return point.param<int64_t>("partition_id") == 456;
+ };
+ EXPECT_EQ(nullptr,
DebugPoints::instance()->get_debug_point_if("conditional", does_not_match));
+ EXPECT_EQ(0, debug_point->execute_num.load());
+
+ auto matches = [](const DebugPoint& point) {
+ return point.param<int64_t>("partition_id") == 123;
+ };
+ EXPECT_NE(nullptr,
DebugPoints::instance()->get_debug_point_if("conditional", matches));
+ EXPECT_EQ(1, debug_point->execute_num.load());
+ EXPECT_EQ(nullptr,
DebugPoints::instance()->get_debug_point_if("conditional", matches));
+ EXPECT_EQ(2, debug_point->execute_num.load());
+ EXPECT_FALSE(DebugPoints::instance()->is_enable("conditional"));
+}
+
+TEST(DebugPointsTest, ConcurrentReplacementSurvivesExhaustedLookup) {
+ config::enable_debug_points = true;
+ DebugPoints::instance()->clear();
+
+ auto exhausted_point = std::make_shared<DebugPoint>();
+ exhausted_point->execute_limit = 1;
+ exhausted_point->execute_num.store(1);
+ DebugPoints::instance()->add("conditional", exhausted_point);
+
+ std::barrier sync_point(2);
+ std::shared_ptr<DebugPoint> lookup_result;
+ std::thread lookup_thread([&] {
+ lookup_result = DebugPoints::instance()->get_debug_point_if(
Review Comment:
The required clang-format 16 gate is currently failing on this new
statement. Running `clang-format-16` wraps the assignment as `lookup_result =`
followed by the `DebugPoints::instance()->get_debug_point_if(...)` call; please
apply the repository formatter to this file so the current-head formatter jobs
can pass.
##########
regression-test/suites/fault_injection_p0/partial_update/test_partial_update_skip_compaction.groovy:
##########
@@ -167,6 +178,17 @@ suite("test_partial_update_skip_compaction",
"nonConcurrent") {
// let the partial update load publish
disable_block_in_publish()
t1.join()
+ disable_publish_spin_wait()
Review Comment:
This ordering protects the outer `enable_spin_wait` lookup, but Cloud mode
still releases the inner loop by deleting `...block` before the load thread
joins. `CloudGlobalTransactionMgr` checks `isEnable(block)`, performs a
separate `getDebugPoint(block)`, and immediately calls
`blockDebugPoint.param(...)`; if this removal lands between those reads, the
insert fails with a `NullPointerException`. This is distinct from the prior
outer-point race. Please release the Cloud loop without deleting the point it
is concurrently reading (for example, replace it with a non-null point carrying
the matching `pass_token`, then remove both points after `join`), or make the
FE lookup atomic/null-safe.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
