Skip to content

chore: Improve network retry configuration for maven and artifact upload - #5782

Open
comphead wants to merge 6 commits into
apache:mainfrom
comphead:chore_ci
Open

chore: Improve network retry configuration for maven and artifact upload#5782
comphead wants to merge 6 commits into
apache:mainfrom
comphead:chore_ci

Conversation

@comphead

@comphead comphead commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Part of #5489

CI hardening, prompted by a native-lib-linux upload failing with
Failed to FinalizeArtifact: Received non-retryable error: Failed request: (403) Forbidden: Error from intermediary.

Rationale for this change

Two independent network flakes in CI, neither of them retried today.

Artifact upload. @actions/artifact retries only 429/500/502/503/504, so
the 403 above is classified non-retryable and fails the job even though the blob
content and SHA-256 already landed. No input or env var widens that list, and
Actions has no built-in step retry.

Maven. Maven 3.9.6's default resolver transport retries 3 times and treats
only 429/503 as retryable, so a 502 or 504 from the CDN in front of Maven
Central fails a build on first hit.

What changes are included in this PR?

  • .mvn/maven.config (new): retries 3 -> 6, retryable statuses
    429,503 -> 408,429,500,502,503,504, connect timeout 10s -> 30s, socket read
    timeout 30min -> 10min. The wrapper pins maven.multiModuleProjectDirectory
    to the directory holding .mvn, so this one file covers every mvnw
    invocation in CI (including cd spark && ../mvnw ...) with no per-workflow
    wiring. Command-line flags still win.
  • .github/actions/upload-artifact-retry (new): composite wrapper around
    actions/upload-artifact@v7 with the same inputs and outputs, three attempts,
    15s then 45s backoff. Retries force overwrite: true in case attempt 1 left a
    server-side record behind. Wired into the six upload sites a later job
    consumes. The three diagnostic uploads in java-test stay on the plain action
    (a local action calling another local action is untested here, and they run
    only on already-failing jobs).
  • Per-producer artifact names (review follow-up): artifact names are scoped
    to the run, not to the calling workflow, so ci.yml's four invocations of
    spark_sql_test_reusable.yml all published native-lib-linux and its four
    invocations of iceberg_spark_test_reusable.yml all published
    native-lib-iceberg. A consumer's download-artifact resolved the name to
    the highest artifact ID rather than to the producer in its needs, and the
    forced overwrite above could delete a sibling's finished record. Both now
    qualify the name with their version inputs
    (native-lib-spark-<full>-jdk<N>,
    native-lib-iceberg-<iceberg-full>-spark-<spark-full>-jdk<N>), matching the
    existing jvm-compiled-spark-<full>-jdk<N> convention. native-lib-linux
    and native-lib-macos keep their names: ci.yml calls pr_build_linux.yml
    and pr_build_macos.yml once each.
  • dev/ci/compute-changes.py (review follow-up): registers the paths the
    gated jobs actually read. .github/actions/upload-artifact-retry/** was in no
    filter, so a later edit touching only action.yaml would compute every output
    false and skip every heavy job. .mvn/** and mvnw were registered only
    under build_linux / build_macos, but setup-spark-builder and the three
    Iceberg jobs also run ./mvnw install. benchmark and docs are left alone:
    benchmark is the complement of the bench-path exclusions in build_linux,
    and docs.yaml runs no Maven.
  • dev/ci/check-ci-config.py (new, run from preflight): pins the routing
    table for the shared build inputs above, and enforces that a reusable workflow
    ci.yml calls more than once qualifies its artifact names with an input and
    that every download-artifact name is produced in the same workflow. Both
    failure modes are silent otherwise.
  • compression-level: 0 on the ~1 GB apache-spark.tar.gz upload. It is
    already gzipped, so level 6 burns minutes of CPU for no size win, and a shorter
    upload is a smaller window for the same flake. Happy to drop this if it reads
    as out of scope.
  • runs-on: ubuntu-latest -> ubuntu-24.04 (review follow-up) in the last
    three jobs on the floating label: pr_build_macos.yml lint, docs.yaml
    build-docs, pyarrow_udf_test.yml pyarrow-udf. ubuntu-latest jobs were
    not getting runners at all: three that eventually started waited 3h24m, 3h34m
    and 4h08m, while ubuntu-24.04 jobs in the same minutes were picked up in 6
    to 24 seconds. That was 3 of 34 jobs, against 21 already on ubuntu-24.04 and
    8 on ubuntu-slim.
  • .github/workflows/README.md: short section covering the above.

The Wagon transport (-Dmaven.resolver.transport=wagon) was evaluated first and
rejected: deprecated in Resolver 1.9 and removed in Maven 4, its retry knobs
mirror the native transport's, and its service-unavailable retry strategy
defaults to none, so adopting it would start by losing the 429/503 retry we
already have. Full comparison in the README.

How are these changes tested?

Verified locally against the pinned toolchain (Maven 3.9.6 / Resolver 1.9.18):

  • .mvn/maven.config is parsed by ./mvnw and the values reach Maven
    (help:evaluate returns 6 and the status list).
  • A real dependency:get from Maven Central succeeds with the config live, which
    exercises HTTP transporter construction (a malformed status list throws there).
  • apache-rat:check, actionlint, prettier --check, check-suites.py and
    check-benchmark-runner.py all pass.
  • check-ci-config.py passes, and was checked by fault injection: deleting the
    new filter entries reports 32 routing failures, and reverting the Spark
    producer name to native-lib-linux reports both the shared name and the
    orphaned consumer.

The retry path itself only runs on a real flake, so CI here should show attempt 1
succeeding with attempts 2 and 3 skipped.

@github-actions github-actions Bot added enhancement New feature or request area:Iceberg labels Sep 8, 2026
@comphead comphead added area:ci CI/CD, GitHub Actions, build tooling and removed area:Iceberg labels Sep 9, 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

Reviewed head b9c95c2a3db6d7e040076161d60396361bcaa123 against base fefee03d94045ecd0ac5d3a1edb98a555f5ff21d.

This addresses two CI failure paths: Maven repository requests that fail without retrying additional transient HTTP statuses, and artifact uploads that fail during finalization after sending their content. The change adds repository-wide Maven Resolver settings and a local composite action with three upload attempts. It also updates the Java diagnostic uploads to upload-artifact v7.

The pinned Maven 3.9.6 uses Resolver 1.9.18, which supports all four properties. The configuration expands retryable statuses, raises the retry count to six, sets the connection timeout to 30 seconds and reduces the idle read timeout to ten minutes. The wrapper locates the repository's .mvn directory when invoked from a subdirectory. Maven's parser accepts these comment lines and gives explicit command-line properties precedence.

One P2 remains in the artifact retry path: parallel native-library producers share artifact names. Forced overwrite can select and delete another producer's completed artifact. The inline comment describes the interleaving and the required producer/download naming change. The attempt conditions otherwise preserve failure outcomes for deciding retries, and the final attempt propagates failure.

No Spark expression, operator, type, null, overflow or fallback semantics change in this CI-only contribution. This review makes no new Spark compatibility claim.

Validation

At 03:52 UTC on September 9, current-head CI has 65 successful and 23 skipped checks, with no failed or pending checks. Four workflows succeeded and one was skipped.

CI run 34287334485 checked out merge commit ad2710dc75ddd7a5378e75ae7477b98af10f5418, whose parents are the reviewed base and head. All eight changed files match the head exactly. Logs from seven selected jobs confirm successful Linux, macOS, Iceberg and Spark artifact uploads, Maven execution and Java report upload. The sampled composite uploads succeeded on attempt one, with attempts two and three skipped. An offline diagnostic executed the exact v7 delete/get selector functions against an in-memory service and confirmed selection of another producer's artifact, including when the retrying producer has an older record. No GitHub failure was injected, and the successful run does not validate retry recovery or deletion timing.

Performance

The normal path adds no retry delay. Failure recovery can perform up to three complete uploads and add 15- and 45-second waits, on top of the artifact client's internal retries. Maven retries also increase work during an outage, while the smaller idle timeout limits a stalled connection. Disabling ZIP compression for the already-gzipped Spark archive avoids redundant compression. The successful upload logs establish execution, but there is no matched measurement supporting a particular speedup. An engine microbenchmark would not test these changes.

Design

A single Maven configuration file keeps the transport settings consistent across wrapper invocations. The composite action retries the upload step without replaying builds or tests, and its final failure remains visible. The resource ownership issue needs to be addressed before this wrapper is safe for shared native-artifact names: each producer and its consumers need a stable, distinct name so cleanup cannot select a peer's completed output.

Abstraction & complexity

The wrapper earns its place by serving six upload sites with one policy. Its three explicit attempts are straightforward to trace, and the inputs, boolean defaults and output wiring match the v7 action. Outputs are populated after successful finalization, so the first nonempty attempt output identifies the completed upload. No broader retry framework is needed for this scope.

if-no-files-found: ${{ inputs.if-no-files-found }}
retention-days: ${{ inputs.retention-days }}
compression-level: ${{ inputs.compression-level }}
overwrite: 'true'

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] Give parallel producers distinct artifact names before retrying with overwrite

ci.yml runs the Linux native build and the Spark 3.5/4.1 builds in parallel, and all three upload native-lib-linux. The v7 delete selector selects the newest artifact with that name and sends its producer's job ID to DeleteArtifact. The pinned action documentation also explicitly supports overwriting an artifact from a different job. If one producer finishes while another fails before creating its artifact, this retry can delete the completed producer's artifact. Consumers depend only on their own producer and can then fail lookup during the replacement upload, or if that upload also fails. Please make the producer and download names unique per workflow/version before enabling forced overwrite. The parallel Iceberg versions also share native-lib-iceberg when those jobs are enabled.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 5db3fc7. Each producer now qualifies the name with its version inputs, and the consumers follow:

  • spark_sql_test_reusable.yml: native-lib-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }}, matching the jvm-compiled-spark-<full>-jdk<N> the same job already publishes.
  • iceberg_spark_test_reusable.yml: native-lib-iceberg-${{ inputs.iceberg-full }}-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }}.

native-lib-linux in pr_build_linux.yml and native-lib-macos in pr_build_macos.yml keep their bare names: ci.yml calls each of those workflows exactly once, so after the rename each has a single producer in the run.

Since a shared name is invisible until a retry lands on it, the invariant is now enforced rather than just fixed. dev/ci/check-ci-config.py (new, run from preflight) fails when a reusable workflow that ci.yml calls more than once uploads an artifact whose name carries no inputs. reference, and when a download-artifact name is not produced by an upload in the same workflow. I checked it by fault injection: reverting just the Spark producer back to native-lib-linux reports both the shared name and the now-orphaned consumer.

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

I agree with @sunchao's inline point about forcing overwrite: true on the retry attempts in .github/actions/upload-artifact-retry/action.yaml, and the collision is not hypothetical. Run 34287334485 on this PR produced three separate artifacts all named native-lib-linux in a single run, because ci.yml runs pr_build_linux.yml and two invocations of spark_sql_test_reusable.yml as jobs of the same run and each uploads under that name. A retry in any one of them would delete whichever record is newest, which is more likely than not a sibling's. Can the producer and consumer names be made unique as part of this change? That also clears up the situation we already have, where each download-artifact@v8 step resolves native-lib-linux to one of three records with nothing tying it to its own producer. The three happen to be byte identical today, but nothing enforces that.

The other thing I would like settled before merge is dev/ci/compute-changes.py. No filter list there mentions .github/actions/upload-artifact-retry/**, so a later PR that edits only action.yaml computes every output false and skips pr_build_linux, pr_build_macos, the Spark SQL workflows and the Iceberg workflows. The action would land with only actionlint having looked at it. While you are in that file, .mvn/** appears only under build_linux and build_macos, but .github/actions/setup-spark-builder/action.yaml runs ./mvnw install inside the Spark and Iceberg jobs, so those filters likely want it too.

The rest holds up for me. The wrapper's inputs match actions/upload-artifact@v7 one for one including archive, the empty retention-days and compression-level defaults fall through to the same values the action itself uses, and the four aether.connector.* keys are the right Resolver 1.9 names for the Maven 3.9.6 that .mvn/wrapper/maven-wrapper.properties pins. CI is green on the current head, though as you say nothing in it exercises the retry path.

@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

Rechecked unchanged head b9c95c2a3db6d7e040076161d60396361bcaa123 against assigned base fefee03d94045ecd0ac5d3a1edb98a555f5ff21d, including the new review. [P2] The existing finding remains unresolved. I have no additional independent P1/P2 finding to post inline.

The duplicate names are confirmed in run 34287334485: the Spark 4.1, Spark 3.5 and Linux native producers created separate native-lib-linux records, IDs 10080903140, 10080907170 and 10080918964. Producer log receipts match the fresh artifact/job metadata. The inspected upload v7 commit is 043fb46d, using @actions/artifact 6.2.0. Its overwrite path can select another job's same-named artifact. The inspected download v8 commit is 3e5f45b2, using 6.2.1, and named downloads select the highest matching artifact ID rather than the producer identified by a workflow's needs. The action commits match the CI receipts. This supports the existing producer/consumer naming request without another inline comment. The timing of a retry determines which record is selected. Record count alone does not establish a probability of selecting a sibling.

One qualification to the new review: the three archived artifacts have equal reported sizes but different archive digests. I did not download their contents, so I cannot verify whether the extracted native libraries are byte-identical. Neither equal size nor different ZIP digests settles that question. No deletion or failed-retry recovery was observed in this successful run.

The change-filter request is also supported. Six bounded offline invocations of the exact compute-changes.py CLI passed their expected checks. An edit only to .github/actions/upload-artifact-retry/action.yaml returns all twelve outputs false. An edit only to .mvn/maven.config or the wrapper properties enables Linux/macOS builds but leaves every Spark SQL and Iceberg output false. The actual multi-file contribution enables those routes because it also changes reusable workflow files, so green CI here does not cover a future isolated action/config edit. Preflight still runs. Manual dispatch forces the outputs true, while label gates still require the path output to be true.

The Maven call path needs a small clarification: Spark SQL uses setup-spark-builder with skip-native-build: true, which runs the repository wrapper. The three Iceberg test jobs invoke ./mvnw install directly before setup-iceberg-builder. Both consume the repository configuration. Extending their filters to cover .mvn/** is therefore supported despite the different callers. No Spark expression, operator, null, overflow or fallback semantics are changed or newly qualified by this CI-only follow-up.

At the September 9, 12:38 UTC refresh, checks remained 65 successful and 23 skipped. Four workflows succeeded and one was skipped. The successful CI checkout was ad2710dc, whose parents are the assigned base and head. All eight authored files and the additional filter/setup/wrapper sources match the reviewed head. At that refresh, the synthetic merge was 66f42fc8, with newer main parent 79d1be1e. I have no execution evidence for that newer merge. Selected uploads in the retained receipts succeeded on attempt one and skipped later attempts. Local validation covered only filter routing. No artifact client, Maven build, product test or retry scenario was executed in this follow-up.

Performance

The filter update should target the existing producer/consumer workflows rather than make every repository change run every suite. Adding the retry-action path to the relevant Linux/macOS, Spark SQL and Iceberg filters, and .mvn/** to the Spark SQL/Iceberg filters, gives later edits execution coverage while preserving unrelated path exclusions and event/label gates.

The green run adds no measurement of retry cost or compression savings beyond the earlier review. The scoped routing checks are not performance benchmarks. I have no new measured regression or reason to request an engine microbenchmark for these CI changes.

Design

The existing naming fix needs to cover both producers and their downloads, including versioned invocations of the reusable Spark and Iceberg workflows. A distinct, stable artifact identity per producer addresses the ownership problem while retaining parallel builds. Merely restricting job dependencies does not disambiguate a run-wide name lookup. Passing a producer's artifact ID can make a consumer's selection explicit, but it does not by itself prevent another producer's forced overwrite from deleting that record. Producer isolation remains necessary.

The reviewed v7 inputs and Resolver properties remain supported by their pinned sources. Maven 3.9.6's POM selects Resolver 1.9.18, whose configuration constants and HTTP transporter consume the four properties used here. No alternative or guessed Resolver key is needed to address the filter gap.

Abstraction & complexity

The follow-up does not call for a broader retry framework. The actionable integration work is the existing producer/consumer naming request and the already-raised filter registration. Keeping those path dependencies in FILTERS follows the current CI design. A small regression check for action-only and Maven-config-only file lists would preserve the routing contract demonstrated by the offline checks.

…ange filters

Addresses review feedback on apache#5782.

Artifact names are scoped to the run, not to the calling workflow, so
ci.yml's four invocations of spark_sql_test_reusable.yml all published
`native-lib-linux` and its four invocations of
iceberg_spark_test_reusable.yml all published `native-lib-iceberg`. A
consumer's `download-artifact` then resolved the name to the highest
artifact ID rather than to the producer in its `needs`, and the forced
`overwrite` on an upload retry could delete a sibling's finished record.
Both producers now qualify the name with their version inputs, matching
the existing `jvm-compiled-spark-<full>-jdk<N>` convention, and the
consumers follow.

dev/ci/compute-changes.py gains the paths the Spark SQL and Iceberg jobs
actually read: `.github/actions/upload-artifact-retry/**` (missing from
every filter, so an action-only edit ran nothing), plus `.mvn/**` and
`mvnw`, which setup-spark-builder and the Iceberg jobs invoke.

dev/ci/check-ci-config.py, new and run from preflight, pins both: a
routing table over the shared build inputs, and the rule that a reusable
workflow ci.yml calls more than once must qualify its artifact names and
that every download name is produced in the same workflow.
@comphead

comphead commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @sunchao @andygrove. Both items are addressed in 5db3fc7.

Artifact naming. Replied inline on the P2 thread. Short version: the two reusable workflows ci.yml calls repeatedly now version-qualify their native-lib artifacts (native-lib-spark-<full>-jdk<N>, native-lib-iceberg-<iceberg-full>-spark-<spark-full>-jdk<N>), consumers updated to match. native-lib-linux and native-lib-macos stay as they are, since pr_build_linux.yml and pr_build_macos.yml are each called once. That also clears the pre-existing ambiguity @andygrove noted, where a download-artifact@v8 step resolved native-lib-linux to whichever of three records had the highest ID.

dev/ci/compute-changes.py. Added to the filters:

  • .github/actions/upload-artifact-retry/** to build_linux, build_macos, all four spark_* and all four iceberg_*. It was in no filter at all, so an edit touching only action.yaml computed every output false.
  • .mvn/** to the spark_* and iceberg_* filters. setup-spark-builder runs ./mvnw install for the Spark SQL jobs and the three Iceberg jobs call ./mvnw install directly, so both consume the repository config.
  • mvnw alongside it in those same eight filters. Same defect, same one-line fix: the wrapper script itself was only registered under build_linux / build_macos. Say the word if you would rather keep this PR to exactly the two paths you named and I will drop it.

I deliberately left benchmark and docs alone. benchmark is the complement of the bench-path exclusions in build_linux, so widening it to .mvn/** would make every Maven config change run the benchmark check, and docs.yaml runs no Maven.

Regression check. Picking up @sunchao's suggestion, dev/ci/check-ci-config.py is new and runs from preflight. It holds two things:

  1. A routing table asserting the exact set of outputs compute-changes.py must report true for the shared build inputs (.mvn/maven.config, .mvn/wrapper/maven-wrapper.properties, mvnw, the retry action), plus two spot checks that the additions did not widen the docs and benchmark routes.
  2. The artifact-name invariant described in the inline reply.

Both halves were verified by fault injection against this working tree: deleting the new filter entries reports 32 routing failures, and reverting the producer rename reports the shared name and the orphaned consumer. .github/workflows/README.md gains a section explaining why the names are version-qualified, since the failure mode is silent.

actionlint, apache-rat:check, prettier --check, check-suites.py and check-benchmark-runner.py all pass locally on the new head. As before, nothing in a green run exercises the retry path itself.

@comphead

comphead commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Rebase note: main had moved to 424c31a and the PR went CONFLICTING, which is why no CI run appeared for 5db3fc7 (GitHub cannot build the merge commit, so pull_request workflows are never scheduled; only the pull_request_target labeler ran). Merged main in at 50c7b53. The only conflict was in ci.yml, where #5762 added a Check pull request type labeling step in the same place as the new Check CI config invariants step. Both are kept.

Preflight is green on the merged head, with Check CI config invariants passing in real CI: run 34370771074. The Spark SQL and Iceberg jobs are still running and are what actually exercises the renamed artifacts end to end.

@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

Re-reviewed 50c7b53f62f08c3a2d11d07d3bd1be4ab630259e against 424c31aa79d13fddf743ffa29bae3c6f146e6c5e, including the seven-file fix since b9c95c2a. The previous P2 is addressed: Spark and Iceberg native artifacts now include their version inputs, and all named consumers match their producers. Linux/macOS each retain one native producer. Independent expansion of the current callers found 46 distinct explicit artifact names, including the optional fallback matrix, with no unmatched named consumer.

The shared retry action, Maven config and wrapper now activate the ten intended build/Spark/Iceberg filters. The exact checker and eight isolated/positive/negative routing cases passed locally. Restoring the old filters and reverting the Spark producer name were both detected. This is offline configuration validation. The textual guard is not a general proof of uniqueness for future input combinations.

CI run 34370771074 checked out 03d6a3c2, whose complete tree matches this head. Both preflight checks survived the merge and passed. Completed Spark 3.5/4.1 catalyst and Iceberg 1.11 runtime jobs downloaded the exact native artifact IDs from their intended producers. The two Spark JVM artifacts also matched. At the September 9, 17:14 UTC refresh, checks were 50 successful, 7 skipped, 9 running and 1 queued. Full CI remains incomplete, and these first-attempt successes do not demonstrate retry recovery.

No new or remaining P1/P2 found. The authored change is confined to CI/build configuration. Spark expression/operator semantics, including types, nulls, overflow, errors and fallback, are unchanged relative to base. No new engine-compatibility claim is made. Maintained Spark 3.4/4.1 source gaps remain qualified.

Performance

The follow-up adds a small preflight check and changes artifact names/routing. It adds no runtime data-processing work. The existing compression setting and retry budgets are unchanged. CI confirms compression level 0 reaches the JVM uploads, but no measured speedup or retry-reliability improvement is claimed.

Design

Qualifying producer names fixes both download ambiguity and cross-producer overwrite with the existing action interface. The merge retains the base event/label gates. Docs and benchmark routing remain unchanged. Both have Maven call paths, so this is not an exhaustive Maven-consumer routing guarantee.

Abstraction & complexity

The focused checker adds no dependency and catches the reported regressions. Its lightweight textual artifact scan is adequate as a guard for the current layout when supplemented by checking actual caller inputs. No further actionable complexity issue found.

@comphead

comphead commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @sunchao and @andygrove for the review, I'll keep eye on CI to check the stability

@comphead

comphead commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

@sunchao do you feel this PR closes #5489 ?

@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

In response to your question, I would keep #5489 open for its remaining work. This PR covers its Maven transfer-policy overlap, but does not replace the full change.

I compared unchanged head 50c7b53f62f08c3a2d11d07d3bd1be4ab630259e against base 424c31aa79d13fddf743ffa29bae3c6f146e6c5e, and #5489 at 093db4c78. The remaining scope is concrete:

  • Shared Maven bootstrap/cache setup and the hosted-runner cache-path fix. Here, that bootstrap loop remains in java-test, and its cache still lists both home and /root paths.
  • Dependency-only SBT retries and SBT/Coursier caching. The Spark build and test steps here still invoke build/sbt directly. Maven's .mvn/maven.config does not configure those downloads.
  • Delta gate failure logs and effective-POM error propagation. Those script/workflow changes from #5489 are absent here.

The artifact wrapper addresses a separate upload-finalization failure. It retries the upload step, uses the same producer-specific name, forces overwrite on later attempts, and propagates the final failure. It does not retry dependency acquisition or build/test commands. The previous artifact naming P2 remains fixed, and I found no new P1/P2. My existing approval stands.

At the September 9, 20:05 UTC refresh, there were 59 successful checks, 9 skipped and one queued macOS lint job. Delta gate and CodeQL succeeded. CI run 34370771074 executes merge 03d6a3c2, whose whole tree matches this head. I reverified the retained receipts against current job/artifact metadata: the five Spark 3.5/4.1 and Iceberg 1.11 producer/consumer artifact pairs still match. Six sampled native/JVM uploads succeeded on attempt one, with all twelve later attempts skipped. This demonstrates successful integration, not retry recovery or sustained stability. Full CI remains incomplete. The exact configuration checker passed locally again.

Spark expression/operator behavior, including types, nulls, overflow, errors and fallback, remains unchanged relative to base. This follow-up makes no new engine compatibility claim, including for the maintained Spark 3.4/4.1 source gaps.

Performance

There is no new timing or reliability measurement. The successful path adds no retry wait, while a failed upload can incur two more uploads and 60 seconds of explicit backoff. The SBT caching and dependency pre-resolution benefits proposed in #5489 are outside this change, so this run does not establish them.

Design

The overlap is specifically the Resolver retry count and retryable-status list. #5489 sets them through MAVEN_OPTS, while this PR puts them in the repository Maven configuration. After this lands, #5489 can remove that duplicated policy and retain its remaining setup, dependency and diagnostic work. Its interval/backoff settings need a separate decision. That is a scope boundary, not an additional blocker on this PR.

Abstraction & complexity

No new abstraction has appeared since the approved head. The upload wrapper and focused configuration guard remain appropriate for this scope. Keeping the broader work tracked in #5489 avoids treating one successful run as evidence that every download and diagnostic path is covered.

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

Both points are addressed, and you went further than I asked by turning them into a gate rather than a fix.

The artifact collision is closed the right way round: the two workflows ci.yml calls repeatedly now version-qualify their names, and native-lib-linux / native-lib-macos stay put because pr_build_linux.yml and pr_build_macos.yml are each called once. I checked that on the branch, and native-lib-linux is now uploaded only from pr_build_linux.yml. That also clears the pre-existing ambiguity, not just the retry-plus-overwrite case I was worried about.

compute-changes.py now carries .github/actions/upload-artifact-retry/** and .mvn/** in all six filter lists, so an edit to either can no longer land with only actionlint having looked at it.

check-ci-config.py is the part I did not expect and like most. I ran it on the merged head and negative-tested it rather than take it on trust: it passes clean, and reintroducing the collision by pointing spark_sql_test_reusable.yml back at native-lib-linux gives

artifact name: .github/workflows/spark_sql_test_reusable.yml: artifact 'native-lib-linux' is
uploaded by a workflow ci.yml calls 4 times; qualify the name with an input
(e.g. ${{ inputs.spark-full }}) so the parallel producers stay distinct

with exit status 1, so it fails the build rather than just printing. The message names the fix, which is what makes a check like this useful a year from now. Both invariants I raised are enforced, so neither can regress silently.

Approving. On your question about #5489: I would let @sunchao answer for their own PR, but from this side the retry configuration and the aether.connector.* keys here do overlap with what #5489 was doing, so it is worth reconciling rather than landing both.

One thing that remains true and is worth stating in the description: nothing in CI exercises the retry path itself, so the wrapper's behaviour under an actual upload failure is still unverified. That is not a reason to hold this, since the wrapper's inputs match actions/upload-artifact@v7 one for one and the failure mode without it is a hard job failure, but it means the first real retry will be the first test.

@comphead

comphead commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

The Lint job is not queued up. Checking if this is a PR issue or infra

UPD: ubuntu-latest runners was not starting up, changed them to ubuntu-24.04

`ubuntu-latest` jobs are not getting runners. Every one in the repo over the
last several hours sat queued or was cancelled while still queued, including
`Deploy Comet site / Build docs` on main. Three samples that did eventually
start waited 3h24m, 3h34m and 4h08m between `created_at` and `started_at`.
In the same minutes, `ubuntu-24.04` jobs in the same repo were picked up in
6 to 24 seconds, and `ubuntu-slim` was likewise healthy.

`ubuntu-latest` is the default label nearly every Apache project uses, so it
is the contended pool; the explicitly versioned labels are served from
elsewhere. These were the only three jobs left on the floating label out of
34, against 21 already on `ubuntu-24.04` and 8 on `ubuntu-slim`. Pinning them
matches the rest of the repo and removes the dependency on whichever version
`ubuntu-latest` currently resolves to.

`pr_build_linux.yml` keeps the job *display name* `ubuntu-latest/rust-test`.
It already runs on `ubuntu-24.04`, and renaming a job renames its check,
which would need a branch-protection update.
@andygrove

Copy link
Copy Markdown
Member

@comphead ci failure

- runtime filtering - join with dynamic partition pruning (1 second, 186 milliseconds)
  #
  # A fatal error has been detected by the Java Runtime Environment:
  #
  #  SIGSEGV (0xb) at pc=0x0000000000000000, pid=1317, tid=4739
  #
  # JRE version: OpenJDK Runtime Environment Zulu11.90+19-CA (11.0.32+9) (build 11.0.32+9-LTS)
  # Java VM: OpenJDK 64-Bit Server VM Zulu11.90+19-CA (11.0.32+9-LTS, mixed mode, tiered, compressed oops, g1 gc, linux-amd64)
  # Problematic frame:
  # C  0x0000000000000000
  #
  # Core dump will be written. Default location: Core dumps may be processed with "/usr/lib/systemd/systemd-coredump %P %u %g %s %t 9223372036854775808 %h %d" (or dumping to /__w/datafusion-comet/datafusion-comet/spark/core.1317)
  #
  # An error report file with more information is saved as:
  # /__w/datafusion-comet/datafusion-comet/spark/hs_err_pid1317.log
  [thread 17129 also had an error]
  #
  # If you would like to submit a bug report, please visit:
  #   http://www.azul.com/support/
  #

probably the known hdfs issue?

@comphead

comphead commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

@comphead ci failure. probably the known hdfs issue?

Checking if I can get more logs

UPD: Got the dump, but I dont see anything hdfs related there.

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 area:Iceberg area:udf enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants