Skip to content

ci: retry the Maven distribution download in every job that runs mvnw - #5881

Open
andygrove wants to merge 1 commit into
apache:mainfrom
andygrove:ci/maven-bootstrap-retry
Open

ci: retry the Maven distribution download in every job that runs mvnw#5881
andygrove wants to merge 1 commit into
apache:mainfrom
andygrove:ci/maven-bootstrap-retry

Conversation

@andygrove

Copy link
Copy Markdown
Member

Which issue does this PR close?

N/A. CI reliability follow-up to #5422.

Rationale for this change

The Lint Java (Spark 4.0, JDK 21) job on #5839 (log) failed one minute in, before compiling anything:

java.io.IOException: Server returned HTTP response code: 403 for URL:
https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.6/apache-maven-3.9.6-bin.zip

#5422 added a retry and a distribution cache for exactly this failure, but only inside the shared java-test action. Every other Maven caller still does an unguarded first ./mvnw on a fresh runner: lint-java, build-spark-4-1, both benchmark verify jobs, setup-spark-builder (via make release), the three Iceberg mvnw install steps, rust-test, pyarrow-udf, delta-build-gate, and preflight's RAT check. Any of them turns a transient Maven Central 403/429 into a red job that says nothing about the patch.

What changes are included in this PR?

  • New composite action .github/actions/bootstrap-maven, holding the restore / retry / save block that used to live inline in java-test. Same behavior as before: up to four attempts with 10s/20s/40s backoff plus jitter, cache keyed on maven-wrapper.properties, cache disabled on macOS per the existing runner-images workaround.
  • setup-builder and setup-macos-builder call it as their last step (after the JDK is installed). Every job that runs ./mvnw goes through one of those two actions, so this covers all the callers listed above in one place.
  • preflight in ci.yml calls it directly before the RAT check, since that job does not use setup-builder and gates everything else.
  • java-test drops its inline copy; its callers already run a setup action first, so the distribution is installed by the time it runs.
  • dev/ci/compute-changes.py routes edits to the new action to the same jobs as the setup actions, and dev/ci/check-ci-config.py pins that with a routing case.

This is the first place a local composite action uses another local composite action (setup-builder -> bootstrap-maven); a comment in java-test that called that pattern untested is updated.

How are these changes tested?

  • python3 dev/ci/check-ci-config.py passes with the new routing case.
  • actionlint --shellcheck=off passes on the workflows.
  • Ran the retry loop from the action against a stub mvnw that fails twice then succeeds (returns 0 after three attempts, two warnings with delays in the 10-14s and 20-24s ranges) and against one that always fails (exits 1 after four attempts with the ::error:: line).
  • CI on this PR exercises the nested action call on Linux, and the bootstrap-maven/** route runs the build tier.

The retry and cache added in apache#5422 only covered callers of the java-test
action. Every other Maven caller (lint-java, build-spark-4-1, the benchmark
verify jobs, setup-spark-builder, the Iceberg installs, preflight's RAT
check) still downloads apache-maven-3.9.6-bin.zip on a cache miss with no
retry, so one 403 from Maven Central fails the job before it builds
anything.

Move the restore/retry/save block into a bootstrap-maven composite action
and call it at the end of setup-builder and setup-macos-builder, which
every ./mvnw caller runs first, plus directly from preflight. Route edits
to the new action to the same jobs as the setup actions.
@github-actions github-actions Bot added build Build environment enhancement New feature or request area:ci CI/CD, GitHub Actions, build tooling labels Sep 12, 2026

@sunchao sunchao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness

The distribution retry previously lived inside java-test, leaving lint, preflight and other Maven callers exposed to an initial download failure. This PR extracts the same three restore/bootstrap/save steps into bootstrap-maven, invokes it after Java setup in the two shared builder actions, and adds a direct preflight call before RAT. No Spark expression, operator, native implementation or build profile changes.

The extraction preserves both cache-step definitions and the retry script apart from its terminal message. The remaining java-test steps are unchanged. I traced the wrapper callers, including the nested Spark builder, Rust prerequisite and Delta gate paths. They have checkout and Java setup before bootstrap. The new action reaches the ten existing build routes, while event policy stays unchanged.

There is one P2 finding on the new preflight caller: its distribution cache cannot be saved because the copied path list includes an inaccessible root-home directory. The inline comment includes the current CI failure and the requested correction. The retry loop itself still works.

Validation

The repository CI checker passed locally. A separate YAML audit verified the extraction and caller ordering. Running the exact retry shell body against a local wrapper test double verified success on attempts one through four, and failure after four failed attempts with the expected three backoff intervals. This tests control flow and arguments, not real HTTP failures.

The current checks report 57 successes and 10 skips. I inspected the preflight, Spark 4.0/JDK 21 lint and Spark 4.1 expression-job logs. All checked out f7effb02, whose parents are the assigned base and head and whose full tree equals head 57ecfd42. Preflight runs bootstrap and passes RAT, the CI checker and actionlint. The container jobs restore Maven 3.9.6 and pass their subsequent work. The expression job reports 1,382 successful ScalaTest cases with zero failures. Preflight's green result includes the cache-save warning described above.

I ran no local Spark/native/JNI suite or benchmark. The macOS and queue-only workflow executions were skipped in this PR run. This CI-only change does not require a new Spark semantic comparison, and the unavailable maintained Spark 3.4/4.1 branches are not counted as source coverage.

Performance

The retry remains confined to --version, so it cannot repeat compilation or tests. It stops on the first success and allows at most three sleeps totaling 70–82 seconds, excluding command duration. Keeping the distribution cache independent of POM changes avoids needless invalidation. The shared setup now adds a cache/version check to native-only producer jobs too, but does not repeat a build.

The preflight cache-path finding matters here as well: the valid downloaded distribution is discarded with the runner instead of being saved for reuse. The inspected container cache hits demonstrate reuse in that environment. They do not establish a preflight cache benefit or a measured reduction in CI failure rate.

Design

Centralizing bootstrap at the shared setup boundary covers existing callers without copying a retry loop into every workflow. Running it after JDK installation preserves its prerequisite, and keeping the preflight call explicit fits that job's smaller setup. The remaining adjustment is to choose cache paths that work for both hosted preflight and container jobs, with the same selection used for restore and save.

Abstraction & complexity

A small composite action is appropriate for the repeated behavior. It adds no configurable retry framework, and java-test loses the duplicate implementation. The ten routing additions and regression case make changes to this shared action visible to the build tier. Apart from the concrete cache-path finding, I found no additional correctness, performance or complexity issue requiring a change.

Comment on lines +76 to +77
~/.m2/wrapper/dists
/root/.m2/wrapper/dists

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness

[P2] Use an accessible distribution cache path in preflight

Could we exclude the inaccessible /root/.m2/wrapper/dists path for the new non-container preflight caller? In this PR's Preflight log, Maven is installed under /home/runner/.m2/wrapper/dists, but Save Maven distribution emits EACCES: permission denied, lstat '/root/.m2/wrapper/dists' and saves no cache. The cache action aborts path enumeration on this error, so the valid user-home directory is not saved either, even though the step stays green. This leaves the pipeline's preflight gate downloading Maven again on a cache miss and misses the caching part of the reliability fix. Please select accessible distribution paths for each runner environment in both restore and save, then verify a cold save and warm restore on ubuntu-slim as well as the container path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:ci CI/CD, GitHub Actions, build tooling build Build environment enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants