ci: retry the scalafix lint when Maven cannot resolve the plugin

[andygrove]

Which issue does this PR close?

Closes #6060.

Rationale for this change

lint-java's Run scalafix check step is a single unguarded ./mvnw ... scalafix:scalafix invocation, so a Maven Central hiccup while resolving the plugin fails the job exactly the way a real lint violation does. Maven ends with No plugin found for prefix 'scalafix', and the cause only shows up in a Failed to retrieve plugin descriptor warning above it. That first happened on #6035. Between 2026-09-27 and 2026-10-10 it happened five more times, three of them on merge-queue runs (for #6065, #6541 and #6647), where a failed lint row evicts the pull request. Central answered 403 twice, 429 once (after the six resolver retries that .mvn/maven.config configures), and 404 twice.

The scalafix-syntactic job directly above already guards against this: it retries the download and then lints offline, "so a nonzero exit can only mean a lint violation, never a download failure". lint-java had no equivalent guard, and .github/actions/maven-bootstrap retries only the Maven distribution download, not resolution during the build.

The offline half of that trick does not transfer to Maven here, because the build resolves plugin and compiler artifacts as it goes, so a -o second pass would turn a cold cache into a hard failure. Retrying is the part that transfers, provided a genuine violation or compile error still fails on the first attempt. A plain retry is not enough for the 404 case, though. Maven caches a not-found result in the local repository and will not ask Central again until the update interval elapses, unless the build passes -U.

What changes are included in this PR?

• Run scalafix check retries the Maven invocation up to four times, with the same backoff as .github/actions/maven-bootstrap (10s, 20s, 40s plus 0-4s of jitter) and the same ::error:: / ::warning:: reporting.
• It retries only when one of Maven's [ERROR] lines names a download: No plugin found for prefix, could not be resolved, Could not transfer, or ClassNotFoundException: scala.tools.nsc.Main, which is how a scala-compiler jar that did not download surfaces (seen on feat: use native shuffle for row input by converting it to Arrow #6607). A lint violation, a compile error or any other failure fails on the first attempt.
• Retries pass -U, so a 404 that the failed attempt cached is checked again.
• The captured log goes to a mktemp file outside the checkout, so a passing run leaves the working tree clean for check-working-tree-clean.sh.
• The step runs under shell: bash. The container default is sh (dash), where $RANDOM is empty.
• Running out of retries reports that the failure was a download, not a lint violation.

How are these changes tested?

• The classifier was replayed over 111 Run scalafix check step logs from CI runs between 2026-09-20 and 2026-10-10. It retries all six download failures and none of the 30 compile errors or 25 lint violations. The other 50 logs passed and never reach it.
• The extracted step was run under bash --noprofile --norc -e -o pipefail, the way Actions runs shell: bash, with real Maven 3.9.6 resolving scalafix-maven-plugin through a file:// mirror with injected faults. A one-off 404 recovers on the second attempt with -U, where the earlier version of this loop failed all four attempts with was cached in the local repository. A one-off transfer error recovers on the second attempt, and a persistent 404 stops after four attempts with the download error. Replayed CI logs for a lint violation and a compile error fail after one Maven run with no sleep.
• actionlint -color --shellcheck=off, the invocation the Preflight job runs, and python3 dev/ci/check-ci-config.py pass.
• The success path is covered by the six Lint Java rows on this pull request.

[sunchao]

Correctness

The current lint step runs one Maven package plus semantic Scalafix check. This PR wraps that same command in four attempts, retains the latest output through tee, and stops immediately when the output contains Scalafix invoked with errors. The Maven profiles, check mode and SemanticDB option stay the same.

There is a blocking regression in the successful path: scalafix-check.log remains in the checkout, and the existing check-working-tree-clean.sh rejects untracked files. All four Java lint rows at merge 3f73798031d96bfa758bcb360750b1e5a3b9a2e3 print BUILD SUCCESS, then fail with ?? scalafix-check.log. Please write the log under $RUNNER_TEMP and read that same path, or remove it before the clean-tree check. The inline comment includes a current-run example.

