Skip to content
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

Improve WEB_CONCURRENCY support #1547

Merged
merged 8 commits into from
Mar 13, 2024
Merged

Improve WEB_CONCURRENCY support #1547

merged 8 commits into from
Mar 13, 2024

Conversation

edmorley
Copy link
Member

@edmorley edmorley commented Mar 5, 2024

The Python buildpack, like several of the other languages buildpacks has for some time automatically set the WEB_CONCURRENCY environment variable at dyno boot (if it's not already set), based on the size of the Heroku dyno. The env var is then used by some Python web servers (such as Gunicorn and Uvicorn) to control the default number of server processes that they launch.

When the original Python buildpack implementation for this was written many years ago, there was not a way to determine dyno available memory vs the host memory (since /sys/fs/cgroup/memory/memory.limit_in_bytes did not exist). As such, the existing implementation relied upon a hardcoded mapping of known process limit values to dyno sizes:
https://devcenter.heroku.com/articles/limits#processes-threads

This mapping was fragile, since if the process limits ever changed in the future, or if new dyno types were added, then WEB_CONCURRENCY would not get set, or be set to an incorrect value.

In addition, the existing choice of concurrency values for Performance dynos (and their Private Space equivalents) was suboptimal, since on Performance-L dynos concurrency defaulted to 11, which is only 1.4 times the Performance-M's default concurrency of 8, even though the former has 4 times the number of CPU cores and 5.6 times the RAM.

As such, the buildpack now instead dynamically calculates the value for WEB_CONCURRENCY based on the dyno's actual specifications, by setting it to the lowest of either <dyno available RAM in MB> / 256 or <number of dyno CPU cores> * 2 + 1. The former ensures each web server process has at least 256 MB RAM available to reduce the chance of OOM, and the latter is based upon benchmarking and the Gunicorn worker guidance here:
https://docs.gunicorn.org/en/latest/design.html#how-many-workers

This new implementation results in the following default concurrency values for each Heroku dyno size:

  • eco / basic / standard-1x: 2 (unchanged)
  • standard-2x / private-s / shield-s: 4 (unchanged)
  • performance-m / private-m / shield-m: 5 (previously 8)
  • performance-l / private-l / shield-l: 17 (previously 11)

To increase awareness of the change in defaults, and to make the buildpack's existing automatic configuration of WEB_CONCURRENCY less of a black box, the .profile.d/ script now also prints memory/CPU/concurrency information to the app's logs (for web dynos only, to avoid breaking scripting use-cases).

For example:

app[web.1]: Python buildpack: Detected 14336 MB available memory and 8 CPU cores.
app[web.1]: Python buildpack: Defaulting WEB_CONCURRENCY to 17 based on the number of CPU cores.

If your app is relying on the buildpack-set WEB_CONCURRENCY value, and you do not wish to use the new default concurrency values, then you can switch back to the previous values (or whatever value performs best in benchmarks of your app), by either:

  • Setting WEB_CONCURRENCY explicitly as a config var on the app.
  • Or, passing --workers <num> to Gunicorn/Uvicorn in your Procfile (which takes priority over any WEB_CONCURRENCY env var).

Lastly, integration tests have been added for the buildpack's .profile.d/ scripts, since there were none before.

See:
https://devcenter.heroku.com/articles/config-vars
https://devcenter.heroku.com/articles/python-gunicorn
https://docs.gunicorn.org/en/latest/settings.html#workers
https://www.uvicorn.org/#command-line-options

GUS-W-14623334.
GUS-W-15109094.
GUS-W-15109115.
GUS-W-15131932.

@edmorley edmorley self-assigned this Mar 5, 2024
@edmorley edmorley marked this pull request as ready for review March 5, 2024 22:06
@edmorley edmorley requested a review from a team as a code owner March 5, 2024 22:06
@edmorley
Copy link
Member Author

edmorley commented Mar 6, 2024

Note: This change will be a no-op for most non-Heroku usages of the buildpack (such as Dokku), since in those environments WEB_CONCURRENCY was not being set before (due to lack of the custom process limits used by the hardcoded mapping), and still won't be now (since the buildpack skips configuring concurrency unless the /sys/fs/cgroup/memory/memory.limit_in_bytes file exists and contains a non-bogus value, which is only the case on Heroku, or when using cgroups v1 via older Docker with an explicit --memory limit set).

In the future we may choose to add support for cgroups v2's memory file too, however, enabling automatic WEB_CONCURRENCY in those non-Heroku environments would be a bigger breaking change, so has been deferred for now.

The Python buildpack, like several of the other languages buildpacks
has for some time automatically set the `WEB_CONCURRENCY` environment
variable at dyno boot (if it's not already set), based on the size
of the Heroku dyno. The env var is then used by some Python web servers
(such as gunicorn and uvicorn) to control the default number of server
processes that they launch.

When the original Python buildpack implementation for this was written
many years ago, there was not a way to determine dyno available memory
(since `/sys/fs/cgroup/memory/memory.limit_in_bytes` did not exist).
As such, the existing implementation relies upon a hardcoded mapping of
known process limit values to dyno sizes:
https://devcenter.heroku.com/articles/limits#processes-threads

This mapping is fragile, since if the process limits ever changes in the
future, or if new dyno types were added, then `WEB_CONCURRENCY` would
not be set, or worse, set to an incorrect value.

In addition, the existing choice of concurrency values for Performance
dynos (and their Private Space equivalents) was suboptimal, since
Performance-L dynos were set to a concurrency of 11, which is only 1.4
times the Performance-M's concurrency of 8, even though the former has
4 times the number of CPU cores, and 5.6 times the RAM.

As such, the buildpack now instead dynamically calculates the value for
`WEB_CONCURRENCY` based on the dyno's actual specifications, by setting
it to the lowest of either `<number of dyno CPU cores> * 2 + 1` or
`<dyno available RAM in MB> / 256`. The latter ensures each web server
process has at least 256 MB RAM available to reduce the chance of OOM,
and the former is based upon the gunicorn worker guidance here:
https://docs.gunicorn.org/en/latest/design.html#how-many-workers

This new implementation results in the following default concurrency
values for each Heroku dyno size:

- `eco` / `basic` / `standard-1x`: 2 (unchanged)
- `standard-2x` / `private-s` / `shield-s`: 4 (unchanged)
- `performance-m` / `private-m` / `shield-m`: 5 (previously 8)
- `performance-l` / `private-l` / `shield-l`: 17 (previously 11)

To increase awareness of this change, and to make the buildpack's
existing automatic configuration of `WEB_CONCURRENCY` less of a black
box, the `.profile.d/` script now also prints memory/CPU/concurrency
information to the app's logs (for web dynos only, to avoid breaking
scripting use-cases).

If your app is relying on the buildpack-set `WEB_CONCURRENCY` value, and
you do not wish to use the new default concurrency values, then you can
switch back to the previous values (or whatever value performs best in
benchmarks of your app), by either:

- Setting `WEB_CONCURRENCY` explicitly as a config var on the app.
- Or, passing `--workers <num>` to gunicorn/uvicorn in your `Procfile`
  (which takes priority over any `WEB_CONCURRENCY` env var).

Lastly, integration tests have been added for the buildpack's
`.profile.d/` scripts, since there were none before.

See:
https://devcenter.heroku.com/articles/config-vars
https://devcenter.heroku.com/articles/python-gunicorn
https://docs.gunicorn.org/en/latest/settings.html#workers
https://www.uvicorn.org/#command-line-options

GUS-W-14623334.
GUS-W-15109094.
GUS-W-15109115.
GUS-W-15131932.
vendor/WEB_CONCURRENCY.sh Outdated Show resolved Hide resolved
spec/hatchet/profile_d_scripts_spec.rb Show resolved Hide resolved
@edmorley edmorley merged commit 36db3fe into main Mar 13, 2024
5 checks passed
@edmorley edmorley deleted the web-concurrency branch March 13, 2024 08:08
@heroku-linguist heroku-linguist bot mentioned this pull request Mar 13, 2024
@edmorley
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants