Skip to content

ci: bootstrap Maven from the setup actions so every mvnw job is covered - #5881

Merged
andygrove merged 4 commits into
apache:mainfrom
andygrove:ci/maven-bootstrap-retry
Sep 17, 2026
Merged

andygrove merged 4 commits into
apache:mainfrom
andygrove:ci/maven-bootstrap-retry

Conversation

@andygrove

@andygrove andygrove commented Sep 12, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

N/A. CI reliability follow-up to #5852, which listed the remaining gaps as future work.

Rationale for this change

./mvnw downloads the Maven distribution itself on a cold runner, and a single 403 or 429 from repo.maven.apache.org fails the job before anything is compiled. The Lint Java (Spark 4.0, JDK 21) job on #5839 (log) died this way one minute in:

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 that, inside the shared java-test action. #5852 lifted it out into .github/actions/maven-bootstrap and added a step to the five jobs in pr_build_linux.yml that call ./mvnw directly, and noted as follow-up that ci.yml's RAT check and the direct ./mvnw calls in pr_benchmark_check.yml, pyarrow_udf_test.yml and iceberg_spark_test_reusable.yml still had the gap.

Adding a step per job does not converge: the list only grows, and a new job that forgets it turns a Maven Central blip into a red check that says nothing about the patch. This wires the bootstrap into the two setup actions that every Maven-running job already uses instead.