This is a CI-only change. It does not alter Spark expressions, operators, types, null handling, ANSI errors or fallback behavior. The existing Spark 3.4, 3.5 and two Spark 4.0 lint rows remain selected. No Spark runtime equivalence or JVM execution was established by the local probes.

Validation

I ran 18 controlled shell cases using the exact extracted step across the three distinct Maven option strings. Success returns zero, a resolution failure followed by success retries, a recognized lint violation stops immediately, and persistent failures remain nonzero after four attempts. Failure codes are normalized to 1. These use Maven and sleep doubles in local Bash, not a live network or Maven build.

Current Comet CI and Required Checks are failed because of the four dirty-tree lint failures. The syntactic lint and the selected Spark 4.1 tests pass. The author reports actionlint success, which I have not rerun. The CI logs show that the actual container shell executes the Scalafix step successfully, so I am not raising a shell-startup issue.

Performance

A successful first attempt adds log capture without rerunning Maven. On failure, however, the retry condition accepts every error that lacks one Scalafix message. A deterministic Scala compilation failure also takes four attempts in the controlled test, repeating the package command three extra times and adding 70–82 seconds of scheduled backoff. The second inline finding asks to restrict this to positively identified transient resolution failures. The attempt counts are verified, but the probes do not measure JVM build time.

Design

Keeping a bounded retry beside the existing invocation is straightforward, and preserving a nonzero result after exhaustion keeps the gate closed. The two failure paths need clearer separation: expected transient resolution failures should retry, while compilation, configuration and lint failures should exit immediately. Moving the diagnostic log outside the checkout also preserves the downstream clean-tree invariant without weakening that check.

Abstraction & complexity

The loop is small and does not introduce a new reusable action or application abstraction. A temporary log path and a narrow retry classifier fit this scope. The contributor guide's formatting and build instructions remain accurate because the underlying commands and runtime behavior are unchanged.

[sunchao]

Correctness

[P1] Keep the captured log outside the checkout

Could this log be written under $RUNNER_TEMP and read from that same path? tee scalafix-check.log creates an untracked file on every successful run, and the later ./dev/ci/check-working-tree-clean.sh fails on any untracked file. All four current Java lint rows already show BUILD SUCCESS followed by Working tree is not clean: ?? scalafix-check.log (Spark 4.0/JDK 21 log). This makes Required Checks fail even when Scalafix passes.

[andygrove]

Fixed in 49ff6ce. The log now goes to a mktemp file outside the checkout, the same way the sbt retry in spark_sql_test_reusable.yml handles it, so a passing run leaves the tree clean.

[sunchao]

Performance

[P2] Retry only recognized transient resolution failures

Could the retry branch require a known transient Maven resolution error, and stop for other failures? This invocation includes package, so a deterministic Scala compilation error exits nonzero without Scalafix invoked with errors. Running the extracted step with that failure repeats the same Maven command four times and adds 70–82 seconds of backoff before returning failure. This repeats compilation for an unchanged broken input in every affected matrix row. The existing bootstrap action avoids this by retrying only its download operation. A focused case for a compiler failure alongside the resolution and lint cases would guard the distinction.

[andygrove]

Agreed, fixed in 49ff6ce. The loop now retries only when one of Maven's [ERROR] lines names a download (No plugin found for prefix, could not be resolved, Could not transfer, or the scala.tools.nsc.Main ClassNotFoundException that a missing compiler jar produces), and anything else fails on the first attempt. Replaying it over 111 Lint Java step logs from CI runs since 09-20, it retries the six download failures and none of the 30 compile errors or 25 lint violations.

[sunchao]

Summary

