Skip to content

Commit

Permalink
Merge pull request #684 from heroku/autotune-fixes
Browse files Browse the repository at this point in the history 
WEB_CONCURRENCY auto-tuning: PHP memory_limit improvements, streamlined output, verbose mode
  • Loading branch information
@dzuelke
dzuelke authored Jan 31, 2024
2 parents cd7fbbb + 3866053 commit 679b8be
Show file tree
Hide file tree
Showing 9 changed files with 243 additions and 157 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@

- Boot scripts now have a `--test`/`-t` option to test PHP-FPM and web server configs and then exit. Can be repeated to dump configs for either or both, see `--help` for details. [David Zuelke]

### FIX

- PHP `memory_limit` in `.user.ini` does not override PHP-FPM `php_value` for `$WEB_CONCURRENCY` calculation [David Zuelke]

## v244 (2024-01-24)

### ADD
Expand Down
118 changes: 71 additions & 47 deletions bin/heroku-php-apache2
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,13 +366,34 @@ if [[ -n ${httpd_config:-} || ( ${httpd_config:=$(findconfig "$httpd_version" "$
fi
httpd_config=$(php_passthrough "$httpd_config")

fpm_pidfile=$(mktemp -t "heroku.php-fpm.pid-$PORT.XXXXXX" -u)
httpd_pidfile=$(httpd -t -D DUMP_RUN_CFG 2> /dev/null | sed -n -E 's/PidFile: "(.+)"/\1/p') # get PidFile location

# we need to dump the FPM config to fetch possible values for memory_limit in php_value and php_admin_value pool declarations
# parsing this by hand is cumbersome - there might be recursive includes in the config, glob patterns, etc
# also, the FPM behavior isn't quite correct (https://github.com/php/php-src/issues/13249), but might be fixed at some point
# so we use php-fpm -tt to dump the config, and extract the values from there
# we need to ensure the log level is NOTICE, since FPM uses its logging for dumping - but we default to WARNING
# that's why we're making a temporary config with the log level overridden
# (the same temporary file is also used for our own -tt and -tttt config test modes)
fpm_config_tmp=$(mktemp "$fpm_config.XXXXX")
cp "$fpm_config" "$fpm_config_tmp"
trap 'trap - EXIT; rm -f "$fpm_config_tmp"' EXIT
echo -e "\n[global]\nlog_level = notice" >> "$fpm_config_tmp"

# build command string arrays for PHP-FPM and HTTPD
# we're using an array, because we need to correctly preserve quoting, spaces, etc
fpm_command=( php-fpm --pid "$fpm_pidfile" --nodaemonize -y "$fpm_config_tmp" ${php_config:+-c "$php_config"} )
httpd_command=( httpd -D NO_DETACH -c "Include $httpd_config" )

mlib="/sys/fs/cgroup/memory/memory.limit_in_bytes"
if [[ -f "$mlib" ]]; then
[[ $verbose ]] && echo "Reading available RAM from '$mlib'" >&2
ram="$(cat "$mlib")"
echo "Detected ${ram} Bytes of RAM" >&2
else
[[ $verbose ]] && echo "No '$mlib' with RAM info found" >&2
ram="512M"
echo "Assuming ${ram}B of RAM" >&2
echo "Assuming RAM to be ${ram} Bytes" >&2
fi
if [[ -z ${WEB_CONCURRENCY:-} ]]; then
max_ram="16G" # blanket upper limit for how much RAM we will use for PHP, unless the machine has less anyway
Expand All @@ -382,62 +403,65 @@ if [[ -z ${WEB_CONCURRENCY:-} ]]; then
max_ram="6G"
fi

[[ $verbose ]] && echo "Calculating WEB_CONCURRENCY..." >&2

# dump the FPM config and parse out memory_limit declarations
# for that to work, we need the WEB_CONCURRENCY env var set, as the FPM config references it
export WEB_CONCURRENCY=1
# inside this subshell, we want pipefail on to ensure that a failing FPM command causes an exit
# grep might not match anything, so if it doesn't (status 1), we carry on, since that's fine (but other exit statuses will bubble up)
fpm_limits=$(set -o pipefail; "${fpm_command[@]}" -tt 2>&1 | { grep -E $'NOTICE: \tphp(_admin)?_value\[memory_limit\]' || test $? = 1; } | cut -f2) || {
# on failure, we should output something meaningful
echo "PHP-FPM config test failed with status $?:" >&2
# restore the original config to avoid any potential for confusion
cp "$fpm_config" "$fpm_config_tmp"
# single -t is enough this time
"${fpm_command[@]}" -t
exit 1
}

# determine number of FPM processes to run
read WEB_CONCURRENCY php_memory_limit <<<$(php ${php_config:+-c "$php_config"} "$bp_dir/bin/util/autotune.php" -y "$fpm_config" -t "$DOCUMENT_ROOT" "$ram" "$max_ram")
[[ $WEB_CONCURRENCY -lt 1 ]] && WEB_CONCURRENCY=1
# we feed it the PHP config we found much earlier (it must be the one FPM loads, not the CLI one!), docroot (for .user.ini), detected RAM, and RAM cap
# on STDIN, we also pass it any php_value or php_admin_value lines from the FPM config dump that may have a memory_limit (via a herestring, easier than echo)
WEB_CONCURRENCY=$(php ${php_config:+-c "$php_config"} -f "$bp_dir/bin/util/autotune.php" -- ${verbose:+-v} -t "$DOCUMENT_ROOT" "$ram" "$max_ram" <<<"$fpm_limits")
[[ $verbose ]] && echo "WEB_CONCURRENCY=${WEB_CONCURRENCY} (RAM / memory_limit)" >&2
[[ $WEB_CONCURRENCY -lt 1 ]] && { WEB_CONCURRENCY=1; [[ $verbose ]] && echo "WEB_CONCURRENCY=1 (was below mininum)" >&2; }
export WEB_CONCURRENCY

echo "PHP memory_limit is ${php_memory_limit} Bytes" >&2
else
echo '$WEB_CONCURRENCY env var is set, skipping automatic calculation' >&2
fi

fpm_conftest=
httpd_conftest=
fpm_config_tmp=$fpm_config
case $conftest in
[24])
# to dump the FPM config, we need to ensure the log level is NOTICE
fpm_config_tmp=$(mktemp "$fpm_config.XXXXX")
cp "$fpm_config" "$fpm_config_tmp"
trap 'trap - EXIT; rm "$fpm_config_tmp"' EXIT
echo -e "\n[global]\nlog_level = notice" >> "$fpm_config_tmp"
;;& # resume
1)
fpm_conftest="-t"
httpd_conftest="-t"
;;
2)
fpm_conftest="-tt"
httpd_conftest="-t"
;;
3)
fpm_conftest="-t"
httpd_conftest="-S"
;;
4)
fpm_conftest="-tt"
httpd_conftest="-S"
;;
esac

