-
Notifications
You must be signed in to change notification settings - Fork 134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes #33312 - Set Puma tuning values #715
base: develop
Are you sure you want to change the base?
Conversation
This initially sets the Puma tuning values based on the same ideas from theforeman/puppet-foreman#986 statically calculated for each of the CPU and memory values that exist for the various profiles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there is a reason not to omit foreman::foreman_service_puma_threads_min
? If it is omitted, then the module will configure it equal to the maximum by default.
Arguably, including it here does make it easier for the user to understand what they are getting in each case?
I looked at it as the tuning profiles are designed to be concrete bases of configuration. Users can vary values on top of that base but the bases are meant to be more tightly controlled. Thus being explicit with the settings seemed in line with that philosophy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't work. Answers are read before built ins (and tuning is considered a built in):
foreman-installer/config/foreman-hiera.yaml
Lines 13 to 23 in dcd015c
- name: "Kafo Answers" | |
path: "%{facts.kafo.scenario.answer_file}" | |
- name: "Built in" | |
paths: | |
- "scenario/%{facts.kafo.scenario.id}/family/%{facts.os.family}-%{facts.os.release.major}.yaml" | |
- "family/%{facts.os.family}-%{facts.os.release.major}.yaml" | |
- "family/%{facts.os.family}.yaml" | |
- "security.yaml" | |
- "tuning/sizes/%{facts.kafo.scenario.custom.tuning}.yaml" | |
- "tuning/common.yaml" | |
- "common.yaml" |
Hiera is also not used to determine defaults so it will first read the defaults from Puppet itself, store that in the answers and ignore these.
How does it handle empty values? If the answers file has an empty value for the puma workers entry will that still take precedence over having a value set in the layer below such as this? |
I think my information is outdated and I actually solved the limitation: It was originally my goal to separate out all scenario defaults so |
So to clarify: right now I think this does work for fresh installations. However, for runtime switching (i.e., from existing medium to large) it won't change the answers. It'll be needed to write a hook that clears the answer, probably in |
Could we skip writing empty values to the answer files? Would that solve this problem? Would it create other problems? |
I think I found it. It's in this bit here: foreman-installer/config/foreman-proxy-content.migrations/200616155948-disable-pulp-3-proxying.rb Lines 11 to 13 in 9e4d03c
|
Oh, and I also thought about dropping a bunch of migrations instead but that's also a big task that I didn't want to start until I had time to finish it. |
This initially sets the Puma tuning values based on the same ideas
from theforeman/puppet-foreman#986 statically
calculated for each of the CPU and memory values that exist for the
various profiles.