• Prior state and problem: A transient Maven resolution failure immediately failed the semantic Scalafix job.
• Design approach: Wrap the existing Maven command in four attempts, capture output, and stop when the recognized Scalafix error marker appears.
• Correctness / compatibility analysis: The existing P1 log-file concern remains confirmed. Successful execution leaves scalafix-check.log untracked, failing the downstream clean-tree check. Spark runtime semantics and the existing Spark 3.4, 3.5 and 4.0 lint profiles are unchanged.
• Key design decisions: The bounded inline loop adds little abstraction or complexity. Keeping the original command preserves its build and lint behavior, but also makes the retry encompass compilation.
• Implementation sketch: Execute through tee with pipefail, return on success, fail on the marker or exhausted attempts, otherwise sleep and retry.
• Behavioral changes worth calling out: The existing P2 retry-classification concern remains confirmed. A deterministic compiler failure invokes Maven four times and schedules at least 70 seconds of backoff. The successful path invokes Maven once.
• Suggested improvements: Address the existing threads by placing the log under $RUNNER_TEMP and retrying only recognized transient resolution failures. No additional introduced P1/P2 issues found within this review.

Reviewed the complete diff from d1bf687ebb13c1439057d0e7e2d15286f233c3d0 to 72244ad5ceb294364df6e2662eb4a402c508a8a8. The PR is not a draft. Read AGENTS.md, contributor guidance, and all existing reviews, comments and threads. Routed skill: review-comet-pr. No sibling skill applies to this CI-only change.

Exact-head CI: run 35516961938 has 19 successful, five failed and 14 skipped checks. All four Java lint rows show BUILD SUCCESS, then fail on ?? scalafix-check.log, making Required Checks fail. Verified that the tested merge commit has the requested base and head as parents. Preflight, including actionlint, passed.

Validation: check-ci-config.py passed. Twenty-eight disposable Bash cases across all four matrix profiles confirmed success, retry, lint failure, exhaustion and compiler-failure behavior, and reproduced the dirty-tree failure. Maven and sleep were stubbed. No local Maven/JVM build, Spark execution or actionlint run was performed. No Spark semantic source comparison was applicable to this workflow-only diff. Project files remain unchanged.

[kazuyukitanimura]

Hi @andygrove
is this still an active PR?

[sunchao]

Summary

• Prior state and problem: The semantic Scalafix step ran Maven once, so transient plugin-resolution failures immediately failed the lint gate, as described in #6060.
• Design approach: Wrap the existing Maven invocation in four attempts, capture its output with tee, and stop immediately on the recognized Scalafix error marker.
• Correctness: The existing P1 log-file concern remains confirmed. Successful execution leaves scalafix-check.log untracked, causing the downstream clean-tree check to fail. All four Java lint CI logs show BUILD SUCCESS followed by this failure. Controlled tests confirmed that Maven failures are not hidden by tee and that recognized lint errors stop retries.
• Compatibility analysis: The Maven arguments, semantic lint mode and Spark 3.4, 3.5 and 4.0 lint profiles are unchanged. The existing Spark 4.1/4.2 semantic-lint exclusions also remain unchanged. No Spark expression, operator, shim or runtime behavior changes, so comparison against Spark semantic sources is not applicable.
• Key design decisions: Retrying the complete package plus Scalafix invocation preserves rule resolution online. The four-attempt limit bounds retries, but the marker identifies only one failure category and does not distinguish transient resolution failures from deterministic build errors.
• Implementation sketch: Enable pipefail, execute Maven through tee, return on success, fail on the lint marker or fourth unsuccessful attempt, otherwise sleep and retry.
• Performance: The existing P2 retry-classification concern remains substantiated. Injecting a deterministic compiler error invokes Maven four times and schedules 70–82 seconds of backoff in Bash. A successful first attempt invokes Maven once and adds log capture. No Maven build-time benchmark was performed.
• Design: A bounded inline retry loop is straightforward for this step. Its integration currently violates the downstream clean-tree requirement, and its broad retry condition repeats work for failures that cannot recover. Both issues are already covered by existing threads.
• Abstraction & complexity: No additional action or application abstraction is introduced. A temporary log location and a narrow retry classifier fit the current structure without requiring a broader refactor.
• Behavioral changes worth calling out: Non-marker failures now retry, terminal failures return status 1, and diagnostics distinguish recognized lint errors from exhausted retries. The generated log currently makes otherwise successful Java lint jobs fail. Product behavior and configuration remain unchanged.
• Suggested improvements: Address the existing threads by writing and reading the log under $RUNNER_TEMP and retrying only recognized transient resolution failures. No additional introduced P1/P2 issues found within this review.

