github-actions[bot] commented on code in PR #65620:
URL: https://github.com/apache/doris/pull/65620#discussion_r3584246987
##########
regression-test/suites/fault_injection_p0/partial_update/test_partial_update_skip_compaction.groovy:
##########
@@ -45,6 +47,7 @@ suite("test_partial_update_skip_compaction", "nonConcurrent")
{
def tabletStat = sql_return_maparray("show tablets from ${table1};").get(0)
def tabletBackendId = tabletStat.BackendId
def tabletId = tabletStat.TabletId
+ def partitionId = tabletStat.PartitionId
Review Comment:
`SHOW TABLETS FROM` does not return a `PartitionId` column.
`sql_return_maparray` only creates keys from the JDBC labels, and
`ShowTabletsFromTableCommand` builds those labels from
`TabletsProcDir.TITLE_NAMES`, which has
`TabletId`/`BackendId`/`MetaUrl`/`CompactionStatus` but no `PartitionId`. That
means this value becomes `null`, so the call below sends `partition_id=null`;
the BE then runs `dp->param<int64_t>("partition_id", -1)` and throws before it
can enter the intended publish wait. Please fetch the partition id from a
source that exposes it, such as `SHOW PARTITIONS` for this single-partition
table, and assert it is present/numeric before enabling the debug point.
##########
be/src/storage/task/engine_publish_version_task.cpp:
##########
@@ -116,17 +116,29 @@ Status EnginePublishVersionTask::execute() {
}
});
DBUG_EXECUTE_IF("EnginePublishVersionTask::execute.enable_spin_wait", {
- auto token = dp->param<std::string>("token", "invalid_token");
- while
(DebugPoints::instance()->is_enable("EnginePublishVersionTask::execute.block"))
{
- auto block_dp = DebugPoints::instance()->get_debug_point(
- "EnginePublishVersionTask::execute.block");
- if (block_dp) {
- auto pass_token = block_dp->param<std::string>("pass_token",
"");
- if (pass_token == token) {
- break;
+ // Scope the fault injection to one partition when requested so
unrelated publish tasks
+ // are not blocked by the same process-wide debug point.
+ auto target_partition_id = dp->param<int64_t>("partition_id", -1);
Review Comment:
This filter runs after `DBUG_EXECUTE_IF` has already called
`DebugPoints::get_debug_point()`, and that lookup consumes the generic
`execute` counter before any code in this block runs. With a scoped point like
`partition_id=<target>&execute=1`, an unrelated publish task on the same BE can
hit this block first, fail the partition match, and spend the only execute
slot. When the target partition arrives later, `get_debug_point()`
removes/returns null before this block runs, so the intended publish never
blocks. Please either perform the partition match before consuming `execute`,
or explicitly reject/document combining `execute` with this new `partition_id`
filter so tests do not silently skip the intended injection.
--
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]
