update puma threads tunings #3248

Imaanpreet · 2024-08-30T18:49:52Z

What changes are you introducing?

Why are you introducing these changes? (Explanation, links to references, issues, etc.)

Anything else to add? (Considerations, potential downsides, alternative solutions you have explored, etc.)

Checklists

I am okay with my commits getting squashed when you merge this PR.
I am familiar with the contributing guidelines.

Please cherry-pick my commits into:

Foreman 3.13
Foreman 3.12/Katello 4.14 (Satellite 6.16)
Foreman 3.11/Katello 4.13
Foreman 3.10/Katello 4.12
Foreman 3.9/Katello 4.11 (Satellite 6.15; orcharhino 6.8/6.9/6.10)
Foreman 3.8/Katello 4.10
Foreman 3.7/Katello 4.9 (Satellite 6.14)
Foreman 3.6/Katello 4.8
Foreman 3.5/Katello 4.7 (Satellite 6.13; orcharhino 6.6/6.7)
We do not accept PRs for Foreman older than 3.5.

github-actions · 2024-08-30T18:52:15Z

The PR preview for e388b42 is available at theforeman-foreman-documentation-preview-pr-3248.surge.sh

The following output files are affected by this PR:

show diff

show diff as HTML

guides/common/modules/con_puma-threads.adoc

maximiliankolb

three more small suggestion, then I'd merge.

guides/common/modules/con_puma-threads.adoc

Imaanpreet · 2024-09-05T17:50:19Z

@maximiliankolb Hello, if all looks good to you can I squash all the commits into one?

maximiliankolb

Thanks @Imaanpreet diff LGTM. Merging is currently blocked by GitHub. Please rebase to "master" and resolve the merge conflicts.

maximiliankolb · 2024-09-19T11:41:17Z

triage: @Imaanpreet friendly reminder: please rebase to "master" and resolve the merge conflict. diff LGTM

Imaanpreet · 2024-09-25T04:34:40Z

Sorry for delayed changes @maximiliankolb, I have missed your reminder.
Thank you for checking.

maximiliankolb

One small fix, then we're good to go.

guides/common/modules/con_puma-threads.adoc

Apply suggestions from code review looks good. thank you guys! Co-authored-by: Aneta Šteflová Petrová <[email protected]>

maximiliankolb

LGTM

asteflova · 2024-10-04T11:26:18Z

@Imaanpreet You are requesting cherry picks into a few selected versions only, skipping quite a few in between. Is there a reason for that?

I'd also appreciate it if you provided some more details about this update by filling out the template in the PR description.

Imaanpreet · 2024-10-21T03:33:05Z

Hello Team, I am not sure why I can't get the updates (via emails) and I end up delaying the response :(
Yes, please go ahead and updates all the foreman versions as selected above.

maximiliankolb · 2024-11-07T12:41:58Z

@Imaanpreet Please ping an engineer to do a technical review.

jhutar · 2024-12-11T14:47:49Z

Hello @maximiliankolb ! Basically Imaan is the engineer here who measured this stuff. I approve this PR.

(cherry picked from commit 1675c0a)

asteflova · 2024-12-11T16:24:46Z

Merged to "master" and cherry-picked:

cff2084..7b0f76d 3.13 -> 3.13
86c92c4..908e617 3.12 -> 3.12

asteflova · 2024-12-11T16:27:04Z

Hello @maximiliankolb ! Basically Imaan is the engineer here who measured this stuff. I approve this PR.

Thanks for the additional ack, @jhutar! To explain: We've been evaluating our review process and part of that is standardizing who we ask to review doc PRs. For example, one suggestion that has been floated around recently is to ask a developer and a QA engineer to review each doc PR, to get multiple perspectives. Thanks for working with us on this PR.

asteflova · 2024-12-11T16:28:32Z

@Imaanpreet Thank you for working on this! I hit a conflict on 3.11 so I didn't cherry-pick there. If you think the update should be included on 3.11 and below, can you please create a separate PR? If you don't think it's necessary, I'm fine with that too.

ekohl

I still have a strong opinion that users shouldn't touch these values if we can avoid it. The values are automatically determined and if that's incorrect then we should fix it. For example, theforeman/puppet-foreman#1197.

My preference would be to drop this whole chapter or make it more like Puma's official documentation (https://msp-greg.github.io/puma/#thread-pool) with specific recommendations for our application.

ekohl · 2024-12-11T16:27:50Z

guides/common/modules/con_puma-threads.adoc

@@ -1,21 +1,13 @@
 [id="Puma_Threads_{context}"]
 = Puma threads

-Number of Puma threads (per Puma worker) is configured by using two values: `threads_min` and `threads_max`.
+To configure the number of Puma threads (per Puma worker), use these values: `threads_min` and `threads_max`.

 Value of `threads_min` determines how many threads each worker spawns at a worker start.
 Then, as concurrent requests are coming and more threads is needed, worker will be spawning more and more workers up to `threads_max` limit.

 We recommend setting `threads_min` to same value as `threads_max` as having fewer Puma threads lead to higher memory usage on your {ProjectServer}.


Since https://projects.theforeman.org/issues/33214 (Foreman 3.1) this is the default behavior if you don't specify the min value.

Can we drop this line?

ekohl · 2024-12-11T16:30:07Z

guides/common/modules/con_puma-threads.adoc

-Setting the minimum Puma threads to `16` results in about 12% less memory usage as compared to `threads_min=0`.
+For example, we have compared these two setups on a virtual machine with 8 CPUs and 40 GiB RAM using concurrent registrations test.
+They both used `--foreman-foreman-service-puma-threads-max=16` and `--foreman-foreman-service-puma-workers=2`.
+Setting the minimum Puma threads to `16` by using `--foreman-foreman-service-puma-threads-min=16` results in about 12% less memory usage as compared to `0`.


