github-actions[bot] commented on code in PR #65542:
URL: https://github.com/apache/doris/pull/65542#discussion_r3575438238
##########
be/src/runtime/workload_group/workload_group.cpp:
##########
@@ -162,7 +162,9 @@ void WorkloadGroup::check_and_update(const
WorkloadGroupInfo& wg_info) {
}
{
std::lock_guard<std::shared_mutex> wl {_mutex};
- if (wg_info.version > _version) {
+ // In serverless mode, user may modify cgroup's memory limit directly
and workload group's config
+ // is not changed. So that we should update workload group's config
ignore version.
+ if (wg_info.version > _version || _memory_limit !=
wg_info.memory_limit) {
Review Comment:
This still does not update the workload group when only the parsed memory
limit changes. `check_and_update()` returns under the shared lock for every
`wg_info.version <= _version`, so the same-version serverless case described by
the new comment exits at line 160 and never reaches this `_memory_limit !=
wg_info.memory_limit` comparison. The stored `_memory_limit` and derived
`_min_memory_limit` then remain stale, and the reserve/watermark/query-limit
paths continue enforcing the old cgroup-derived limit. Please include the
memory-limit comparison in the fast-path predicate, or otherwise allow
equal-version updates to acquire the write lock and refresh those fields; a
same-version `check_and_update()`/`get_or_create_workload_group()` unit test
would cover this.
--
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]
