Trim outdated sections from the tuning guide #3534
Conversation
|
The PR preview for 2387f88 is available at theforeman-foreman-documentation-preview-pr-3534.surge.sh The following output files are affected by this PR: |
|
@Imaanpreet could you have a look? Edit: also, if we say that you always want 16 Puma threads, why don't we make that the default? |
guides/common/modules/proc_manually-tuning-puma-workers-and-threads-count.adoc
Show resolved
Hide resolved
guides/common/modules/con_puma-workers-and-threads-recommendations.adoc
Outdated
Show resolved
Hide resolved
| @@ -22,9 +22,12 @@ postgresql::server::config_entries: | |||
| autovacuum_vacuum_cost_limit: 2000 | |||
| ---- | |||
| + | |||
| . Run the installer: | |||
| [options="nowrap" subs="attributes"] | |||
There was a problem hiding this comment.
| [options="nowrap" subs="attributes"] | |
| + | |
| [options="nowrap" subs="+quotes,verbatim,attributes"] |
There was a problem hiding this comment.
I have no problem removing this section, as it only covers the theory behind Puma workers, etc.
There was a problem hiding this comment.
I agree with removing the section, as we've already highlighted these changes in multiple places throughout the tuning guide.
There was a problem hiding this comment.
Do we have any Kbase article on Puma DB pool, aside from the bug report? (I'm asking because I'm not sure if we've mentioned it elsewhere before removing the section.)
There was a problem hiding this comment.
We applied a change in the installer to fix the issue and that's why I think we should no longer tell users to change it. But https://projects.theforeman.org/issues/33974 points me to https://bugzilla.redhat.com/show_bug.cgi?id=1976728 and that links to https://access.redhat.com/solutions/6964062. I don't have my RH login here so I can't check if that text reflects the real issue has been solved properly.
There was a problem hiding this comment.
Is there a specific reason for removing the following lines? We're simply sharing how our performance tests performed using these values.
There was a problem hiding this comment.
If you look at the commit in its own context (19b84e6) I described my reasoning. It's redundant given the installer matches min and max values now and I don't want users to use --foreman-foreman-service-puma-threads-min at all so I don't want to mention it.
There was a problem hiding this comment.
Looks good, we can go ahead and remove this section.
Back with Pulp 2 and Passenger were used this was needed, but these days it's no longer needed.
The installer now automatically sets the pool size large enough to accommodate the additional connections needed by Katello and users shouldn't touch these values anymore.
The installer matches threads_min to threads_max and users shouldn't touch this.
Rather than referring readers to a chapter that has some generic instructions, this includes them so the procedure is complete. Fixes: d217c11 ("Add procedure to apply changes to configuration (theforeman#1444)")
This pleases Vale.
|
Trivial rebase done and I implemented @maximiliankolb's suggestions. |
What changes are you introducing?
The goal is to make the tuning guide as short as possible with only relevant information. Overall we should work towards a good out-of-the-box experience without the need to manually tune. Manual tuning really is for the cases where our automatic code doesn't work.
Why are you introducing these changes? (Explanation, links to references, issues, etc.)
For most changes in this PR we have included the changes in the installer or the platform changed. See individual commits for details. Where possible I tied it back to the Redmine issue that changed it.
Anything else to add? (Considerations, potential downsides, alternative solutions you have explored, etc.)
For now I've only selected 3.13 as the cherry pick, but looking a the changes I think it also applies to many older versions.
Checklists
Please cherry-pick my commits into: