github-actions[bot] commented on code in PR #65924:
URL: https://github.com/apache/doris/pull/65924#discussion_r3643577230
##########
docker/thirdparties/docker-compose/polaris/init-catalog.sh:
##########
@@ -19,22 +19,60 @@ set -eu
HOST=${POLARIS_HOST:-polaris-s3}
PORT=${POLARIS_PORT:-8181}
+ADMIN_PORT=${POLARIS_ADMIN_PORT:-8182}
USER=${POLARIS_BOOTSTRAP_USER:-root}
PASS=${POLARIS_BOOTSTRAP_PASSWORD:-secret123}
CATALOG=${POLARIS_CATALOG_NAME:-minio}
BASE_LOCATION=${CATALOG_BASE_LOCATION:-s3://warehouse/wh/}
-echo "[polaris-init] Waiting for Polaris health check at
http://$HOST:$PORT/q/health ..."
-for i in $(seq 1 120); do
- if curl -sSf "http://$HOST:8182/q/health"; >/dev/null; then
+CURL_CONNECT_TIMEOUT_SECONDS=${POLARIS_INIT_CONNECT_TIMEOUT_SECONDS:-5}
Review Comment:
These overrides are read inside `polaris-init`, but the normal startup path
launches that script in a Compose service whose `environment:` mapping forwards
none of the new `POLARIS_INIT_*` names. For example, exporting
`POLARIS_INIT_REQUEST_TIMEOUT_SECONDS=3` before `run-thirdparties-docker.sh`
still leaves the container at the default 15 seconds. Please propagate the
supported overrides through `docker-compose.yaml.tpl` (and the settings path as
needed) so the advertised configuration works in the repository's actual
entrypoint.
##########
docker/thirdparties/docker-compose/polaris/init-catalog.sh:
##########
@@ -19,22 +19,60 @@ set -eu
HOST=${POLARIS_HOST:-polaris-s3}
PORT=${POLARIS_PORT:-8181}
+ADMIN_PORT=${POLARIS_ADMIN_PORT:-8182}
USER=${POLARIS_BOOTSTRAP_USER:-root}
PASS=${POLARIS_BOOTSTRAP_PASSWORD:-secret123}
CATALOG=${POLARIS_CATALOG_NAME:-minio}
BASE_LOCATION=${CATALOG_BASE_LOCATION:-s3://warehouse/wh/}
-echo "[polaris-init] Waiting for Polaris health check at
http://$HOST:$PORT/q/health ..."
-for i in $(seq 1 120); do
- if curl -sSf "http://$HOST:8182/q/health"; >/dev/null; then
+CURL_CONNECT_TIMEOUT_SECONDS=${POLARIS_INIT_CONNECT_TIMEOUT_SECONDS:-5}
+CURL_REQUEST_TIMEOUT_SECONDS=${POLARIS_INIT_REQUEST_TIMEOUT_SECONDS:-15}
+CURL_RETRY_COUNT=${POLARIS_INIT_RETRY_COUNT:-3}
+CURL_RETRY_DELAY_SECONDS=${POLARIS_INIT_RETRY_DELAY_SECONDS:-2}
+CURL_RETRY_MAX_TIME_SECONDS=${POLARIS_INIT_RETRY_MAX_TIME_SECONDS:-45}
+HEALTH_CHECK_ATTEMPTS=${POLARIS_INIT_HEALTH_CHECK_ATTEMPTS:-60}
+HEALTH_CHECK_INTERVAL_SECONDS=${POLARIS_INIT_HEALTH_CHECK_INTERVAL_SECONDS:-2}
+HEALTH_CHECK_REQUEST_TIMEOUT_SECONDS=${POLARIS_INIT_HEALTH_CHECK_REQUEST_TIMEOUT_SECONDS:-2}
+
+# Keep every network operation bounded when an endpoint accepts a connection
but never responds.
+trap 'rc=$?; if [ "$rc" -ne 0 ]; then echo "[polaris-init] ERROR:
Initialization failed with exit code $rc." >&2; fi' 0
+
+curl_with_retry() {
+ curl -sS \
+ --connect-timeout "$CURL_CONNECT_TIMEOUT_SECONDS" \
+ --max-time "$CURL_REQUEST_TIMEOUT_SECONDS" \
+ --retry "$CURL_RETRY_COUNT" \
+ --retry-delay "$CURL_RETRY_DELAY_SECONDS" \
+ --retry-max-time "$CURL_RETRY_MAX_TIME_SECONDS" \
+ --retry-all-errors \
+ "$@"
+}
+
+echo "[polaris-init] Waiting for Polaris health check at
http://$HOST:$ADMIN_PORT/q/health ..."
+health_check_succeeded=false
+i=1
+while [ "$i" -le "$HEALTH_CHECK_ATTEMPTS" ]; do
+ if curl -sSf \
+ --connect-timeout "$CURL_CONNECT_TIMEOUT_SECONDS" \
+ --max-time "$HEALTH_CHECK_REQUEST_TIMEOUT_SECONDS" \
+ "http://$HOST:$ADMIN_PORT/q/health"; >/dev/null; then
+ health_check_succeeded=true
break
fi
- sleep 2
+ if [ "$i" -lt "$HEALTH_CHECK_ATTEMPTS" ]; then
+ sleep "$HEALTH_CHECK_INTERVAL_SECONDS"
+ fi
+ i=$((i + 1))
done
+if [ "$health_check_succeeded" != "true" ]; then
Review Comment:
This nonzero exit is not reliably observed by the repository's actual
startup path. `start_polaris` uses `docker compose up -d --wait`, and Compose
treats this no-healthcheck one-shot service as ready as soon as it is running;
the parallel launcher then returns success and never inspects it again. The
health/OAuth/API timeout can therefore fire later without failing External
Regression startup (and a very fast exit can instead race the readiness poll).
Please explicitly wait for `polaris-init` to complete and propagate its exit
code, for example with `docker compose wait polaris-init` after detached
startup, and cover delayed success and delayed failure.
##########
docker/thirdparties/docker-compose/polaris/init-catalog.sh:
##########
@@ -19,22 +19,60 @@ set -eu
HOST=${POLARIS_HOST:-polaris-s3}
PORT=${POLARIS_PORT:-8181}
+ADMIN_PORT=${POLARIS_ADMIN_PORT:-8182}
USER=${POLARIS_BOOTSTRAP_USER:-root}
PASS=${POLARIS_BOOTSTRAP_PASSWORD:-secret123}
CATALOG=${POLARIS_CATALOG_NAME:-minio}
BASE_LOCATION=${CATALOG_BASE_LOCATION:-s3://warehouse/wh/}
-echo "[polaris-init] Waiting for Polaris health check at
http://$HOST:$PORT/q/health ..."
-for i in $(seq 1 120); do
- if curl -sSf "http://$HOST:8182/q/health"; >/dev/null; then
+CURL_CONNECT_TIMEOUT_SECONDS=${POLARIS_INIT_CONNECT_TIMEOUT_SECONDS:-5}
+CURL_REQUEST_TIMEOUT_SECONDS=${POLARIS_INIT_REQUEST_TIMEOUT_SECONDS:-15}
Review Comment:
A nonempty override of `0` reaches curl unchanged, and curl defines
`--max-time 0` as disabling the transfer timeout. An endpoint that accepts the
connection and never responds then blocks forever before retries or health-loop
attempts can advance, recreating the hang this PR fixes. Please reject
non-positive values for both `POLARIS_INIT_REQUEST_TIMEOUT_SECONDS` and
`POLARIS_INIT_HEALTH_CHECK_REQUEST_TIMEOUT_SECONDS` (and add a zero-value
negative test).
##########
docker/thirdparties/docker-compose/polaris/init-catalog.sh:
##########
@@ -19,22 +19,60 @@ set -eu
HOST=${POLARIS_HOST:-polaris-s3}
PORT=${POLARIS_PORT:-8181}
+ADMIN_PORT=${POLARIS_ADMIN_PORT:-8182}
USER=${POLARIS_BOOTSTRAP_USER:-root}
PASS=${POLARIS_BOOTSTRAP_PASSWORD:-secret123}
CATALOG=${POLARIS_CATALOG_NAME:-minio}
BASE_LOCATION=${CATALOG_BASE_LOCATION:-s3://warehouse/wh/}
-echo "[polaris-init] Waiting for Polaris health check at
http://$HOST:$PORT/q/health ..."
-for i in $(seq 1 120); do
- if curl -sSf "http://$HOST:8182/q/health"; >/dev/null; then
+CURL_CONNECT_TIMEOUT_SECONDS=${POLARIS_INIT_CONNECT_TIMEOUT_SECONDS:-5}
+CURL_REQUEST_TIMEOUT_SECONDS=${POLARIS_INIT_REQUEST_TIMEOUT_SECONDS:-15}
+CURL_RETRY_COUNT=${POLARIS_INIT_RETRY_COUNT:-3}
+CURL_RETRY_DELAY_SECONDS=${POLARIS_INIT_RETRY_DELAY_SECONDS:-2}
+CURL_RETRY_MAX_TIME_SECONDS=${POLARIS_INIT_RETRY_MAX_TIME_SECONDS:-45}
+HEALTH_CHECK_ATTEMPTS=${POLARIS_INIT_HEALTH_CHECK_ATTEMPTS:-60}
+HEALTH_CHECK_INTERVAL_SECONDS=${POLARIS_INIT_HEALTH_CHECK_INTERVAL_SECONDS:-2}
+HEALTH_CHECK_REQUEST_TIMEOUT_SECONDS=${POLARIS_INIT_HEALTH_CHECK_REQUEST_TIMEOUT_SECONDS:-2}
+
+# Keep every network operation bounded when an endpoint accepts a connection
but never responds.
+trap 'rc=$?; if [ "$rc" -ne 0 ]; then echo "[polaris-init] ERROR:
Initialization failed with exit code $rc." >&2; fi' 0
+
+curl_with_retry() {
+ curl -sS \
+ --connect-timeout "$CURL_CONNECT_TIMEOUT_SECONDS" \
+ --max-time "$CURL_REQUEST_TIMEOUT_SECONDS" \
+ --retry "$CURL_RETRY_COUNT" \
+ --retry-delay "$CURL_RETRY_DELAY_SECONDS" \
+ --retry-max-time "$CURL_RETRY_MAX_TIME_SECONDS" \
+ --retry-all-errors \
+ "$@"
+}
+
+echo "[polaris-init] Waiting for Polaris health check at
http://$HOST:$ADMIN_PORT/q/health ..."
+health_check_succeeded=false
+i=1
+while [ "$i" -le "$HEALTH_CHECK_ATTEMPTS" ]; do
+ if curl -sSf \
+ --connect-timeout "$CURL_CONNECT_TIMEOUT_SECONDS" \
+ --max-time "$HEALTH_CHECK_REQUEST_TIMEOUT_SECONDS" \
+ "http://$HOST:$ADMIN_PORT/q/health"; >/dev/null; then
+ health_check_succeeded=true
break
fi
- sleep 2
+ if [ "$i" -lt "$HEALTH_CHECK_ATTEMPTS" ]; then
+ sleep "$HEALTH_CHECK_INTERVAL_SECONDS"
+ fi
+ i=$((i + 1))
done
+if [ "$health_check_succeeded" != "true" ]; then
+ echo "[polaris-init] ERROR: Polaris health check failed after
$HEALTH_CHECK_ATTEMPTS attempts." >&2
+ exit 1
+fi
+
echo "[polaris-init] Fetching OAuth token via client_credentials ..."
# Try to obtain token using correct OAuth endpoint
-TOKEN_JSON=$(curl -sS \
+TOKEN_JSON=$(curl_with_retry \
Review Comment:
`curl --retry-all-errors` cannot clear stdout between attempts. If the OAuth
endpoint emits a token body and then the transfer ends short (for example, an
overstated `Content-Length`), curl retries and appends the successful response
to the first body. The existing `sed` then yields multiple tokens, so the next
`Authorization` header is malformed even though the retry succeeded. Please
write the token response to a named file (which curl resets on retry) or clear
per-attempt output in an explicit retry loop, and cover
partial-response-then-success.
--
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]
