imbajin commented on code in PR #3187:
URL: https://github.com/apache/hugegraph/pull/3187#discussion_r3933756783
##########
hugegraph-server/hugegraph-dist/docker/README.md:
##########
@@ -144,3 +144,30 @@ 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 whole number of seconds between 1 and 86400
Review Comment:
⚠️ The new variable is not propagated by any provided Compose topology.
`docker/docker-compose.yml`, `docker/docker-compose-hstore.yml`, and
`docker/docker-compose-3pd-3store-3server.yml` do not declare
`HG_SERVER_STARTUP_TIMEOUT_S` in the Server environment. Reproduction on this
head: `HG_SERVER_STARTUP_TIMEOUT_S=450 docker compose -f
docker/docker-compose.yml config --format json` emits no such key, so the
container still uses the entrypoint default `120` and the healthcheck budget
cannot be aligned. Please add the variable to each Server environment/anchor
with unset-only default semantics and cover it in the Compose render test.
##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh:
##########
@@ -232,4 +233,60 @@ 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"* ]]
+
+(
+ cd "${TEST_HOME}"
+ HG_SERVER_STARTUP_TIMEOUT_S=86400 bash ./docker-entrypoint.sh
+)
+[[ "$(last_start_args)" == *"-t 86400"* ]]
+
+# An empty value is a set value, not an absent one: Compose writes it whenever
+# an interpolated host variable is missing. 2m is the shape of a typo, and the
+# two large values bracket the point where the deadline arithmetic in
+# wait_for_startup would wrap negative and end the wait before its first probe.
+for invalid_timeout in "" " " 0 +5 2m 86401 9223372036854775807; do
+ start_calls_before_invalid=$(wc -l <
"${TEST_HOME}/docker/start-hugegraph-args")
+ init_calls_before_invalid=$(wc -l < "${TEST_HOME}/docker/init-store-calls")
+ if (
+ cd "${TEST_HOME}"
+ HG_SERVER_STARTUP_TIMEOUT_S="${invalid_timeout}" \
+ bash ./docker-entrypoint.sh
+ ); then
+ echo "startup timeout '${invalid_timeout}' unexpectedly succeeded" >&2
+ exit 1
+ fi
+ # The server must not have started, and the guard must have run ahead of
+ # init-store, as the comment above it in the entrypoint claims. Spelled
+ # with an explicit exit rather than a bare [[ ]]: bash 3.2, still the
+ # /bin/bash of macOS, does not apply set -e to a failing [[ ]], so a bare
+ # assertion passes silently there while CI catches the regression.
+ [[ "$(wc -l < "${TEST_HOME}/docker/start-hugegraph-args")" -eq \
+ "${start_calls_before_invalid}" ]] || {
+ echo "startup timeout '${invalid_timeout}' started the server" >&2
+ exit 1
+ }
+ [[ "$(wc -l < "${TEST_HOME}/docker/init-store-calls")" -eq \
+ "${init_calls_before_invalid}" ]] || {
+ echo "startup timeout '${invalid_timeout}' was rejected only after" \
+ "init-store ran" >&2
+ exit 1
+ }
+done
+
+# An unset variable still keeps the historical default.
+(
+ cd "${TEST_HOME}"
+ bash ./docker-entrypoint.sh
Review Comment:
⚠️ This block claims to exercise an unset variable but only starts a child
shell; it does not unset an inherited `HG_SERVER_STARTUP_TIMEOUT_S`. Running
the test with `export HG_SERVER_STARTUP_TIMEOUT_S=450` makes this call exercise
`450` instead of the default `120`, and the bare assertion can still report
success on Bash 3.2. Please invoke it with `env -u HG_SERVER_STARTUP_TIMEOUT_S`
or unset the variable inside the subshell, while keeping an explicit failing
assertion.
##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh:
##########
@@ -232,4 +233,60 @@ 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"* ]]
Review Comment:
⚠️ `last_start_args` records the whole `$*`, and `*"-t 120"*` / `*"-t 450"*`
only checks a substring. A launcher receiving `-t 1200` or `-t 4500` would pass
these assertions; the stub never invokes the real launcher. Please record
`"$@"` as separate arguments or parse the positional arguments and assert the
exact `-t` value.
##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh:
##########
@@ -232,4 +233,60 @@ 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"* ]]
Review Comment:
⚠️ The positive timeout checks are false-green on the supported macOS Bash
3.2.57. With `set -euo pipefail`, a failing standalone `[[ ... ]]` returns
nonzero but the script continues (`set -e; [[ 1 == 2 ]]; echo survived` prints
`survived`), so a broken override can still reach PASS. Please use explicit `if
! ...; then ... exit 1; fi` assertions for the default and override cases and
the final unset case.
--
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]
