You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "sririshindra (via GitHub)" <gi...@apache.org> on 2026/10/18 13:12:37 UTC
[PR] Make site guide tests resilient to failed Spark downloads [polaris]
sririshindra opened a new pull request, #5812:
URL: https://github.com/apache/polaris/pull/5812
<!--
Describe what changes you're proposing.
-->
The `test-site-guides` CI job downloads Spark 3.5.8 through the Apache mirror redirector (`closer.lua`, which redirects to `dlcdn.apache.org`) and pipes it into `tar`. On 2026-10-18 this job failed on two consecutive runs of an unrelated PR (#5721, spec-only) with:
```
gzip: stdin: not in gzip format
tar: Child returned status 1
subprocess.CalledProcessError: Command '[tar, --extract, --gunzip, ...]' returned non-zero exit status 2.
```
Failing runs/jobs:
- https://github.com/apache/polaris/actions/runs/39126418439/job/117672395323
- https://github.com/apache/polaris/actions/runs/39128763781/job/117679387857
The same job passed on other PRs/main earlier the same day and the day before (e.g. runs 39082544721, 38989844622, 39016950497), so this looks like a transient mirror/CDN problem: the download returned a small non-gzip body (about 452 bytes) rather than the tarball. I could not reproduce it locally; the redirect and the `archive.apache.org` copy both respond normally from my machine.
Current behavior: `curl` has no `--fail`/retry, and there is a single download URL, so one bad response from the mirror fails the whole job.
Change in `site/it/site_checks/spark.py`:
- `curl` now uses `--fail --retry 3`.
- If the default download URL fails (curl error or `tar` cannot unpack it), retry once from `https://archive.apache.org/dist/spark/spark-3.5.8/spark-3.5.8-bin-hadoop3.tgz`.
- A custom URL passed via `--spark-tarball` is not given a fallback (it may be a different Spark version).
Assumptions:
- The failure is transient and mirror-related rather than a permanent change to the Spark 3.5.8 location. If 3.5.8 is eventually dropped from the mirrors, the fallback to the archive keeps working.
- No checksum verification is added, to keep the change small (the existing flow has none); `tar --gunzip` already rejects non-gzip responses.
Tested: added unit tests for the fallback and the no-fallback-for-custom-URL case; `pytest` in `site/it` passes (29 tests), `ruff format`/`ruff check` clean. The CI-only change needs no CHANGELOG or site docs update.
## Checklist
- [x] ๐ก๏ธ Don't disclose security issues! (contact security@apache.org)
- [x] ๐ Clearly explained why the changes are needed, or linked related issues: Fixes #
- [x] ๐งช Added/updated tests with good coverage, or manually tested (and explained how)
- [x] ๐ก Added comments for complex logic
- [ ] ๐งพ Updated `CHANGELOG.md` (if needed) - not needed, CI-only
- [ ] ๐ Updated documentation in `site/content/in-dev/unreleased` (if needed) - not needed, CI-only
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ci: retry Spark download in site guide tests [polaris]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode merged PR #5812:
URL: https://github.com/apache/polaris/pull/5812
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ci: retry Spark download in site guide tests [polaris]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode commented on PR #5812:
URL: https://github.com/apache/polaris/pull/5812#issuecomment-6332778448
LGTM, thanks for the fix @sririshindra.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make site guide tests resilient to failed Spark downloads [polaris]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode commented on code in PR #5812:
URL: https://github.com/apache/polaris/pull/5812#discussion_r4333585906
##########
site/it/site_checks/spark.py:
##########
@@ -43,7 +45,22 @@ def ensure_spark(
f"Spark binary {spark_sql_bin} is not present or download has been explicitly requested, "
f"downloading from {spark_tarball_url}..."
)
- curl_cmd = ["curl", "--location", spark_tarball_url]
+ urls = [spark_tarball_url]
+ if spark_tarball_url == SPARK_TGZ_URL:
+ urls.append(SPARK_TGZ_FALLBACK_URL)
Review Comment:
+ 1
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make site guide tests resilient to failed Spark downloads [polaris]
Posted by "orca-review-bot (via GitHub)" <gi...@apache.org>.
orca-review-bot commented on code in PR #5812:
URL: https://github.com/apache/polaris/pull/5812#discussion_r4331080363
##########
site/it/site_checks/spark.py:
##########
@@ -43,7 +48,25 @@ def ensure_spark(
f"Spark binary {spark_sql_bin} is not present or download has been explicitly requested, "
f"downloading from {spark_tarball_url}..."
)
- curl_cmd = ["curl", "--location", spark_tarball_url]
+ for attempt in range(1, DOWNLOAD_ATTEMPTS + 1):
+ try:
+ _download_and_extract(spark_tarball_url, spark_dir)
+ break
+ except subprocess.CalledProcessError as e:
+ if attempt == DOWNLOAD_ATTEMPTS:
+ raise
+ # Exponential backoff with jitter, so that concurrent CI jobs do not retry in lockstep.
+ delay = DOWNLOAD_BASE_DELAY_SECONDS * 2 ** (attempt - 1)
+ delay = random.uniform(delay / 2, delay)
+ print(
+ f"Downloading Spark failed ({e}), retrying in {delay:.1f}s (attempt {attempt}/{DOWNLOAD_ATTEMPTS})..."
+ )
+ time.sleep(delay)
+ print("::endgroup::")
+
+
+def _download_and_extract(url: str, spark_dir: Path) -> None:
+ curl_cmd = ["curl", "--location", "--fail", url]
Review Comment:
If the mirror stalls instead of erroring, curl just sits there and the retry loop never kicks in. The job hangs until the Actions timeout. Can we add something like `--connect-timeout 30 --speed-limit 1024 --speed-time 60` here so a stalled download fails fast and gets retried?
<sub>๐ค AI-generated review comment โ agents make mistakes, so please verify before acting on it.</sub>
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make site guide tests resilient to failed Spark downloads [polaris]
Posted by "sririshindra (via GitHub)" <gi...@apache.org>.
sririshindra commented on code in PR #5812:
URL: https://github.com/apache/polaris/pull/5812#discussion_r4320552294
##########
site/it/site_checks/spark.py:
##########
@@ -43,7 +45,22 @@ def ensure_spark(
f"Spark binary {spark_sql_bin} is not present or download has been explicitly requested, "
f"downloading from {spark_tarball_url}..."
)
- curl_cmd = ["curl", "--location", spark_tarball_url]
+ urls = [spark_tarball_url]
+ if spark_tarball_url == SPARK_TGZ_URL:
+ urls.append(SPARK_TGZ_FALLBACK_URL)
Review Comment:
Yeah, maybe retries is the best way to ago. Let me add retries with staggered exponetial backoff and hopefully that is enough to solve the transient failures. Let me update the PR.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make site guide tests resilient to failed Spark downloads [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5812:
URL: https://github.com/apache/polaris/pull/5812#discussion_r4320110635
##########
site/it/site_checks/spark.py:
##########
@@ -43,7 +45,22 @@ def ensure_spark(
f"Spark binary {spark_sql_bin} is not present or download has been explicitly requested, "
f"downloading from {spark_tarball_url}..."
)
- curl_cmd = ["curl", "--location", spark_tarball_url]
+ urls = [spark_tarball_url]
+ if spark_tarball_url == SPARK_TGZ_URL:
+ urls.append(SPARK_TGZ_FALLBACK_URL)
Review Comment:
This logic looks pretty complicated to me. Would adding retries to the curl command above not be sufficient?
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ci: retry Spark download in site guide tests [polaris]
Posted by "sririshindra (via GitHub)" <gi...@apache.org>.
sririshindra commented on code in PR #5812:
URL: https://github.com/apache/polaris/pull/5812#discussion_r4344287482
##########
site/it/site_checks/spark.py:
##########
@@ -43,7 +48,25 @@ def ensure_spark(
f"Spark binary {spark_sql_bin} is not present or download has been explicitly requested, "
f"downloading from {spark_tarball_url}..."
)
- curl_cmd = ["curl", "--location", spark_tarball_url]
+ for attempt in range(1, DOWNLOAD_ATTEMPTS + 1):
+ try:
+ _download_and_extract(spark_tarball_url, spark_dir)
+ break
+ except subprocess.CalledProcessError as e:
+ if attempt == DOWNLOAD_ATTEMPTS:
+ raise
+ # Exponential backoff with jitter, so that concurrent CI jobs do not retry in lockstep.
+ delay = DOWNLOAD_BASE_DELAY_SECONDS * 2 ** (attempt - 1)
+ delay = random.uniform(delay / 2, delay)
+ print(
+ f"Downloading Spark failed ({e}), retrying in {delay:.1f}s (attempt {attempt}/{DOWNLOAD_ATTEMPTS})..."
+ )
+ time.sleep(delay)
+ print("::endgroup::")
+
+
+def _download_and_extract(url: str, spark_dir: Path) -> None:
+ curl_cmd = ["curl", "--location", "--fail", url]
Review Comment:
Added `--connect-timeout 30 --speed-limit 1024 --speed-time 60` so a stalled download fails and gets retried.
##########
site/it/site_checks/spark.py:
##########
@@ -43,7 +48,25 @@ def ensure_spark(
f"Spark binary {spark_sql_bin} is not present or download has been explicitly requested, "
f"downloading from {spark_tarball_url}..."
)
- curl_cmd = ["curl", "--location", spark_tarball_url]
+ for attempt in range(1, DOWNLOAD_ATTEMPTS + 1):
+ try:
+ _download_and_extract(spark_tarball_url, spark_dir)
+ break
+ except subprocess.CalledProcessError as e:
+ if attempt == DOWNLOAD_ATTEMPTS:
+ raise
+ # Exponential backoff with jitter, so that concurrent CI jobs do not retry in lockstep.
+ delay = DOWNLOAD_BASE_DELAY_SECONDS * 2 ** (attempt - 1)
+ delay = random.uniform(delay / 2, delay)
+ print(
+ f"Downloading Spark failed ({e}), retrying in {delay:.1f}s (attempt {attempt}/{DOWNLOAD_ATTEMPTS})..."
+ )
+ time.sleep(delay)
+ print("::endgroup::")
Review Comment:
Good catch. The retry loop is now wrapped in `try/finally`, so `::endgroup::` is always printed. Added a test for it.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ci: retry Spark download in site guide tests [polaris]
Posted by "sririshindra (via GitHub)" <gi...@apache.org>.
sririshindra commented on PR #5812:
URL: https://github.com/apache/polaris/pull/5812#issuecomment-6332026476
Thanks for the reviews. I pushed an update:
- Added curl connect/stall timeouts so a hung mirror is retried instead of hanging the job.
- `::endgroup::` is now always printed, including when all attempts fail.
- Added a test where curl exits non-zero and the next attempt succeeds.
- Retitled the PR and dropped the empty `Fixes #`.
I did not restore the `archive.apache.org` fallback: the ASF download policy requires `closer.lua` ([link](https://infra.apache.org/release-download-pages.html#download-scripts)), and review feedback preferred keeping this simple. Retries with backoff are what remains.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make site guide tests resilient to failed Spark downloads [polaris]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode commented on code in PR #5812:
URL: https://github.com/apache/polaris/pull/5812#discussion_r4333585906
##########
site/it/site_checks/spark.py:
##########
@@ -43,7 +45,22 @@ def ensure_spark(
f"Spark binary {spark_sql_bin} is not present or download has been explicitly requested, "
f"downloading from {spark_tarball_url}..."
)
- curl_cmd = ["curl", "--location", spark_tarball_url]
+ urls = [spark_tarball_url]
+ if spark_tarball_url == SPARK_TGZ_URL:
+ urls.append(SPARK_TGZ_FALLBACK_URL)
Review Comment:
+1
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make site guide tests resilient to failed Spark downloads [polaris]
Posted by "sririshindra (via GitHub)" <gi...@apache.org>.
sririshindra commented on code in PR #5812:
URL: https://github.com/apache/polaris/pull/5812#discussion_r4320552294
##########
site/it/site_checks/spark.py:
##########
@@ -43,7 +45,22 @@ def ensure_spark(
f"Spark binary {spark_sql_bin} is not present or download has been explicitly requested, "
f"downloading from {spark_tarball_url}..."
)
- curl_cmd = ["curl", "--location", spark_tarball_url]
+ urls = [spark_tarball_url]
+ if spark_tarball_url == SPARK_TGZ_URL:
+ urls.append(SPARK_TGZ_FALLBACK_URL)
Review Comment:
Yeah, maybe retries is the best way to go. Let me add retries with staggered exponetial backoff and hopefully that is enough to solve the transient failures. Let me update the PR.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make site guide tests resilient to failed Spark downloads [polaris]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode commented on code in PR #5812:
URL: https://github.com/apache/polaris/pull/5812#discussion_r4333660537
##########
site/it/site_checks/spark.py:
##########
@@ -43,7 +48,25 @@ def ensure_spark(
f"Spark binary {spark_sql_bin} is not present or download has been explicitly requested, "
f"downloading from {spark_tarball_url}..."
)
- curl_cmd = ["curl", "--location", spark_tarball_url]
+ for attempt in range(1, DOWNLOAD_ATTEMPTS + 1):
+ try:
+ _download_and_extract(spark_tarball_url, spark_dir)
+ break
+ except subprocess.CalledProcessError as e:
+ if attempt == DOWNLOAD_ATTEMPTS:
+ raise
+ # Exponential backoff with jitter, so that concurrent CI jobs do not retry in lockstep.
+ delay = DOWNLOAD_BASE_DELAY_SECONDS * 2 ** (attempt - 1)
+ delay = random.uniform(delay / 2, delay)
+ print(
+ f"Downloading Spark failed ({e}), retrying in {delay:.1f}s (attempt {attempt}/{DOWNLOAD_ATTEMPTS})..."
+ )
+ time.sleep(delay)
+ print("::endgroup::")
Review Comment:
With line 57, if we exhausted all of the retry, it will raise and halt the script. In this case, line 65 is never print. Should we wrap this in a finally clause?
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make site guide tests resilient to failed Spark downloads [polaris]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode commented on PR #5812:
URL: https://github.com/apache/polaris/pull/5812#issuecomment-6259381976
Hello @sririshindra,
Thanks for the PR. However, I think this change is intentional: https://github.com/apache/polaris/pull/2038
Thanks,
Yong
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ci: retry Spark download in site guide tests [polaris]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode commented on PR #5812:
URL: https://github.com/apache/polaris/pull/5812#issuecomment-6461698759
Looking at https://github.com/apache/polaris/actions/runs/40397132814/job/121833012266?pr=5781, the retry doesn't appear to help.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org