fpm_pidfile=$(mktemp -t "heroku.php-fpm.pid-$PORT.XXXXXX" -u)
httpd_pidfile=$(httpd -t -D DUMP_RUN_CFG 2> /dev/null | sed -n -E 's/PidFile: "(.+)"/\1/p') # get PidFile location

# build command string arrays for PHP-FPM and HTTPD
# we're using an array, because we need to correctly preserve quoting, spaces, etc
fpm_command=( php-fpm ${fpm_conftest} --pid "$fpm_pidfile" --nodaemonize -y "$fpm_config_tmp" ${php_config:+-c "$php_config"} )
httpd_command=( httpd ${httpd_conftest} -D NO_DETACH -c "Include $httpd_config" )

if [[ $conftest ]]; then
echo -e "\n$(basename "$0"): Config testing php-fpm using ${fpm_command[@]}:" >&2
"${fpm_command[@]}"
echo -e "\n$(basename "$0"): Config testing httpd using ${httpd_command[@]}:" >&2
"${httpd_command[@]}"
case $conftest in
1)
fpm_conftest="-t"
httpd_conftest="-t"
;;
2)
fpm_conftest="-tt"
httpd_conftest="-t"
;;
3)
fpm_conftest="-t"
httpd_conftest="-S"
;;
*)
fpm_conftest="-tt"
httpd_conftest="-S"
;;
esac

echo -e "\n$(basename "$0"): Config testing php-fpm using ${fpm_command[@]} ${fpm_conftest}:" >&2
"${fpm_command[@]}" ${fpm_conftest}
echo -e "\n$(basename "$0"): Config testing httpd using ${httpd_command[@]} ${httpd_conftest}:" >&2
"${httpd_command[@]}" ${httpd_conftest}
echo -e "\n$(basename "$0"): All configs okay." >&2
exit 0
fi

