github-actions[bot] commented on code in PR #64899:
URL: https://github.com/apache/doris/pull/64899#discussion_r3485847430
##########
be/test/exec/pipeline/pipeline_task_test.cpp:
##########
@@ -662,6 +662,44 @@ TEST_F(PipelineTaskTest,
TEST_WAKE_UP_SUBMIT_PROTECTED_FROM_FINALIZE) {
EXPECT_TRUE(task->_operators.empty());
}
+TEST_F(PipelineTaskTest, TEST_FINALIZED_TASK_REJECTS_HYBRID_SUBMIT) {
+ auto num_instances = 1;
+ auto pip_id = 0;
+ auto task_id = 0;
+ auto pip = std::make_shared<Pipeline>(pip_id, num_instances,
num_instances);
+ OperatorPtr source_op;
+ source_op.reset(new DummyOperator());
+ EXPECT_TRUE(pip->add_operator(source_op, num_instances).ok());
+
+ int op_id = 1;
+ int node_id = 2;
+ int dest_id = 3;
+ DataSinkOperatorPtr sink_op;
+ sink_op.reset(new DummySinkOperatorX(op_id, node_id, dest_id));
+ EXPECT_TRUE(pip->set_sink(sink_op).ok());
+
+ auto profile = std::make_shared<RuntimeProfile>("Pipeline : " +
std::to_string(pip_id));
+ std::map<int,
+ std::pair<std::shared_ptr<BasicSharedState>,
std::vector<std::shared_ptr<Dependency>>>>
+ shared_state_map;
+ _runtime_state->resize_op_id_to_local_state(-1);
+ auto task = std::make_shared<PipelineTask>(pip, task_id,
_runtime_state.get(), _context,
+ profile.get(),
shared_state_map, task_id);
+ task->_exec_time_slice = 10'000'000'000ULL;
+
+ std::vector<TScanRangeParams> scan_range;
+ int sender_id = 0;
+ TDataSink tsink;
+ EXPECT_TRUE(task->prepare(scan_range, sender_id, tsink).ok());
+ EXPECT_TRUE(task->close(Status::OK()).ok());
Review Comment:
This test only submits after `finalize()` has already run, but `finalize()`
now also calls `_stop_accepting_submit()` before clearing `_sink` and
`_operators`. That means the test would still pass if the new `close()` gate
were removed, even though `close(close_sink=true)` is the place that now stops
`HybridTaskScheduler::submit()` before sink/operator close can race with
`is_blockable()`. Please add coverage for the close-time window, for example by
holding terminal `close()` in progress or submitting after
`close(Status::OK())` but before `finalize()`, and assert that the hybrid
submit path returns without evaluating `is_blockable()` or queueing the task.
--
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]
