bitflicker64 commented on code in PR #3187:
URL: https://github.com/apache/hugegraph/pull/3187#discussion_r3924578435
##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh:
##########
@@ -98,6 +98,19 @@ if [[ -n "${HG_SERVER_AUTH_TOKEN_SECRET:-}" ]]; then
fi
fi
+# How long the entrypoint lets the server take to answer on its REST port
+# before it gives up and ends the container. An orchestrator that already
+# owns this budget through a startup probe needs to raise it, otherwise the
+# container terminates a JVM that is still starting and the probe never gets
+# to decide. Validated here so a typo fails before init-store runs, rather
+# than reaching the arithmetic in wait_for_startup.
+SERVER_STARTUP_TIMEOUT_S="${HG_SERVER_STARTUP_TIMEOUT_S:-120}"
+if [[ ! "${SERVER_STARTUP_TIMEOUT_S}" =~ ^[1-9][0-9]*$ ]]; then
Review Comment:
Fixed in 6ca3df360. Reproduced first: with `now_s=1788438716`, `$((now_s +
9223372036854775807))` gives `-9223372035066337093`, so `[ "$now_s" -lt
"$stop_s" ]` is false and the loop never probes.
The accepted range is now 1 to 86400 seconds. The pattern is bounded to five
digits (`^[1-9][0-9]{0,4}$`) before the upper-bound comparison runs, so the
comparison itself cannot overflow either; bash wraps a 20-digit literal
silently rather than erroring, so the regex has to do that part. Boundary tests
added for 86400 accepted, 86401 and 9223372036854775807 rejected.
##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh:
##########
@@ -98,6 +98,19 @@ if [[ -n "${HG_SERVER_AUTH_TOKEN_SECRET:-}" ]]; then
fi
fi
+# How long the entrypoint lets the server take to answer on its REST port
+# before it gives up and ends the container. An orchestrator that already
+# owns this budget through a startup probe needs to raise it, otherwise the
+# container terminates a JVM that is still starting and the probe never gets
+# to decide. Validated here so a typo fails before init-store runs, rather
+# than reaching the arithmetic in wait_for_startup.
+SERVER_STARTUP_TIMEOUT_S="${HG_SERVER_STARTUP_TIMEOUT_S:-120}"
Review Comment:
Fixed in 6ca3df360, taking the suggestion. Confirmed the behaviour:
`${v:-120}` on an empty value yields `120`, `${v-120}` yields empty and so
fails the guard, and an unset variable still yields `120` either way.
`docker/docker-compose.yml:34-35` uses the `${VAR:-}` style, so this was
reachable.
The README sentence now says an empty value is rejected too, rather than the
reverse. Empty and whitespace-only are both in the rejected set in the test.
##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh:
##########
@@ -232,4 +233,25 @@ rm -f "${TEST_HOME}/docker/init_complete"
)
grep -Fqx -- '-n' "${TEST_HOME}/docker/init-store-password"
+last_start_args() { tail -n 1 "${TEST_HOME}/docker/start-hugegraph-args"; }
+
+[[ "$(last_start_args)" == *"-t 120"* ]]
+
+(
+ cd "${TEST_HOME}"
+ HG_SERVER_STARTUP_TIMEOUT_S=450 bash ./docker-entrypoint.sh
+)
+[[ "$(last_start_args)" == *"-t 450"* ]]
+
+start_calls_before_invalid_timeout=$(wc -l <
"${TEST_HOME}/docker/start-hugegraph-args")
+if (
+ cd "${TEST_HOME}"
+ HG_SERVER_STARTUP_TIMEOUT_S=2m bash ./docker-entrypoint.sh
+); then
+ echo "invalid startup timeout unexpectedly succeeded" >&2
+ exit 1
+fi
+[[ "$(wc -l < "${TEST_HOME}/docker/start-hugegraph-args")" -eq \
+ "${start_calls_before_invalid_timeout}" ]]
Review Comment:
Fixed in 6ca3df360. Your relocated-guard variant is now caught: I reproduced
it by moving the block below all three `init-store.sh` call sites, and the
suite fails with `startup timeout '' was rejected only after init-store ran`.
One thing worth flagging from checking it. The bare `[[ ]]` assertion style
does not fail on bash 3.2, still the `/bin/bash` of macOS: `set -e` there
ignores a failing `[[ ]]`, so my first run of your variant passed locally and
only failed under bash 5.3. The new assertions use an explicit `|| { echo ...;
exit 1; }` so they fail on both. Verified under 3.2.57 and 5.3.15.
##########
hugegraph-server/hugegraph-dist/docker/README.md:
##########
@@ -144,3 +144,20 @@ native `HEALTHCHECK` instructions. `docker ps` shows real
health status:
| `hugegraph/hugegraph-store` | `GET /v1/health` on port 8520 |
The entrypoints supervise the Java process directly — when Java exits, the
container exits. If started with a restart policy (the provided compose files
use `restart: unless-stopped`), Docker will bring it back automatically. The
old cron-based monitor (`-m true`) is for VM/bare-metal deployments only and is
not used in Docker images.
+
+## 7. Server Startup Timeout
+
+The entrypoint gives the Server a fixed budget to answer on its REST port and
+ends the container when the budget runs out. It is 120 seconds by default. Set
+`HG_SERVER_STARTUP_TIMEOUT_S` to a positive whole number of seconds to change
+it:
+
+```bash
+docker run -itd --name=graph -p 8080:8080 -e HG_SERVER_STARTUP_TIMEOUT_S=450
hugegraph/hugegraph:1.7.0
+```
+
+Raise it on slow or contended hosts, and wherever an orchestrator already owns
Review Comment:
Fixed in 6ca3df360. Section 7 now says the health check keeps a budget of
its own that this variable does not move: the images' `--interval=15s
--start-period=90s --retries=3` marks a container `unhealthy` around 135
seconds, raised with `--health-start-period`, while the Compose files
substitute `start_period: 60s`, `interval: 10s`, `retries: 30`, roughly 360
seconds, which is what `depends_on: condition: service_healthy` gates Hubble on.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
