mrhhsg commented on code in PR #67845:
URL: https://github.com/apache/doris/pull/67845#discussion_r4227669929
##########
be/src/runtime/workload_group/workload_group_manager.cpp:
##########
@@ -678,32 +702,50 @@ int64_t
WorkloadGroupMgr::revoke_memory_from_other_groups_() {
// then not revoke memory from it.
continue;
}
- if (total_used_memory - min_memory_limit > max_exceeded_memory) {
- max_wg = workload_group.second;
- max_exceeded_memory = total_used_memory - min_memory_limit;
- }
+ exceeded_wgs.emplace_back(total_used_memory - min_memory_limit,
workload_group.second);
}
}
- if (max_wg == nullptr) {
- return 0;
- }
- if (max_exceeded_memory < 1 << 27) {
- LOG(INFO) << "The workload group that exceed most memory is :"
- << max_wg->memory_debug_string() << ", max_exceeded_memory: "
- << PrettyPrinter::print(max_exceeded_memory, TUnit::BYTES)
- << " less than 128MB, no need to revoke memory";
- return 0;
- }
- int64_t freed_mem = static_cast<int64_t>((double)max_exceeded_memory *
0.1);
- // Revoke 10% of memory from the workload group that exceed most memory
- max_wg->revoke_memory(freed_mem, "exceed_memory", profile.get());
- std::stringstream ss;
- profile->pretty_print(&ss);
- LOG(INFO) << fmt::format(
- "[MemoryGC] process memory not enough, revoke memory from
workload_group: {}, "
- "free memory {}. cost(us): {}, details: {}",
- max_wg->memory_debug_string(),
PrettyPrinter::print_bytes(freed_mem),
- watch.elapsed_time() / 1000, ss.str());
+ std::sort(exceeded_wgs.begin(), exceeded_wgs.end(),
+ [](const auto& lhs, const auto& rhs) { return lhs.first >
rhs.first; });
+
+ int64_t freed_mem = 0;
+ size_t tried_wgs = 0;
+ for (const auto& [exceeded_memory, wg] : exceeded_wgs) {
+ if (exceeded_memory < 1 << 27) {
+ // The remaining workload groups exceed even less.
+ LOG(INFO) << "The workload group that exceed most memory among the
untried ones is :"
+ << wg->memory_debug_string() << ", exceeded_memory: "
+ << PrettyPrinter::print(exceeded_memory, TUnit::BYTES)
+ << " less than 128MB, no need to revoke memory";
+ break;
+ }
Review Comment:
Fixed in a428b3e787d. The walk now recomputes each peer's excess from its
current usage (`total_mem_used() - min_memory_limit()`) right before calling
`revoke_memory()` on it, and skips the group with `continue` when that excess
is below 128 MiB (the snapshot order is still used for the early `break`); the
10% target is also derived from the current excess. Added
`process_mem_exceeded_below_min_memory_skips_peer_within_min`: the first peer
holds only a query whose cancellation exceeded `revoke_memory_max_tolerance_ms`
(frees nothing), and while it is scanned the second peer's query releases 180
MiB, dropping the group from 250 MiB to 70 MiB with a 100 MiB minimum. The
remaining 70 MiB query is no longer cancelled and the paused query falls
through to the hard-limit fallback; without the source change the case fails on
`second_peer_query->is_cancelled()`.
##########
be/src/runtime/workload_group/workload_group_manager.cpp:
##########
@@ -602,14 +603,33 @@ bool WorkloadGroupMgr::handle_process_memory_exceeded_(
return false;
}
+ // The process memory pressure may have been relieved by cache reclamation
or by other
+ // queries that finished. Check it before routing the query below,
otherwise a query in a
+ // workload group that uses less than its min memory limit has to wait for
the timeout.
+ // Test the recorded reservation itself: this is the predicate the failed
reservation used,
+ // so the query is resumed exactly when its request fits now.
+ if
(!GlobalMemoryArbitrator::is_exceed_soft_mem_limit(query_it->reserve_size_)) {
+ LOG(INFO) << "Query: " <<
print_id(resource_ctx->task_controller()->task_id())
Review Comment:
Fixed in a428b3e787d. `add_paused_query()` now keeps the largest reservation
recorded while the query is paused: when the query already has an entry and the
new `reserve_size` is larger, the set node is extracted, its `reserve_size_`
raised and reinserted (same key, `enqueue_at` unchanged). The query is resumed
as a whole and every blocked task retries its own request, and
`is_exceed_soft_mem_limit(bytes)` is monotonic in `bytes`, so the query is only
woken once the largest pending request fits; a smaller sibling can no longer
wake it and reset the timer. Added
`process_mem_exceeded_keeps_largest_pending_reservation` (1 KiB, then 64 MiB,
then 4 MiB recorded for one query: 16 MiB of headroom keeps it paused for three
rounds, 65 MiB resumes it) and
`process_mem_exceeded_largest_pending_reservation_cancels_after_timeout` (same
two sizes with 16 MiB headroom past the timeout: cancelled instead of being
woken by the 1 KiB request). Both fail without the source change.
--
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]
