github-actions[bot] commented on code in PR #65924:
URL: https://github.com/apache/doris/pull/65924#discussion_r3664498864
##########
docker/thirdparties/run-thirdparties-docker.sh:
##########
@@ -1644,7 +1644,8 @@ start_polaris() {
register_stack_metadata "polaris" "${POLARIS_DIR}/docker-compose.yaml" ""
compose_cmd "${POLARIS_DIR}/docker-compose.yaml" "" down --remove-orphans
if [[ "${STOP}" -ne 1 ]]; then
- compose_cmd "${POLARIS_DIR}/docker-compose.yaml" "" up -d --wait
--remove-orphans
+ compose_cmd "${POLARIS_DIR}/docker-compose.yaml" "" up -d --wait
--remove-orphans polaris
+ compose_cmd "${POLARIS_DIR}/docker-compose.yaml" "" run --rm --no-deps
polaris-init
Review Comment:
The parallel launcher records the background `start_polaris` shell PID, but
this new foreground Compose process is its child. On a sibling startup failure,
`kill_running_jobs` signals only the recorded shell; same-shape and
exact-Compose probes left the child/one-off running after that shell exited.
Default cleanup does not rescue it: `register_stack_metadata` also runs inside
the background shell, so its array writes never reach the parent and
`cleanup_started_stacks` skips Polaris. Initialization can therefore continue
after the launcher returns failure. Please make the stack metadata
parent-visible and terminate/wait for the full component process tree (or
perform equivalent local signal/container cleanup), with a sibling-failure test
while `polaris-init` is delayed.
##########
docker/thirdparties/docker-compose/polaris/init-catalog.sh:
##########
@@ -19,25 +19,84 @@ 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}
+TOKEN_RESPONSE_FILE=/tmp/polaris-token.json
+
+# 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
+
+require_positive_integer() {
+ name=$1
+ value=$2
+ case "$value" in
+ ''|*[!0-9]*)
+ ;;
+ *)
+ if [ "$value" -gt 0 ] 2>/dev/null; then
+ return
+ fi
+ ;;
+ esac
+ echo "[polaris-init] ERROR: $name must be a positive integer, got '$value'."
>&2
+ exit 1
+}
+
+require_positive_integer POLARIS_INIT_REQUEST_TIMEOUT_SECONDS
"$CURL_REQUEST_TIMEOUT_SECONDS"
+require_positive_integer POLARIS_INIT_HEALTH_CHECK_REQUEST_TIMEOUT_SECONDS
"$HEALTH_CHECK_REQUEST_TIMEOUT_SECONDS"
+
+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" \
Review Comment:
`--retry-max-time` does not cap an already scheduled `--retry-delay`. With
the pinned curl image, an immediate refusal using `--retry-delay 10
--retry-max-time 1` was still sleeping after three seconds. Therefore
`POLARIS_INIT_RETRY_DELAY_SECONDS=3600` and
`POLARIS_INIT_RETRY_MAX_TIME_SECONDS=1` can still block every API call for an
hour, defeating the advertised total retry bound. This is distinct from the
existing BusyBox `sleep inf` thread because it is curl's finite internal retry
delay. Please validate/cap the delay against the total budget (or enforce an
explicit remaining-time loop) and add an exact-image delay-greater-than-max
test.
--
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]
