Doris-Breakwater commented on issue #70575: URL: https://github.com/apache/doris/issues/70575#issuecomment-6856936268
Breakwater-GitHub-Analysis-Slot: slot_7eb8aa7f51fc The reported failure is supported by the source: both Docker entrypoints can create a v2 hierarchy without making the CPU controller available, while BE startup continues after CPU-control initialization fails. This warrants a Docker/BE workload-group initialization fix. The issue is currently open with no labels; Docker, cgroup v2, and workload-group CPU control are the relevant areas for triage. I checked the local `4.1.4` tag (`4accb51bc1f27d84980668f3a2eaf0ac96fb3cba`) and fetched the files at the reported master commit `e82d2650e38e57886679faecf0ad5569169c54aa`. This is source verification; I have not independently reproduced the container tests or tested the proposed patch. The verified chain is: - Both entrypoints fix `WORKLOAD_GROUP_PATH` to `/sys/fs/cgroup/cpu/doris`, create it before checking the cgroup version, and append that path to `be.conf`. The v2 branch only changes permissions on the mount root's `cgroup.procs`; it does not enable CPU at the intervening ancestors. See the pinned [BE entrypoint](https://github.com/apache/doris/blob/e82d2650e38e57886679faecf0ad5569169c54aa/docker/runtime/be/resource/be_entrypoint.sh#L462-L479) and [disaggregated entrypoint](https://github.com/apache/doris/blob/e82d2650e38e57886679faecf0ad5569169c54aa/docker/runtime/be/resource/be_disaggregated_entrypoint.sh#L525-L542). - `is_a_valid_cgroup_path()` checks v2 core files, so a directory without CPU controller files can pass validation. `init_doris_cgroup_path()` then attempts `+cpu` at `doris` and `doris/query`, followed by moving the BE process into `query`. If CPU is unavailable at `doris`, initialization stops at the first write. The failure becomes an INFO summary, without invalidating the previously accepted query path or propagating a startup error. See [initialization](https://github.com/apache/doris/blob/e82d2650e38e57886679faecf0ad5569169c54aa/be/src/agent/cgroup_cpu_ctl.cpp#L32-L162). - Workload-group initialization subsequently requires `query/<wg-id>/cpu.max` and `cpu.weight`. Missing files cause initialization to fail; `WorkloadGroup::create_cgroup_cpu_ctl_no_lock()` logs that failure at INFO and leaves the controller unset, so CPU-limit updates are skipped while thread-pool setup can proceed. See [v2 controller initialization](https://github.com/apache/doris/blob/e82d2650e38e57886679faecf0ad5569169c54aa/be/src/agent/cgroup_cpu_ctl.cpp#L372-L440) and [workload-group handling in 4.1.4](https://github.com/apache/doris/blob/4accb51bc1f27d84980668f3a2eaf0ac96fb3cba/be/src/runtime/workload_group/workload_group.cpp#L531-L550). One correction to the logging description: `write_cg_sys_file()` already emits **ERROR** when `open()` or `write()` fails, including the failed path. The summary is INFO, and neither failure is fatal. Thus the source supports continued startup without working CPU isolation, but does not support an unconditional claim that the failed `+cpu` write produces no ERROR. Please include the surrounding startup logs, including `set cpu controller`, `write sys file failed`, and `cgroup init failed`. Improving the summary to identify disabled CPU isolation, with the operation, path, and errno, would still help. See the [write helper](https://github.com/apache/doris/blob/e82d2650e38e57886679faecf0ad5569169c54aa/be/src/agent/cgroup_cpu_ctl.cpp#L219-L243). The proposed container-local threaded subtree is a reasonable direction. CPU availability must propagate from the parent, and CPU supports threaded subtrees; these are consistent with the [kernel's controller rules](https://docs.kernel.org/admin-guide/cgroup-v2.html#controlling-controllers) and [threaded-mode rules](https://docs.kernel.org/admin-guide/cgroup-v2.html#threads). Review should cover resolving `0::` membership against the actual cgroup2 mount/root in `/proc/self/mountinfo`, keeping the subtree beneath the container's existing limit, and checking whether existing controller/topology state permits threaded mode. Pre-creating both `doris` and `query` with the appropriate type matters because BE enables controllers and moves its process before creating threaded workload-group children. Failures should be explicit when `ENABLE_WORKLOAD_GROUP=true`; preserve startup behavior when it is false and retain a separately validated v1 path. Please open the PR with the tested patch. The remaining evidence needed for acceptance is: - Exact official image tag/digest, BE version/commit, effective `doris_cgroup_cpu_path` and `num_cores`, runtime version, namespace mode, and cgroup mount options. The shell-only reproduction establishes the hierarchy problem but does not establish the prevalence across Kubernetes configurations. - Before/after `/proc/self/cgroup`, relevant cgroup2 mountinfo, and `cgroup.type`, `cgroup.controllers`, `cgroup.subtree_control`, and `cpu.max` at the container, `doris`, `query`, and workload-group levels; include actual syscall errors where setup fails. - An end-to-end Doris test with workload-group DDL/properties, session assignment, concurrent CPU-bound queries, worker TID membership, and `cpu.stat`/CPU-time deltas. A manually throttled busy thread verifies kernel behavior, but does not verify Doris worker placement or quota calculation. BE computes quota from its detected core count, and thread-pool workers currently discard the return status of cgroup assignment ([worker assignment](https://github.com/apache/doris/blob/4accb51bc1f27d84980668f3a2eaf0ac96fb3cba/be/src/util/threadpool.cpp#L543-L554)). - Coverage of both entrypoints, private and host cgroup namespaces, repeated startup, v1 compatibility, and clear failure for read-only cgroupfs or unavailable CPU delegation. Under load, verify both workload-group throttling and preservation of the container's aggregate CPU cap; retain the `docker exec` check. The fresh-hierarchy failure mechanism is supported by code and the supplied reproduction. Universality across v2 deployments and correctness of the full proposed fix remain unverified; an externally prepared hierarchy can have different controller availability. -- 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]
