Repository navigation
Reduce Puma and Pulp worker counts for development tuning - #925
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe development tuning profile sets Foreman Puma and three Pulp worker counts to 2. Unit tests load the profile through a shared fixture and check these values. The Candlepin heap test also uses the fixture and retains its existing expectations. ChangesDevelopment tuning
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~8 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The development profile consistently applies the lightweight worker counts chosen for this change. No actionable merge risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files. (1 skipped: 1 unsupported.)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
|
| foreman_puma_workers: 3 | ||
|
|
||
| pulp_worker_count: 2 | ||
| pulp_content_service_worker_count: 5 | ||
| pulp_api_service_worker_count: 3 |
There was a problem hiding this comment.
the numbers feel arbitrary, so why not go even harder and set 2 for everything?
There was a problem hiding this comment.
I mostly did "default / 2", but I don't see why we cannot do that :)
Per review feedback on PR theforeman#925: the previous numbers (3/2/5/3) were derived from the pulp_worker_count*2+1 / +1 formulas and felt arbitrary for a profile that just needs to be as light as possible. Set foreman_puma_workers, pulp_worker_count, pulp_content_service_worker_count, and pulp_api_service_worker_count all to 2. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Would you mind squashing and rebasing this? Then the IoP container tests will turn green. |
Pulp worker/Puma counts are derived from live ansible_facts (processor_nproc, memtotal_mb), not from the tuning profile, so any dev box above the bare 4-core/10GB floor scales them up automatically (e.g. foreman_puma_workers up to 12, pulp_content_service_worker_count up to 17 on an 8-core host) and OOMs on constrained development environments. Pin them to small fixed values for the development profile, the same way 6c79648 already pinned Candlepin's heap. Set foreman_puma_workers, pulp_worker_count, pulp_content_service_worker_count, and pulp_api_service_worker_count all to 2, rather than deriving them from the pulp_worker_count*2+1 / +1 formulas, since the development profile just needs to be as light as possible. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ba0ea5e to
168e81e
Compare
lzap
left a comment
lzap
left a comment
There was a problem hiding this comment.
Love it, RAM is not cheap today.
Would you mind also updating the RPM installation? I just witnessed 6 puma workers there too.
|
Ignore, I somewhat managed to copy-paste |
Implemented in theforeman/foreman-installer#1070. |
3 participants
Why are you introducing these changes?
Development environments can OOM when syncing several repositories concurrently. The root cause is that
foreman_puma_workersand thepulp_*_worker_countvars are computed from live host facts (ansible_facts['processor_nproc'],ansible_facts['memtotal_mb']) rather than from the selected tuning profile. That means--tuning development's 4-core/10GB floor doesn't actually cap resource usage: any dev box above that floor (a common case -- e.g. an 8-core/16GB laptop or CI VM) scales these up automatically, same as it would formedium/large.Computed at each profile's minimum hardware floor, before this change:
foreman_puma_workerspulp_worker_countpulp_content_service_worker_countpulp_api_service_worker_countdevelopmentanddefaultland on identical numbers (both have 4 cores as their floor; RAM isn't a factor for Pulp, and only mildly caps Puma), and on any dev box with more than the bare minimum, these climb further, same asmedium.developmenttuning doesn't actually imply "scaled down for a small box" -- it only gates the minimum.This mirrors the same problem
6c7964810a72("limited development Candlepin heap") already fixed for Candlepin's JVM heap.What are the changes?
Pin
foreman_puma_workers,pulp_worker_count,pulp_content_service_worker_count, andpulp_api_service_worker_countto2across the board insrc/vars/tuning/development.yml, overriding the hardware-scaled role defaults -- same mechanism, same file, same precedent as the existing Candlepin heap override.After this change:
foreman_puma_workerspulp_worker_countpulp_content_service_worker_countpulp_api_service_worker_countdefault,medium,large,extra-large, andextra-extra-largeare untouched and keep scaling from live hardware facts as before; onlydevelopmentis pinned.Also split/reordered the tuning unit tests to mirror the vars file's key order (Puma -> Pulp -> Candlepin) and factored out the repeated YAML-loading boilerplate into a
load_tuning_profile(name)helper + fixture, so a future test for another profile doesn't have to repeat it.Result
Development deployments use fixed, minimal worker counts, regardless of the actual host's CPU/RAM, significantly reducing memory pressure and avoiding OOM when syncing multiple repositories concurrently in dev environments.
🤖 Generated with Claude Code