Improve WEB_CONCURRENCY support (#1547)

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.

.github/workflows/ci.yml

@@ -35,6 +35,7 @@ jobs:
     env:
       HATCHET_APP_LIMIT: 200
       HATCHET_DEFAULT_STACK: ${{ matrix.stack }}
+      HATCHET_EXPENSIVE_MODE: 1
       HATCHET_RETRIES: 2
       HEROKU_API_KEY: ${{ secrets.HEROKU_API_KEY }}
       HEROKU_API_USER: ${{ secrets.HEROKU_API_USER }}

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## [Unreleased]
 
+- Improved the automatic `WEB_CONCURRENCY` feature: ([#1547](https://github.com/heroku/heroku-buildpack-python/pull/1547))
+  - Switched to a dynamic calculation based on dyno CPU cores and memory instead of a hardcoded mapping.
+  - Decreased default concurrency on `performance-m` / `private-m` / `shield-m` dynos from `8` to `5`.
+  - Increased default concurrency on `performance-l` / `private-l` / `shield-l` dynos from `11` to `17`.
+  - Added logging of memory/CPU/concurrency information to the app logs (for web dynos only).
 
 ## [v243] - 2024-02-07
 

Makefile

@@ -10,10 +10,14 @@ STACK_IMAGE_TAG := heroku/$(subst -,:,$(STACK))-build
 
 lint: lint-scripts lint-ruby
 
+# TODO: Enable scanning for files that are currently missed and/or restructure repo
+# layout to make it more viable to use wildcards here, given:
+# https://github.com/koalaman/shellcheck/issues/962
 lint-scripts:
 	@shellcheck -x bin/compile bin/detect bin/release bin/test-compile bin/utils bin/warnings bin/default_pythons
 	@shellcheck -x bin/steps/collectstatic bin/steps/nltk bin/steps/pip-install bin/steps/pipenv bin/steps/pipenv-python-version bin/steps/python
 	@shellcheck -x bin/steps/hooks/*
+	@shellcheck -x vendor/WEB_CONCURRENCY.sh
 	@shellcheck -x builds/*.sh
 
 lint-ruby:

spec/fixtures/procfile/Procfile

@@ -0,0 +1,2 @@
+web: true
+example-worker: true

spec/hatchet/profile_d_scripts_spec.rb

@@ -0,0 +1,124 @@
+# frozen_string_literal: true
+
+require_relative '../spec_helper'
+
+RSpec.describe '.profile.d/ scripts' do
+  it 'sets the required run-time env vars' do
+    Hatchet::Runner.new('spec/fixtures/procfile', run_multi: true).deploy do |app|
+      # These are written as a single test to reduce end to end test time. This repo uses parallel_split_test,
+      # so we can't perform app setup in a `before(:all)` and have multiple tests run against the single app.
+
+      list_envs_cmd = 'env | sort | grep -vE "^(_|DYNO|PORT|PS1|SHLVL|TERM)="'
+
+      # Check all env vars are set correctly when there are no user-provided env vars.
+      # Also checks that the WEB_CONCURRENCY related log output is not shown for one-off dynos.
+      app.run_multi(list_envs_cmd) do |output, _|
+        expect(output).to eq(<<~OUTPUT)
+          DYNO_RAM=512
+          FORWARDED_ALLOW_IPS=*
+          GUNICORN_CMD_ARGS=--access-logfile -
+          HOME=/app
+          LANG=en_US.UTF-8
+          LD_LIBRARY_PATH=/app/.heroku/vendor/lib:/app/.heroku/python/lib:
+          LIBRARY_PATH=/app/.heroku/vendor/lib:/app/.heroku/python/lib:
+          PATH=/app/.heroku/python/bin:/usr/local/bin:/usr/bin:/bin
+          PWD=/app
+          PYTHONHASHSEED=random
+          PYTHONHOME=/app/.heroku/python
+          PYTHONPATH=/app
+          PYTHONUNBUFFERED=true
+          WEB_CONCURRENCY=2
+        OUTPUT
+      end
+
+      # Check user-provided env var values are preserved/overridden as appropriate.
+      # Also checks that the WEB_CONCURRENCY related log output is not shown for worker dynos.
+      user_env_vars = [
+        'DYNO_RAM=this-should-be-overridden',
+        'FORWARDED_ALLOW_IPS=this-should-be-overridden',
+        'GUNICORN_CMD_ARGS=this-should-be-preserved',
+        'HOME=this-should-be-overridden',
+        'LANG=this-should-be-overridden',
+        'LD_LIBRARY_PATH=/this-should-be-preserved',
+        'LIBRARY_PATH=/this-should-be-preserved',
+        'PATH=/this-should-be-preserved:/usr/local/bin:/usr/bin:/bin',
+        'PYTHONHASHSEED=this-should-be-preserved',
+        'PYTHONHOME=/this-should-be-overridden',
+        'PYTHONPATH=/this-should-be-preserved',
+        'PYTHONUNBUFFERED=this-should-be-overridden',
+        'WEB_CONCURRENCY=this-should-be-preserved'
+      ]
+      app.run_multi(list_envs_cmd, heroku: { env: user_env_vars.join(';'), type: 'example-worker' }) do |output, _|
+        expect(output).to eq(<<~OUTPUT)
+          DYNO_RAM=512
+          FORWARDED_ALLOW_IPS=*
+          GUNICORN_CMD_ARGS=this-should-be-preserved
+          HOME=/app
+          LANG=C.UTF-8
+          LD_LIBRARY_PATH=/app/.heroku/vendor/lib:/app/.heroku/python/lib:/this-should-be-preserved
+          LIBRARY_PATH=/app/.heroku/vendor/lib:/app/.heroku/python/lib:/this-should-be-preserved
+          PATH=/app/.heroku/python/bin:/this-should-be-preserved:/usr/local/bin:/usr/bin:/bin
+          PWD=/app
+          PYTHONHASHSEED=this-should-be-preserved
+          PYTHONHOME=/app/.heroku/python
+          PYTHONPATH=/this-should-be-preserved
+          PYTHONUNBUFFERED=true
+          WEB_CONCURRENCY=this-should-be-preserved
+        OUTPUT
+      end
+
+      list_concurrency_envs_cmd = 'env | sort | grep -E "^(DYNO_RAM|WEB_CONCURRENCY)="'
+
+      # Check WEB_CONCURRENCY support when using a Standard-1X dyno.
+      # We set the process type to `web` so that we can test the web-dyno-only log output.
+      app.run_multi(list_concurrency_envs_cmd, heroku: { size: 'standard-1x', type: 'web' }) do |output, _|
+        expect(output).to eq(<<~OUTPUT)
+          Python buildpack: Detected 512 MB available memory and 8 CPU cores.
+          Python buildpack: Defaulting WEB_CONCURRENCY to 2 based on the available memory.
+          DYNO_RAM=512
+          WEB_CONCURRENCY=2
+        OUTPUT
+      end
+
+      # Standard-2X
+      app.run_multi(list_concurrency_envs_cmd, heroku: { size: 'standard-2x', type: 'web' }) do |output, _|
+        expect(output).to eq(<<~OUTPUT)
+          Python buildpack: Detected 1024 MB available memory and 8 CPU cores.
+          Python buildpack: Defaulting WEB_CONCURRENCY to 4 based on the available memory.
+          DYNO_RAM=1024
+          WEB_CONCURRENCY=4
+        OUTPUT
+      end
+
+      # Performance-M
+      app.run_multi(list_concurrency_envs_cmd, heroku: { size: 'performance-m', type: 'web' }) do |output, _|
+        expect(output).to eq(<<~OUTPUT)
+          Python buildpack: Detected 2560 MB available memory and 2 CPU cores.
+          Python buildpack: Defaulting WEB_CONCURRENCY to 5 based on the number of CPU cores.
+          DYNO_RAM=2560
+          WEB_CONCURRENCY=5
+        OUTPUT
+      end
+
+      # Performance-L
+      app.run_multi(list_concurrency_envs_cmd, heroku: { size: 'performance-l', type: 'web' }) do |output, _|
+        expect(output).to eq(<<~OUTPUT)
+          Python buildpack: Detected 14336 MB available memory and 8 CPU cores.
+          Python buildpack: Defaulting WEB_CONCURRENCY to 17 based on the number of CPU cores.
+          DYNO_RAM=14336
+          WEB_CONCURRENCY=17
+        OUTPUT
+      end
+
+      # Check that WEB_CONCURRENCY is preserved if set, but that we still set DYNO_RAM.
+      app.run_multi(list_concurrency_envs_cmd, heroku: { env: 'WEB_CONCURRENCY=999', type: 'web' }) do |output, _|
+        expect(output).to eq(<<~OUTPUT)
+          Python buildpack: Detected 512 MB available memory and 8 CPU cores.
+          Python buildpack: Skipping automatic configuration of WEB_CONCURRENCY since it's already set.
+          DYNO_RAM=512
+          WEB_CONCURRENCY=999
+        OUTPUT
+      end
+    end
+  end
+end

vendor/WEB_CONCURRENCY.sh

@@ -1,29 +1,92 @@
-case $(ulimit -u) in
-
-# Automatic configuration for Gunicorn's Workers setting.
-
-# Standard-1X (+Free, +Hobby) Dyno
-256)
-  export DYNO_RAM=512
-  export WEB_CONCURRENCY=${WEB_CONCURRENCY:-2}
-  ;;
-
-# Standard-2X Dyno
-512)
-  export DYNO_RAM=1024
-  export WEB_CONCURRENCY=${WEB_CONCURRENCY:-4}
-  ;;
-
-# Performance-M Dyno
-16384)
-  export DYNO_RAM=2560
-  export WEB_CONCURRENCY=${WEB_CONCURRENCY:-8}
-  ;;
-
-# Performance-L Dyno
-32768)
-  export DYNO_RAM=14336
-  export WEB_CONCURRENCY=${WEB_CONCURRENCY:-11}
-  ;;
-
-esac
+#!/usr/bin/env bash
+
+# This script was created by the Python buildpack to automatically set the `WEB_CONCURRENCY`
+# environment variable at dyno boot (if it's not already set), based on the available memory
+# and number of CPU cores. 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.
+#
+# The default `WEB_CONCURRENCY` value is calculated as the lowest of either:
+# - `<number of dyno CPU cores> * 2 + 1`
+# - `<dyno available RAM in MB> / 256` (to ensure each process has at least 256 MB RAM)
+#
+# Currently, on Heroku dynos this results in the following concurrency values:
+# - Eco / Basic / Standard-1X: 2 (capped by the 512 MB available memory)
+# - Standard-2X / Private-S / Shield-S: 4 (capped by the 1 GB available memory)
+# - Performance-M / Private-M / Shield-M: 5 (based on the 2 CPU cores)
+# - Performance-L / Private-L / Shield-L: 17 (based on the 8 CPU cores)
+#
+# To override these default values, either set `WEB_CONCURRENCY` as an explicit config var
+# on the app, or pass `--workers <num>` when invoking gunicorn/uvicorn in your Procfile.
+
+# Note: Since this is a .profile.d/ script it will be sourced, meaning that we cannot enable
+# exit on error, have to use return not exit, and returning non-zero doesn't have an effect.
+
+function detect_memory_limit_in_mb() {
+  local memory_limit_file='/sys/fs/cgroup/memory/memory.limit_in_bytes'
+
+  # This memory limits file only exists on Heroku, or when using cgroups v1 (Docker < 20.10).
+  if [[ -f "${memory_limit_file}" ]]; then
+    local memory_limit_in_mb=$(($(cat "${memory_limit_file}") / 1048576))
+
+    # Ignore values above 1TB RAM, since when using cgroups v1 the limits file reports a
+    # bogus value of thousands of TB RAM when there is no container memory limit set.
+    if ((memory_limit_in_mb <= 1048576)); then
+      echo "${memory_limit_in_mb}"
+      return 0
+    fi
+  fi
+
+  return 1
+}
+
+function output() {
+  # Only display log output for web dynos, to prevent breaking one-off dyno scripting use-cases,
+  # and to prevent confusion from messages about WEB_CONCURRENCY in the logs of non-web workers.
+  # (We still actually set the env vars for all dyno types for consistency and easier debugging.)
+  if [[ "${DYNO:-}" == web.* ]]; then
+    echo "Python buildpack: $*" >&2
+  fi
+}
+
+if ! available_memory_in_mb=$(detect_memory_limit_in_mb); then
+  # This should never occur on Heroku, but will be common for non-Heroku environments such as Dokku.
+  output "Couldn't determine available memory. Skipping automatic configuration of WEB_CONCURRENCY."
+  return 0
+fi
+
+if ! cpu_cores=$(nproc); then
+  # This should never occur in practice, since this buildpack only supports being run on our base
+  # images, and nproc is installed in all of them.
+  output "Couldn't determine number of CPU cores. Skipping automatic configuration of WEB_CONCURRENCY."
+  return 0
+fi
+
+output "Detected ${available_memory_in_mb} MB available memory and ${cpu_cores} CPU cores."
+
+# This env var is undocumented and not consistent with what other buildpacks set, however,
+# GitHub code search shows there are Python apps in the wild that do rely upon it.
+export DYNO_RAM="${available_memory_in_mb}"
+
+if [[ -v WEB_CONCURRENCY ]]; then
+  output "Skipping automatic configuration of WEB_CONCURRENCY since it's already set."
+  return 0
+fi
+
+minimum_memory_per_process_in_mb=256
+
+# Prevents WEB_CONCURRENCY being set to zero if the environment is extremely memory constrained.
+if ((available_memory_in_mb < minimum_memory_per_process_in_mb)); then
+  max_concurrency_for_available_memory=1
+else
+  max_concurrency_for_available_memory=$((available_memory_in_mb / minimum_memory_per_process_in_mb))
+fi
+
+max_concurrency_for_cpu_cores=$((cpu_cores * 2 + 1))
+
+if ((max_concurrency_for_available_memory < max_concurrency_for_cpu_cores)); then
+  export WEB_CONCURRENCY="${max_concurrency_for_available_memory}"
+  output "Defaulting WEB_CONCURRENCY to ${WEB_CONCURRENCY} based on the available memory."
+else
+  export WEB_CONCURRENCY="${max_concurrency_for_cpu_cores}"
+  output "Defaulting WEB_CONCURRENCY to ${WEB_CONCURRENCY} based on the number of CPU cores."
+fi