Reviewed the entire base-relative diff from d1bf687ebb13c1439057d0e7e2d15286f233c3d0 to 72244ad5ceb294364df6e2662eb4a402c508a8a8: one commit and one workflow file. Confirmed the PR is not a draft. Read AGENTS.md, contributor guidance, existing reviews, issue comments, inline comments and review threads. Routed skill: review-comet-pr. No sibling skill applies to this CI-only change.

Exact-head CI: 19 checks passed, five failed and 14 were skipped. In Comet CI run 35516961938, all four Java lint rows failed on the untracked log, which also failed Required Checks. Preflight, including actionlint, passed. Verified that tested merge 3f73798031d96bfa758bcb360750b1e5a3b9a2e3 has the requested base and head as parents.

Validation: python3 dev/ci/check-ci-config.py passed. Twenty-eight disposable Bash cases using the four matrix rows’ Maven arguments verified success, recovery, lint failure, exhaustion and compiler-failure behavior. All 12 successful cases reproduced the downstream dirty-tree failure. Maven and sleep were stubbed, so these tests establish control flow rather than real network recovery or JVM execution. No local Maven/JVM build, Spark suite or actionlint run was performed. Project files and GitHub state remain unchanged.

[andygrove]

@ErikBPF you have made several CI changes (#6099, #6112, #6113). If you have time, would you be able to review this scalafix retry change? Thanks.

[ErikBPF]

Sure, Andy. I reviewed the change and put a runnable demonstration on a branch of my fork, based on this PR's head. Instructions are here.

The reproducer uses Maven 3.9.6 against a local HTTP repository returning 503. For both Scala 2.12 and 2.13, the short scalafix: goal hides the transfer cause behind No plugin found for prefix 'scalafix'. Fully qualified goals expose the HTTP 503 while retaining the version from the POM's plugin management. All four cases passed on JDK17 and JDK21.

This matters when narrowing the retry condition to recognized transient transfer errors. My recommendations are:

• Keep the captured log under $RUNNER_TEMP, as @sunchao suggested, so successful runs pass the checkout-cleanliness check.
• Retry only recognized transient resolution failures. Compiler and Scalafix errors should fail immediately.
• Use fully qualified plugin goals so the retry classifier can see the underlying transfer error.
• Declare shell: bash, since container steps default to sh and the loop uses Bash features.

The published branch contains the reproducer and instructions. If this approach looks good to you, I can follow up with the workflow patch and regression coverage.

[andygrove]

Thanks @ErikBPF, the reproducer helped. Since the PR was already open I pushed the workflow change myself in 49ff6ce, so no need for a follow-up patch. It writes the log to a mktemp file, retries only when Maven's error names a download, and runs under shell: bash. You were right about the shell. The step ran under sh -e {0}, so $RANDOM was empty and the jitter was always zero.

I kept the short scalafix: goal and match No plugin found for prefix directly. The root POM declares the plugin, so in this job that error only means the descriptor did not download, and Maven still logs the HTTP cause in the Failed to retrieve plugin descriptor warning just above it. A qualified goal would need the Scala suffix for each matrix row, and it would still miss a 404, which Maven reports as Could not find artifact rather than Could not transfer artifact.

The 404 case turned out to matter. Since this PR was opened the lint has failed this way five more times: 403s on the merge-queue runs for #6065 and #6541, a 429 on the merge-queue run for #6647 after the resolver's own six retries, and 404s on two PR runs. Maven caches a 404 in the local repository, so the original loop would have failed all four attempts on those. I confirmed that with Maven 3.9.6 against a file:// mirror: a plain rerun fails with "was cached in the local repository" and a rerun with -U succeeds, so the retries now pass -U.

@kazuyukitanimura yes, still active. Those three merge-queue evictions are why.