bitflicker64 commented on code in PR #3187:
URL: https://github.com/apache/hugegraph/pull/3187#discussion_r3921393601
##########
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:
🧹 This block pins down that the server was not started, but not the ordering
stated in the comment above the guard (`Validated here so a typo fails before
init-store runs`). I copied the entrypoint, moved the whole validation block
from line 101 down to just above the `./bin/start-hugegraph.sh` call so it runs
after all three `init-store.sh` call sites, and `docker-entrypoint-test.sh`
still exited 0. The invalid run executes `init-store.sh` in that variant, and
every `init-store-calls` assertion sits earlier in the file, so nothing
observes it.
Snapshotting the init-store count alongside the start count closes the gap.
Verified under bash 5.3: exit 0 against this PR, exit 1 against the relocated
guard.
```suggestion
start_calls_before_invalid_timeout=$(wc -l <
"${TEST_HOME}/docker/start-hugegraph-args")
init_calls_before_invalid_timeout=$(wc -l <
"${TEST_HOME}/docker/init-store-calls")
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}" ]]
[[ "$(wc -l < "${TEST_HOME}/docker/init-store-calls")" -eq \
"${init_calls_before_invalid_timeout}" ]]
```
##########
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:
⚠️ `:-` treats an explicitly empty value as unset, so
`HG_SERVER_STARTUP_TIMEOUT_S=` never reaches the regex below and silently
becomes 120. Driving this entrypoint with a stubbed `bin/`: empty exits 0 with
`-t 120`, while `0`, `+5` and a whitespace-only value are all rejected. That
contradicts the README text added in this PR (lines 161-163: a value that is
not a positive whole number "stops the container at startup instead of silently
falling back to the default"), and it reinstates the failure this PR exists to
fix, since a deployment that believes it set 450 still dies at 120. The repo's
own compose files use that interpolation style
(`docker/docker-compose.yml:34`), so `HG_SERVER_STARTUP_TIMEOUT_S:
${SOME_VAR:-}` yields an empty value whenever the host variable is unset.
Dropping the colon makes an empty value fail the guard while an unset one
still defaults. Verified: unset gives `-t 120`, empty exits 1, 450 gives `-t
450`, and `docker-entrypoint-test.sh` still exits 0.
```suggestion
SERVER_STARTUP_TIMEOUT_S="${HG_SERVER_STARTUP_TIMEOUT_S-120}"
```
If you would rather keep empty meaning "unset", the README sentence needs to
say so instead.
##########
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:
🧹 Section 6 directly above says `docker ps` shows real health status, but
the health budget does not move with this variable. For the `docker run`
example above, `hugegraph-server/Dockerfile:74` and
`hugegraph-server/Dockerfile-hstore:76` set `--start-period=90s --interval=15s
--retries=3`, so a container given 450s is marked `unhealthy` about two minutes
in while the entrypoint is still legitimately waiting. The compose files
replace that with their own (`docker/docker-compose.yml:41-46`: `start_period:
60s`, `interval: 10s`, `retries: 30`, so roughly 360s) and `hubble` gates on it
through `depends_on: condition: service_healthy`, so a timeout raised past that
budget blocks dependents rather than only mislabeling the container. Please add
a sentence pointing at `--health-start-period` and the compose `start_period`.
--
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]
