Merge pull request #931 from heroku/refactor-web-concurrency

Refactor $WEB_CONCURRENCY handling

CHANGELOG.md

@@ -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))

profile/WEB_CONCURRENCY.sh

@@ -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() {
@@ -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
@@ -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

test/unit

@@ -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() {