# we're done dumping stuff; restore original FPM config
cp "$fpm_config" "$fpm_config_tmp"

# make a shared pipe; we'll write the name of the process that exits to it once that happens, and wait for that event below
# this particular call works on Linux and Mac OS (will create a literal ".XXXXXX" on Mac, but that doesn't matter).
wait_pipe=$(mktemp -t "heroku.waitpipe-$PORT.XXXXXX" -u)
Expand Down
118 changes: 71 additions & 47 deletions bin/heroku-php-nginx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,13 +366,34 @@ nginx_config=$(php_passthrough "$nginx_config")

nginx_main=$(findconfig "$nginx_version" "$bp_dir/conf/nginx/main.conf")

fpm_pidfile=$(mktemp -t "heroku.php-fpm.pid-$PORT.XXXXXX" -u)
nginx_pidfile=$(mktemp -t "heroku.nginx.pid-$PORT.XXXXXX" -u)

# we need to dump the FPM config to fetch possible values for memory_limit in php_value and php_admin_value pool declarations
# parsing this by hand is cumbersome - there might be recursive includes in the config, glob patterns, etc
# also, the FPM behavior isn't quite correct (https://github.com/php/php-src/issues/13249), but might be fixed at some point
# so we use php-fpm -tt to dump the config, and extract the values from there
# we need to ensure the log level is NOTICE, since FPM uses its logging for dumping - but we default to WARNING
# that's why we're making a temporary config with the log level overridden
# (the same temporary file is also used for our own -tt and -tttt config test modes)
fpm_config_tmp=$(mktemp "$fpm_config.XXXXX")
cp "$fpm_config" "$fpm_config_tmp"
trap 'trap - EXIT; rm -f "$fpm_config_tmp"' EXIT
echo -e "\n[global]\nlog_level = notice" >> "$fpm_config_tmp"

# build command string arrays for PHP-FPM and Nginx
# we're using an array, because we need to correctly preserve quoting, spaces, etc
fpm_command=( php-fpm --pid "$fpm_pidfile" --nodaemonize -y "$fpm_config_tmp" ${php_config:+-c "$php_config"} )
nginx_command=( nginx -c "$nginx_main" -g "pid $nginx_pidfile; include $nginx_config;" )

mlib="/sys/fs/cgroup/memory/memory.limit_in_bytes"
if [[ -f "$mlib" ]]; then
[[ $verbose ]] && echo "Reading available RAM from '$mlib'" >&2
ram="$(cat "$mlib")"
echo "Detected ${ram} Bytes of RAM" >&2
else
[[ $verbose ]] && echo "No '$mlib' with RAM info found" >&2
ram="512M"
echo "Assuming ${ram}B of RAM" >&2
echo "Assuming RAM to be ${ram} Bytes" >&2
fi
if [[ -z ${WEB_CONCURRENCY:-} ]]; then
max_ram="16G" # blanket upper limit for how much RAM we will use for PHP, unless the machine has less anyway
Expand All @@ -382,62 +403,65 @@ if [[ -z ${WEB_CONCURRENCY:-} ]]; then
max_ram="6G"
fi

[[ $verbose ]] && echo "Calculating WEB_CONCURRENCY..." >&2

# dump the FPM config and parse out memory_limit declarations
# for that to work, we need the WEB_CONCURRENCY env var set, as the FPM config references it
export WEB_CONCURRENCY=1
# inside this subshell, we want pipefail on to ensure that a failing FPM command causes an exit
# grep might not match anything, so if it doesn't (status 1), we carry on, since that's fine (but other exit statuses will bubble up)
fpm_limits=$(set -o pipefail; "${fpm_command[@]}" -tt 2>&1 | { grep -E $'NOTICE: \tphp(_admin)?_value\[memory_limit\]' || test $? = 1; } | cut -f2) || {
# on failure, we should output something meaningful
echo "PHP-FPM config test failed with status $?:" >&2
# restore the original config to avoid any potential for confusion
cp "$fpm_config" "$fpm_config_tmp"
# single -t is enough this time
"${fpm_command[@]}" -t
exit 1
}

