bitflicker64 commented on code in PR #3105:
URL: https://github.com/apache/hugegraph/pull/3105#discussion_r3664130168
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/util.sh:
##########
@@ -185,8 +344,17 @@ function wait_for_startup() {
return 1
fi
- status=$(curl -I -sS -k -w "%{http_code}" -o /dev/null "$server_url"
2> "$error_file_name")
- if [[ $status -eq 200 || $status -eq 401 ]]; then
+ # Bound each probe by the time left in the overall deadline: without
+ # --max-time a single blackholed request blocks past ${timeout_s}s.
+ # TODO(wait_for_startup): overshoot is now bounded but not zero - the
+ # loop still sleeps 2s after a probe and only then re-reads the clock,
+ # so the total can exceed ${timeout_s}s by roughly one sleep interval.
+ local remain_s=$((stop_s - now_s))
+ [ "$remain_s" -lt 1 ] && remain_s=1
+ local connect_s=$((remain_s < 5 ? remain_s : 5))
+ status=$(curl -I -sS -k --connect-timeout "$connect_s" --max-time
"$remain_s" \
Review Comment:
Confirmed. Bounding each request bounds the request, not the loop — the
pause was a flat `sleep 2` taken before the clock was re-read, so `timeout_s=1`
ran for about 2s. I had noted the overshoot in a `TODO(wait_for_startup)` and
left it; you are right that documenting it is not the same as honouring the
timeout. Fixed in `d93b6f9d`:
```bash
now_s=$(date '+%s')
local sleep_s=$((stop_s - now_s))
[ "$sleep_s" -le 0 ] && break
[ "$sleep_s" -gt 2 ] && sleep_s=2
sleep "$sleep_s"
now_s=$(date '+%s')
```
The `break` matters as much as the cap: with the deadline spent, falling
through to the `while` guard on a whole-second clock would spin on repeated
instant probes until the second ticked over.
On the assertion — you asked for elapsed time, and the suite now checks it,
but the load-bearing check is the other one. Wall-clock at `date +%s`
resolution cannot separate a 1s run from a 2s run reliably, so both are
asserted: `sleep` is shimmed to record what was requested and then take it for
real, so the duration stays a real measurement.
```
PASS retry sleeps stay inside the 1s deadline
PASS wait_for_startup returns within the deadline (1s)
```
Total requested sleep must be `<= timeout_s`, which is exact — restoring
`sleep 2` fails it every time, while the elapsed check alone would be a coin
flip on the second boundary. Ran it five times, 1s each time.
`wait_for_shutdown` has the same fixed-pause-then-check shape and the same
overrun. Left alone with a `TODO(wait_for_shutdown)` pointing at this fix,
since it is untouched by this PR and widening the diff is the thing you asked
me not to do. Happy to fix it here if you would rather.
--
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]