github-actions[bot] commented on code in PR #65713:
URL: https://github.com/apache/doris/pull/65713#discussion_r3600896651
##########
be/src/runtime/workload_group/workload_group_manager.cpp:
##########
@@ -865,11 +865,13 @@ void
WorkloadGroupMgr::update_queries_limit_(WorkloadGroupPtr wg, bool enable_ha
// If the query is a pure load task, then should not modify its limit.
Or it will reserve
// memory failed and we did not hanle it.
if (!resource_ctx->task_controller()->is_pure_load_task()) {
- // If user's set mem limit is less than query weighted mem limit,
then should not modify its limit.
- // Use user settings.
- if (resource_ctx->memory_context()->user_set_mem_limit() >
query_weighted_mem_limit) {
-
resource_ctx->memory_context()->set_mem_limit(query_weighted_mem_limit);
- }
+ // The effective limit should be min(user_set_mem_limit,
query_weighted_mem_limit).
+ // This ensures limits are both lowered under memory pressure and
restored when
+ // pressure eases (e.g., when concurrent queries finish or WG
memory drops below
+ // low watermark).
+ int64_t effective_limit =
std::min(resource_ctx->memory_context()->user_set_mem_limit(),
Review Comment:
[P1] Preserve the requested limit across process expansion
`user_set_mem_limit()` is not always the user's requested limit: query
initialization replaces an absent limit or any request above the current
process limit with the current `MemInfo::mem_limit()`, then snapshots that cap
as the user limit. If a cgroup-derived process limit grows from 8 GiB to 32
GiB, a query that requested 16 GiB (or no limit) therefore still supplies 8 GiB
to this `min`, so it remains stuck at the old process-memory limit under NONE,
FIXED, or DYNAMIC even though the refreshed workload-group limit is larger.
This is distinct from the existing restoration thread, where the desired `U`
survives in `user_set_mem_limit()`. Please preserve the original
request/unlimited sentinel for resize recomputation and add an
above-old-process expansion regression.
##########
be/src/runtime/workload_group/workload_group_manager.cpp:
##########
@@ -865,11 +865,13 @@ void
WorkloadGroupMgr::update_queries_limit_(WorkloadGroupPtr wg, bool enable_ha
// If the query is a pure load task, then should not modify its limit.
Or it will reserve
// memory failed and we did not hanle it.
if (!resource_ctx->task_controller()->is_pure_load_task()) {
- // If user's set mem limit is less than query weighted mem limit,
then should not modify its limit.
- // Use user settings.
- if (resource_ctx->memory_context()->user_set_mem_limit() >
query_weighted_mem_limit) {
-
resource_ctx->memory_context()->set_mem_limit(query_weighted_mem_limit);
- }
+ // The effective limit should be min(user_set_mem_limit,
query_weighted_mem_limit).
+ // This ensures limits are both lowered under memory pressure and
restored when
+ // pressure eases (e.g., when concurrent queries finish or WG
memory drops below
+ // low watermark).
+ int64_t effective_limit =
std::min(resource_ctx->memory_context()->user_set_mem_limit(),
+ query_weighted_mem_limit);
+ resource_ctx->memory_context()->set_mem_limit(effective_limit);
Review Comment:
[P1] Keep the paused-query hard clamp until retry
This unconditional store also runs during the normal maintenance pass. For a
DYNAMIC group below its low watermark, `handle_paused_queries()` can lower a
requestor to its per-slot `adjusted_mem_limit()` and resume it specifically so
the retry fails as `QUERY_MEMORY_EXCEEDED` and reaches spill handling. If that
task has not retried before the next maintenance iteration, the normal pass
selects the much larger group-high limit and this store raises the tracker
again; the reservation can then pass the query check and fail at the
workload-group check again, repeating the same pause instead of spilling. The
previous conditional preserved this clamp when the user limit was at or below
the normal group-high limit. Please retain explicit hard-limit state until the
resumed retry consumes or clears it, and add a DYNAMIC regression with a
maintenance refresh between resume and 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]