# determine number of FPM processes to run
read WEB_CONCURRENCY php_memory_limit <<<$(php ${php_config:+-c "$php_config"} "$bp_dir/bin/util/autotune.php" -y "$fpm_config" -t "$DOCUMENT_ROOT" "$ram" "$max_ram")
[[ $WEB_CONCURRENCY -lt 1 ]] && WEB_CONCURRENCY=1
# we feed it the PHP config we found much earlier (it must be the one FPM loads, not the CLI one!), docroot (for .user.ini), detected RAM, and RAM cap
# on STDIN, we also pass it any php_value or php_admin_value lines from the FPM config dump that may have a memory_limit (via a herestring, easier than echo)
WEB_CONCURRENCY=$(php ${php_config:+-c "$php_config"} -f "$bp_dir/bin/util/autotune.php" -- ${verbose:+-v} -t "$DOCUMENT_ROOT" "$ram" "$max_ram" <<<"$fpm_limits")
[[ $verbose ]] && echo "WEB_CONCURRENCY=${WEB_CONCURRENCY} (RAM / memory_limit)" >&2
[[ $WEB_CONCURRENCY -lt 1 ]] && { WEB_CONCURRENCY=1; [[ $verbose ]] && echo "WEB_CONCURRENCY=1 (was below mininum)" >&2; }
export WEB_CONCURRENCY

echo "PHP memory_limit is ${php_memory_limit} Bytes" >&2
else
echo '$WEB_CONCURRENCY env var is set, skipping automatic calculation' >&2
fi

fpm_conftest=
nginx_conftest=
fpm_config_tmp=$fpm_config
case $conftest in
[24])
# to dump the FPM config, we need to ensure the log level is NOTICE
fpm_config_tmp=$(mktemp "$fpm_config.XXXXX")
cp "$fpm_config" "$fpm_config_tmp"
trap 'trap - EXIT; rm "$fpm_config_tmp"' EXIT
echo -e "\n[global]\nlog_level = notice" >> "$fpm_config_tmp"
;;& # resume
1)
fpm_conftest="-t"
nginx_conftest="-t"
;;
2)
fpm_conftest="-tt"
nginx_conftest="-t"
;;
3)
fpm_conftest="-t"
nginx_conftest="-T"
;;
4)
fpm_conftest="-tt"
nginx_conftest="-T"
;;
esac

fpm_pidfile=$(mktemp -t "heroku.php-fpm.pid-$PORT.XXXXXX" -u)
nginx_pidfile=$(mktemp -t "heroku.nginx.pid-$PORT.XXXXXX" -u)

# build command string arrays for PHP-FPM and Nginx
# we're using an array, because we need to correctly preserve quoting, spaces, etc
fpm_command=( php-fpm ${fpm_conftest} --pid "$fpm_pidfile" --nodaemonize -y "$fpm_config_tmp" ${php_config:+-c "$php_config"} )
nginx_command=( nginx ${nginx_conftest} -c "$nginx_main" -g "pid $nginx_pidfile; include $nginx_config;" )

if [[ $conftest ]]; then
echo -e "\n$(basename "$0"): Config testing php-fpm using ${fpm_command[@]}:" >&2
"${fpm_command[@]}"
echo -e "\n$(basename "$0"): Config testing nginx using ${nginx_command[@]}:" >&2
"${nginx_command[@]}"
case $conftest in
1)
fpm_conftest="-t"
nginx_conftest="-t"
;;
2)
fpm_conftest="-tt"
nginx_conftest="-t"
;;
3)
fpm_conftest="-t"
nginx_conftest="-T"
;;
*)
fpm_conftest="-tt"
nginx_conftest="-T"
;;
esac

echo -e "\n$(basename "$0"): Config testing php-fpm using ${fpm_command[@]} ${fpm_conftest}:" >&2
"${fpm_command[@]}" ${fpm_conftest}
echo -e "\n$(basename "$0"): Config testing nginx using ${nginx_command[@]} ${nginx_conftest}:" >&2
"${nginx_command[@]}" ${nginx_conftest}
echo -e "\n$(basename "$0"): All configs okay." >&2
exit 0
fi

# we're done dumping stuff; restore original FPM config
cp "$fpm_config" "$fpm_config_tmp"

# make a shared pipe; we'll write the name of the process that exits to it once that happens, and wait for that event below
# this particular call works on Linux and Mac OS (will create a literal ".XXXXXX" on Mac, but that doesn't matter).
wait_pipe=$(mktemp -t "heroku.waitpipe-$PORT.XXXXXX" -u)
Expand Down
Loading

0 comments on commit 679b8be

Please sign in to comment.