What changes are included in this PR?

  • setup-builder and setup-macos-builder call maven-bootstrap as their last step, once the JDK is installed. Every job that runs ./mvnw — directly, through make release, or through setup-spark-builder / setup-iceberg-builder — goes through one of those two, so the whole follow-up list is covered at once, rust-test, the Delta build gate and the PyArrow suite included.
  • The five now-redundant steps in pr_build_linux.yml are removed.
  • preflight in ci.yml calls the action directly, since it uses no setup action and gates every other job. (publish_snapshot.yml's deploy job already calls it directly for the same reason.)
  • java-test drops the inline restore / retry / save block it still carried; all of its callers run a setup action first.
  • The cache path is now resolved at runtime rather than listed. It used to name both ~/.m2/wrapper/dists and /root/.m2/wrapper/dists, so that one list would cover the Linux job containers, where the wrapper installs under /root while $HOME is /github/home. On a hosted runner the wrapper installs under /home/runner and /root is unreadable, and actions/cache abandons the whole save as soon as one listed path fails to stat — so preflight re-downloaded Maven on every run and saved nothing. A shell step now asks the JVM for the directory the wrapper will actually use (MAVEN_USER_HOME, else user.home, which comes from the passwd entry rather than $HOME) and passes that single path to both restore and save.
  • dev/ci/compute-changes.py routes edits to the action to the same jobs as the setup actions, and the routing case in dev/ci/check-ci-config.py is widened from the Linux build jobs to match.

The retry itself is unchanged from #5852: four attempts, 10s/20s/40s backoff plus jitter, cache keyed on maven-wrapper.properties, cache disabled on macOS per the existing runner-images workaround, with macOS still getting the retry.

setup-builder -> maven-bootstrap is the first place a local composite action calls another local composite action. The comment in java-test that called that pattern untested is updated, along with the matching note in the workflows README.

How are these changes tested?

  • python3 dev/ci/check-ci-config.py and actionlint --shellcheck=off both pass.
  • Checked mechanically that no job in any workflow runs ./mvnw, make release or java-test without either a setup action or an explicit bootstrap step, and that each of the five jobs losing its step in pr_build_linux.yml does call setup-builder.
  • Compared maven-bootstrap's membership in FILTERS against setup-builder and setup-macos-builder across all 17 outputs; it matches their union on every one. check-ci-config.py only pins the route you write down, so a case left too narrow after a widening like this one is not something it can catch.
  • Ran the retry loop verbatim against a stub mvnw: failing twice then succeeding returns 0 after three attempts (delays 14s and 24s), and always failing exits 1 after four attempts with the ::error:: line (delays 12s, 20s, 43s).
  • Ran the directory-resolution step locally: it returns the path the wrapper really installs under, and honours a MAVEN_USER_HOME override.
  • CI on this PR exercises the nested action call on Linux, and an edit under maven-bootstrap/** routes to the build tier. The macOS path runs in the merge queue.

@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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Confirmed from the preflight log: the save step stopped at the EACCES on /root and never saved. The two-path list existed because the wrapper installs under the JVM's user.home, which is /root in the containers while $HOME is /github/home. Replaced it with a step that resolves the one directory the wrapper will use (MAVEN_USER_HOME, else user.home/.m2) and passes it to both restore and save. Also rebased over #5852 so there is a single maven-bootstrap action; the five direct calls in pr_build_linux.yml are gone since setup-builder now runs it.

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.
The cache steps listed both ~/.m2/wrapper/dists and /root/.m2/wrapper/dists
so that one path list would cover the Linux job containers, where the
wrapper installs under /root while $HOME is /github/home. On a hosted
runner the wrapper installs under /home/runner and /root is unreadable, and
actions/cache abandons the whole save as soon as one listed path fails to
stat, so preflight downloaded Maven on every run and never saved it.

Resolve the one directory that applies in a shell step, the same way the
wrapper does (MAVEN_USER_HOME, else the JVM's user.home, which comes from
the passwd entry rather than $HOME), and pass it to both restore and save.
@andygrove
andygrove force-pushed the ci/maven-bootstrap-retry branch from 57ecfd4 to d45ff1f Compare September 14, 2026 15:01

@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.

Re-reviewed d45ff1f4646e106665ed7ea69153fa1124497890 against the previously reviewed 57ecfd4270e6abb450df221b225861e3fb40b9af and current base 481aefea9c60592612650de73bd2e1c7aa173979. The previous P2 cache-path finding is addressed. No new or remaining P1/P2 findings.

The action now resolves one distribution directory and uses it for both restore and save. Current preflight CI selected /home/runner/.m2/wrapper/dists, missed the cache, bootstrapped Maven 3.9.6 and successfully saved it. The container lint job selected /root/.m2/wrapper/dists, restored successfully and passed. This resolves the earlier EACCES failure. Both jobs checked out 14243862, whose entire tree equals the reviewed head.

The rebase consolidates on maven-bootstrap and removes the five redundant workflow calls. The local CI checker and caller audit passed: all 29 Maven-consuming steps retain bootstrap beforehand, with no duplicate bootstrap within a job. The retry body is unchanged from the previous review. A focused local probe compared the exact resolver shell with the tracked Maven wrapper's actual path-selection method in six cases, including hosted/container user-home values, explicit Java selection and Maven-home paths containing spaces. All matched.

Current checks show 53 successes and 10 skips. I observed a hosted cold save and container warm restores, but not a subsequent hosted warm restore or macOS execution. Local validation did not download Maven, exercise hosted cache actions or run a Spark/native build. This update adds no Spark expression/operator behavior or measured speedup claim.

GitHub currently reports merge conflicts. A rebase is needed before merge, followed by validation of the updated head.

Route edits to the Maven bootstrap composite to the Delta gate and PyArrow
UDF jobs as well, since both call setup-builder, which now runs it.

@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.

Re-reviewed 4799d935 against the previously approved d45ff1f4 and base 4479e722. The previous P2 cache-path finding remains fixed. No new or remaining P1/P2 findings.

This update merges current main, resolving the earlier merge conflict, and routes bootstrap-action edits to the Delta and PyArrow consumers. The bootstrap action, resolver/retry body, both setup actions, java-test, and Maven wrapper are byte-identical to the approved revision. Event policy matches the current base. The local CI checker and five focused routing cases passed. A static caller audit found one preceding bootstrap for each of 30 Maven-consuming steps, with no duplicate bootstrap in the 22 jobs using it.

Current preflight selected /home/runner/.m2/wrapper/dists, bootstrapped Maven 3.9.6 after a cache miss, saved it successfully, and passed the CI checker and actionlint. It checked out d5122979, whose parents are the assigned base and head and whose entire tree equals this head.

CI is still incomplete: at 18:58 UTC, there were 5 successful, 11 skipped, 18 cancelled and 1 failed checks. Required Checks failed because its build dependencies were cancelled. Those jobs provide no current-head build/test validation. I did not run a local Maven download or Spark/native build, and did not observe a current-head hosted warm restore or container/macOS execution. This remains a CI-only change with no Spark expression/operator behavior or measured speedup claim.

Conflict in dev/ci/check-ci-config.py's maven-bootstrap routing case. apache#5939
reformatted the narrow case apache#5852 added (the Linux build jobs, matching the
call sites in pr_build_linux.yml) at the same time this branch widened it.
Kept this branch's widened case: the bootstrap now runs inside setup-builder
and setup-macos-builder, so it reaches every job that runs ./mvnw, the Delta
gate and the PyArrow suite included.

@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.

Re-reviewed 75db39a2c9542e7ac9c95f8f4daedb09d5ce9465 against 8c229a703ccb024a8b5b1b849a56ceef0b66a4bd, following the prior approval. No new or remaining P1/P2 issue found. The earlier P2 cache-path issue remains fixed.

The update merges current CI routing from main. The conflict resolution retains the Maven action's full 15-route coverage, including build_linux_all_profiles. Routing policy and functions match the new base. Bootstrap, shared setup, Java-test and wrapper behavior are unchanged from the prior review. The CI checker and seven focused event-routing cases passed. A static caller audit confirms one preceding bootstrap for all 30 Maven-consuming steps.

Current preflight checked out merge b1784f150c0909fdc2f5ab0facf72794969dd7f6 (same tree as the head), installed Maven 3.9.6 under /home/runner/.m2/wrapper/dists, saved the cold cache, and passed RAT, CI configuration checks and actionlint. At 2026-09-16 15:48:58 UTC, checks were 5 successful, 13 skipped and 2 queued. Product suites remain pending. No local Maven download, Spark/native tests or benchmark was run. Earlier cancelled jobs and historical warm-cache evidence are not current-head validation.

@andygrove andygrove changed the title ci: retry the Maven distribution download in every job that runs mvnw ci: bootstrap Maven from the setup actions so every mvnw job is covered Sep 16, 2026
@andygrove

Copy link
Copy Markdown
Member Author

Triage note: #5489 was already open doing this and I missed it when I opened this PR. It extends the same Maven bootstrap to the other callers, touches four of the same files, and goes further by retrying the artifact downloads rather than just the distribution.

I have asked on #5489 which one should carry the change. Either way these should not both be reviewed, so please hold off on this one until that is settled.

@andygrove
andygrove added this pull request to the merge queue Sep 17, 2026
Merged via the queue into apache:main with commit 10f0fdd Sep 17, 2026
39 checks passed
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