github-actions[bot] commented on code in PR #67845:
URL: https://github.com/apache/doris/pull/67845#discussion_r4228261202
##########
be/src/runtime/workload_group/workload_group_manager.cpp:
##########
@@ -300,6 +302,19 @@ void WorkloadGroupMgr::add_paused_query(const
std::shared_ptr<ResourceContext>&
LOG(INFO) << "Insert one new paused query: "
<< resource_ctx->task_controller()->debug_string()
<< ", workload group: " << wg->debug_string();
+ } else {
+ // Another task of the same query already failed a reservation. The
query is resumed as
+ // a whole and every blocked task then retries its own request, so
record the sum of the
+ // pending requests: the query is woken up only once all of them fit
at the same time.
+ // Waking it up as soon as one of them fits would fail the others
again and start a new
+ // wait from scratch.
+ LOG(INFO) << "Query: " <<
print_id(resource_ctx->task_controller()->task_id())
+ << " is already paused, add the pending reservation "
+ << PrettyPrinter::print_bytes(reserve_size) << " to the
recorded "
+ << PrettyPrinter::print_bytes(it->reserve_size_);
+ auto node = queries_list.extract(it);
+ node.value().reserve_size_ += reserve_size;
Review Comment:
[P2] Allow sequential progress with partial reservation headroom. Adding all
sibling requests requires simultaneous free memory even though pipeline tasks
reserve independently; one task can finish and release its temporary memory
before another runs. With 64 MiB and 4 MiB pending, stable 65 MiB headroom can
let both run in sequence, but this 68 MiB total keeps the whole query paused
until timeout and cancels it. Preserve the original deadline across retries or
admit tasks separately so feasible work can proceed within the bounded wait.
##########
be/test/testutil/mock/mock_query_task_controller.h:
##########
@@ -35,6 +38,52 @@ struct MockQueryTaskController : public QueryTaskController {
}
void set_cancelled_time(int64_t ctime) { cancelled_time_ = ctime; }
+
+ // Memory reclamation asks every candidate query whether it is cancelled
while it scans a
+ // workload group, so this is where a test can let another query release
memory during that
+ // scan.
+ bool is_cancelled() const override {
+ if (on_is_cancelled_) {
+ on_is_cancelled_();
+ }
+ return QueryTaskController::is_cancelled();
+ }
+
+ // Pipeline state seen by WorkloadGroupMgr::handle_single_query_. The
query context of a
+ // unit test has no fragments, so the real implementation never reports a
running or a
+ // revocable task; these knobs simulate them.
+ // NOLINTNEXTLINE(readability-make-member-function-const): overrides a
non-const virtual.
+ void get_revocable_info(size_t* revocable_size, size_t* memory_usage,
+ bool* has_running_task) override {
+ QueryTaskController::get_revocable_info(revocable_size, memory_usage,
has_running_task);
+ *has_running_task = has_running_task_;
+ }
+
+ // The manager only checks whether the list is empty before it calls
revoke_memory(), which
+ // is mocked below, so a placeholder entry is enough to stand for a
revocable task.
+ std::vector<PipelineTask*> get_revocable_tasks() override {
+ if (on_get_revocable_tasks_) {
+ on_get_revocable_tasks_();
+ }
+ return has_revocable_task_ ? std::vector<PipelineTask*> {nullptr}
+ : std::vector<PipelineTask*> {};
+ }
+
+ Status revoke_memory() override {
Review Comment:
[P2] Exercise the spill callback and real retry in this test. This mock
returns OK without calling `set_memory_sufficient(true)`, so the manager
removes the paused-list entry while the query's memory dependency remains
blocked. The hard-limit test then calls `add_paused_query` directly; it passes
without exercising the claimed resume, failed reservation, and second pause.
Have the mock complete the spill/resume and assert that transition before
testing the retry.
--
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]
