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.

Sign up for GitHub

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

Jump to bottom

Refactor $WEB_CONCURRENCY handling #931

Merged
merged 3 commits into from
Jul 1, 2021
Merged

Refactor $WEB_CONCURRENCY handling #931

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8770f1b
fix double local var def in bound_memory
dzuelke Mar 19, 2021
1c1c5ca
split WEB_CONCURRENCY validation from calculation
dzuelke Mar 19, 2021
52a8d7f
warn_high_web_concurrency, no more duplicate bounds check
dzuelke Jul 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Node.js Buildpack Changelog

## main
- Refactor $WEB_CONCURRENCY logic ([#931](https://github.com/heroku/heroku-buildpack-nodejs/pull/931))

## v185 (2021-06-03)
- Drop Heroku-16 from CI test matrix ([#920](https://github.com/heroku/heroku-buildpack-nodejs/pull/920))
Expand Down
47 changes: 33 additions & 14 deletions profile/WEB_CONCURRENCY.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,25 @@
calculate_concurrency() {
local available=$1
local web_memory=$2
local concurrency

concurrency=${WEB_CONCURRENCY-$(($available/$web_memory))}
echo $(($available/$web_memory))
}

validate_concurrency() {
local concurrency=$1
local ret=0

if (( concurrency < 1 )); then
concurrency=1
ret=1
elif (( concurrency > 200 )); then
# Ex: This will happen on Dokku on DO
concurrency=1
ret=2
fi

echo "$concurrency"
return $ret
}

log_concurrency() {
Expand All @@ -32,7 +41,7 @@ detect_memory() {

bound_memory() {
local detected=$1
local detected max_detected_memory=14336
local max_detected_memory=14336

# The hardcoded value is 16GB of memory
if (( detected > max_detected_memory )); then
Expand All @@ -42,25 +51,35 @@ bound_memory() {
fi
}

warn_bad_web_concurrency() {
local concurrency=$((MEMORY_AVAILABLE/WEB_MEMORY))
if [ "$concurrency" -gt "200" ]; then
echo "Could not determine a reasonable value for WEB_CONCURRENCY.
warn_high_web_concurrency() {
echo "Could not determine a reasonable value for WEB_CONCURRENCY.
This is likely due to running the Heroku NodeJS buildpack on a non-Heroku
platform.

WEB_CONCURRENCY has been set to 1. Please review whether this value is
appropriate for your application."
echo ""
fi
WEB_CONCURRENCY has been set to ${1}. Please review whether this value is
appropriate for your application.
"
}

DETECTED=$(detect_memory 512)
export MEMORY_AVAILABLE=${MEMORY_AVAILABLE-$(bound_memory $DETECTED)}
export WEB_MEMORY=${WEB_MEMORY-512}
export WEB_CONCURRENCY=$(calculate_concurrency $MEMORY_AVAILABLE $WEB_MEMORY)

warn_bad_web_concurrency
WEB_CONCURRENCY=${WEB_CONCURRENCY-$(calculate_concurrency "$MEMORY_AVAILABLE" "$WEB_MEMORY")}
validated_concurrency=$(validate_concurrency "$WEB_CONCURRENCY")
case $? in # validate_concurrency exit code indicates result
1)
# too low
export WEB_CONCURRENCY=$validated_concurrency
;;
2)
# too high
warn_high_web_concurrency "$validated_concurrency" "$WEB_CONCURRENCY"
export WEB_CONCURRENCY=$validated_concurrency
;;
0)
export WEB_CONCURRENCY
;;
esac

if [[ "${LOG_CONCURRENCY+isset}" && "$LOG_CONCURRENCY" == "true" ]]; then
log_concurrency
Expand Down
4 changes: 2 additions & 2 deletions test/unit
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,9 +334,9 @@ testWebConcurrencyProfileScript() {
assertEquals "28" "$(calculate_concurrency 14336 512)"

# In case some very large memory available value gets passed in
assertEquals "1" "$(calculate_concurrency 103401 512)"
assertEquals "1" "$(validate_concurrency $(calculate_concurrency 103401 512))"
# of if web memory is set really low
assertEquals "1" "$(calculate_concurrency 512 1)"
assertEquals "1" "$(validate_concurrency $(calculate_concurrency 512 1))"
}

isUUID() {
Expand Down