Repository navigation
perf(pool): size the default pool from the CPUs the process may run on - #944
Conversation
A guide to CPU sets, memory limits, ports, journals and spill files per instance, and to what is safe when instances share a database: any number of readers, one writing process per database root, no in-place edits of table files, one rayforce version. The storage pages said symbol-file writes were safe from several processes and that symbol files carry checksums; neither holds. The spill-file naming now lists direct blocks too. The REPL and pipeline pages describe the pool size and core pinning.
The default pool had one thread per online CPU also under taskset or a cgroup cpuset, so an instance confined to 8 CPUs ran a thread per machine CPU on them, and under a CPU quota (docker --cpus, systemd CPUQuota=) more threads than the quota lets run, throttled in turn. The default size is now the process affinity mask on Linux (the online count elsewhere), or the CPU quota rounded up when that is smaller: the smallest v2 cpu.max or v1 cfs quota over its period on the process's cgroup and its ancestors. Threads are not tied to CPUs; the scheduler places them within the mask. A -c larger than that is obeyed with a warning on stderr. .sys.info reported the online CPU count; cores is now the CPUs the process may run on, and the new key workers is the pool size, main included. The banner prints the same CPU count. The RFL runner takes ";; @requires: linux" for files that check Linux-only facilities (taskset, cgroups).
The TSan job builds only the test binary and runs the files whose name holds "pool"; pool_cpuset started ./rayforce children, which that job does not build. Renamed to cpu_set_size.
morsel-ray
left a comment
morsel-ray
left a comment
There was a problem hiding this comment.
Read the complete21-file feature diff at faf79f1 (actual merge base d7153e7, not the newly accepted014 release tree). The placement of this work is appropriate: affinity/quota reads are at initialization/explicit CLI sizing, not in row/task loops, and explicit overrides remain overrides.
Two Linux boundary-detection cases still lose the accessible limit; details inline. I independently compiled the exact platform/pool TUs and exercised controlled fopen/affinity/sysconf fixtures: canonical v1/v2→2 participants and tighter ancestor→1 work; subtree-v2/noncanonical-v1 layouts return8 instead of2, and a large-kernel-mask EINVAL falls back to2048 online instead of the permitted2. These are interposed API/file-layout controls, not a claim of a real cgroup/mount deployment, large-host run or full PR build. No pool was created for the large-mask control.
Also please make the positive quota test's prerequisites/skip visible: the systemd-user guard can return0 without executing a quota assertion, and a tighter inherited user-manager quota can legitimately make the expected total1 rather than the literal2. Green file status alone should not stand for positive quota coverage. The new .sys.info pool initialization/frozen configuration is a user-visible compatibility detail worth calling out explicitly.
No independent whole-suite/TSan/performance or cross-platform compile claim. These requests are initialization-boundary fixes, not a request for new executor validation or altered task geometry.
… masks The quota was read under /sys/fs/cgroup at the membership path of /proc/self/cgroup. Where a container mounts its own v2 subtree there, or a v1 cpu controller sits elsewhere, those paths miss the quota files and the pool is sized to every allowed CPU again. The hierarchy's mount point and the subtree it shows now come from /proc/self/mountinfo; the membership is translated through that root and its visible ancestors are read up to the mount point. The conventional mounts remain the fallback without mountinfo. On a kernel built for more CPUs than a cpu_set_t holds, sched_getaffinity rejects the fixed buffer with EINVAL however few CPUs are allowed, and the count fell back to every online CPU. It now retries with a larger buffer. .sys.info created the pool to report its size; it now reports the size a default pool would get while none exists, from the rule the pool uses. The quota check of a child moves to pool/cpu_quota_child, which skips visibly without a systemd user manager or ./rayforce and expects at most the size the same child gets in a scope without a quota.
morsel-ray
left a comment
morsel-ray
left a comment
There was a problem hiding this comment.
Follow-up at cf21e6e: the two original single-view quota mappings and dynamic affinity retry are repaired in my independent controls; affinity thread resolved. The remaining multiple-visible-mount selection case is detailed in the existing quota thread, which stays open.
Fresh all-TU ASan/UBSan: platform8, pool auto2, CLI cores2, cpu-set RFL1, reserved-namespace2, system-branch1 passed. Positive real-quota child is explicitly SKIP here (no systemd user manager), not a passed quota workload. GCC five changed TUs×DEBUG/NDEBUG10 warning-clean. A real CLI without -c, with ENV3, gives (1 1 4) for task-count before/after .sys.info and reported workers: introspection no longer creates the pool. Source/HEAD/both binaries and whole relevant diagnostic logs verified. No full/TSan/native-large-host/cgroup-deployment/performance claim.
One test-only portability item below: production wide-mask handling is fixed, but the new regression still snapshots affinity with a fixed buffer. Please also avoid assuming1023 is larger than every real runner's allowed/default CPU count in the warning test; an affinity-confined child or a policy-derived expected warning avoids that assumption.
A cgroup hierarchy can be mounted more than once (a subtree and the whole tree, or an unrelated subtree first). The quota reader stopped at the first mount of the hierarchy, readable or not; it now reads every mount whose subtree holds the membership and keeps the tightest quota visible through any of them. The tests snapshot the affinity mask with a fixed buffer, which a kernel with a wider mask rejects: they now skip there. The -c warning test expects the warning exactly when -c passes the default pool size rather than assuming fewer CPUs than 1023.
morsel-ray
left a comment
morsel-ray
left a comment
There was a problem hiding this comment.
Approved e76dab1 after reading the complete original feature and both revisions, and independently verifying the requested repairs. All my findings are resolved.
- Exact-helper controls: all9 policy cases pass (canonical v1/v2, subtree/noncanonical paths, tighter ancestor, large mask-buffer retry, multiple/unreadable views and the wider visible ancestor). First-eligible-view-only counterfactual fails as intended. Synthetic API/file fixtures—not real mounts or large hardware.
- Fresh forced all-TU nonroot Clang ASan/UBSan: platform8/8, pool auto2/2, CLI cores2/2,
cpu_set_sizeRFL1/1, reserved namespace2/2, system branch1/1; whole relevant logs diagnostic-free. Real quota child:1 explicit SKIP, no systemd user manager here. - GCC six changed TUs×DEBUG/NDEBUG12 syntax checks pass. Real CLI with no
-cand ENV3 reports(1 1 4)for task-count before/after.sys.infoand hypothetical workers: introspection no longer creates the pool. Exact clean HEAD, all tracked sources and both binary hashes independently verified.
No independent full-suite/TSan/strict-docs, real quota deployment, worker-participation or performance claim. Please normally integrate the current accepted dev and obtain fresh required gates before landing; this approval covers the examined feature, not unreviewed future changes. Body breadcrumb: the tested RFL's actual name is rfl/system/cpu_set_size.rfl, rather than the old pool_cpuset spelling.
3 participants
The default pool had one thread per online CPU even under
taskset, a cgroup cpuset ordocker --cpuset-cpus. An instance confined to 8 CPUs of a 48-CPU machine ran 48 threads on them. Under a CPU quota (docker --cpus, systemdCPUQuota=) the pool also had more threads than the quota lets run, and they were throttled in turn.cpu.max, v1 cfs quota/period, smallest over the cgroup and its ancestors). Threads are not tied to CPUs.-clarger than that is obeyed, with a warning on stderr..sys.info:coresis the CPUs the process may run on; the new keyworkersis the pool size, main included. The banner prints the same CPU count.pool/auto_follows_cpu_setandrfl/system/pool_cpuset.rfl(child processes undertasksetand a CPU quota, Linux only via the new;; @requires: linuxtag).