bitflicker64 commented on PR #3187:
URL: https://github.com/apache/hugegraph/pull/3187#issuecomment-5560515947
@imbajin Ready for a review pass when you have time. Pushed `585b099` and
`7e2818c`, both docs and test-harness only.
**Why those two commits.** The `### Server startup timeout` section in
`docker/README.md` demonstrated `HG_SERVER_STARTUP_TIMEOUT_S=450 docker compose
up -d`, and 450 is past the budget that command actually has to clear. The
server health check in `docker/docker-compose.yml` and
`docker-compose-hstore.yml` is `start_period: 60s` plus `retries: 30` at
`interval: 10s`, roughly 360 seconds, and hubble carries `depends_on: server:
condition: service_healthy` in both files. `docker compose up -d` honors
`service_healthy` without `--wait`, so on the slow host that example is written
for, the pasted command exits non-zero with hubble unstarted. The caveat two
sentences below named only `up -d --wait`, which reads as safe to anyone not
passing that flag.
`585b099` lowers the example to 300 and reworks the caveat to name the
`depends_on: service_healthy` path as well, so a reader who genuinely needs
more than 360 seconds is still told what to change. It also gives
`compose_active` in `docker/test-compose.sh` the same `env -u
HG_SERVER_STARTUP_TIMEOUT_S` that `compose_auth` already had, so a developer
with the variable exported empty does not get server containers exiting on the
new guard during `test-compose.sh smoke` with the crash looking unrelated to
their shell. CI was unaffected either way, since the variable is unset there.
`7e2818c` is one token. `hugegraph-server/hugegraph-dist/docker/README.md`
showed the same Compose command at 450, so the two READMEs contradicted each
other; it is now 300 in both. The `docker run` example in that file stays at
450 deliberately: `docker run` does not block on health, the image defaults are
`--interval=15s --start-period=90s --retries=3`, and the section already tells
the reader the container reports unhealthy around 135 seconds and to raise
`--health-start-period`.
**What I checked on the branch.** The validation matrix against the real
entrypoint: unset, empty, `" 300 "` with spaces, `+300`, `0300`, `300s`, `1e3`,
`-1`, `0`, `86401`, the 64-bit max, and a trailing newline. Each behaves as
documented and the entrypoint refuses loudly rather than proceeding on a
garbage value. A trailing newline is rejected because bash ERE `$` is end of
string, not end of line.
`(( ))` evaluates variable contents recursively, so I went looking for an
expression that could reach it. The guard is `[[ ! ... =~ ^[1-9][0-9]{0,4}$ ]]
|| (( ... ))`, which short-circuits, so the arithmetic only ever runs on an
already-matched plain integer. On the Compose side the unset-only `-` default
means an explicitly empty value still reaches the container and is refused
there instead of being dropped as a YAML null, and nothing lets the value
inject into the rendered config.
Ordering holds: the guard is at `docker-entrypoint.sh:114-122`, the first
`set_prop` at 149, `init-store.sh` at 219, so a bad value mutates nothing. A
typo produces a clear refusal on every restart rather than a quiet one, and
that is still better than the old behaviour, where a slow host let the server
die and loop at 120s with nothing explaining why. `docker-entrypoint-test.sh`
fails with `expected start-hugegraph.sh -t 450, got -t 120` when the entrypoint
is reverted to the merge base, so it catches the regression it exists for.
**One nit I left alone.** The regex admits up to 99999 and a second check
refuses anything over 86400, so the bounds live in two places. The error
message is correct either way and collapsing them would churn the tests, so it
did not seem worth it. Happy to do it if you would rather have one check.
--
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]