Since this is the default behavior, should we drop this line altogether and avoid users touching --foreman-foreman-service-puma-threads-min?

pr-processor bot added the Not yet reviewed label Aug 30, 2024

maximiliankolb reviewed Sep 2, 2024

View reviewed changes

guides/common/modules/con_puma-threads.adoc Outdated Show resolved Hide resolved

pr-processor bot removed the Not yet reviewed label Sep 2, 2024

maximiliankolb reviewed Sep 4, 2024

View reviewed changes

guides/common/modules/con_puma-threads.adoc Outdated Show resolved Hide resolved

guides/common/modules/con_puma-threads.adoc Outdated Show resolved Hide resolved

guides/common/modules/con_puma-threads.adoc Outdated Show resolved Hide resolved

maximiliankolb requested changes Sep 6, 2024

View reviewed changes

pr-processor bot added the Waiting on contributor Requires an action from the author label Sep 6, 2024

Imaanpreet force-pushed the master branch from 8edce18 to c396868 Compare September 25, 2024 04:23

pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author labels Sep 25, 2024

maximiliankolb requested changes Oct 1, 2024

View reviewed changes

guides/common/modules/con_puma-threads.adoc Outdated Show resolved Hide resolved

pr-processor bot added Waiting on contributor Requires an action from the author Needs re-review and removed Needs re-review Waiting on contributor Requires an action from the author labels Oct 1, 2024

update puma threads tunings

e388b42

Apply suggestions from code review looks good. thank you guys! Co-authored-by: Aneta Šteflová Petrová <[email protected]>

Imaanpreet force-pushed the master branch from 5c22c49 to e388b42 Compare October 3, 2024 00:39

maximiliankolb approved these changes Oct 4, 2024

View reviewed changes

pr-processor bot removed the Needs re-review label Oct 4, 2024

maximiliankolb requested a review from asteflova October 4, 2024 06:36

asteflova approved these changes Oct 21, 2024

View reviewed changes

asteflova added tech review done No issues from the technical perspective style review done No issues from docs style/grammar perspective Needs tech review Requires a review from the technical perspective and removed tech review done No issues from the technical perspective labels Nov 7, 2024

asteflova added tech review done No issues from the technical perspective and removed Needs tech review Requires a review from the technical perspective labels Dec 11, 2024

asteflova merged commit 1675c0a into theforeman:master Dec 11, 2024
9 checks passed

asteflova pushed a commit that referenced this pull request Dec 11, 2024

Update puma threads tunings (#3248)

908e617

(cherry picked from commit 1675c0a)

asteflova pushed a commit that referenced this pull request Dec 11, 2024

Update puma threads tunings (#3248)

7b0f76d

(cherry picked from commit 1675c0a)

ekohl reviewed Dec 11, 2024

View reviewed changes

asteflova mentioned this pull request Dec 11, 2024

Evaluate and address concerns for Puma thread tunings #3519

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update puma threads tunings #3248

update puma threads tunings #3248

Imaanpreet commented Aug 30, 2024 •

edited by maximiliankolb

Loading

github-actions bot commented Aug 30, 2024 •

edited

Loading

maximiliankolb left a comment

Imaanpreet commented Sep 5, 2024

maximiliankolb left a comment

maximiliankolb commented Sep 19, 2024

Imaanpreet commented Sep 25, 2024

maximiliankolb left a comment

maximiliankolb left a comment

asteflova commented Oct 4, 2024

Imaanpreet commented Oct 21, 2024

maximiliankolb commented Nov 7, 2024 •

edited

Loading

jhutar commented Dec 11, 2024

asteflova commented Dec 11, 2024

asteflova commented Dec 11, 2024

asteflova commented Dec 11, 2024

ekohl left a comment

ekohl Dec 11, 2024

ekohl Dec 11, 2024

update puma threads tunings #3248

update puma threads tunings #3248

Conversation

Imaanpreet commented Aug 30, 2024 • edited by maximiliankolb Loading

What changes are you introducing?

Why are you introducing these changes? (Explanation, links to references, issues, etc.)

Anything else to add? (Considerations, potential downsides, alternative solutions you have explored, etc.)

Checklists

github-actions bot commented Aug 30, 2024 • edited Loading

maximiliankolb left a comment

Choose a reason for hiding this comment

Imaanpreet commented Sep 5, 2024

maximiliankolb left a comment

Choose a reason for hiding this comment

maximiliankolb commented Sep 19, 2024

Imaanpreet commented Sep 25, 2024

maximiliankolb left a comment

Choose a reason for hiding this comment

maximiliankolb left a comment

Choose a reason for hiding this comment

asteflova commented Oct 4, 2024

Imaanpreet commented Oct 21, 2024

maximiliankolb commented Nov 7, 2024 • edited Loading

jhutar commented Dec 11, 2024

asteflova commented Dec 11, 2024

asteflova commented Dec 11, 2024

asteflova commented Dec 11, 2024

ekohl left a comment

Choose a reason for hiding this comment

ekohl Dec 11, 2024

Choose a reason for hiding this comment

ekohl Dec 11, 2024

Choose a reason for hiding this comment

Imaanpreet commented Aug 30, 2024 •

edited by maximiliankolb

Loading

github-actions bot commented Aug 30, 2024 •

edited

Loading

maximiliankolb commented Nov 7, 2024 •

edited

Loading