ci: share one Linux native build across CI workflows

[sunchao]

Which issue does this PR close?

Part of #5830. Follow-up to #3249. Planned to land after #5976; this branch is rebased on current main, where #5976 is still unmerged.

Rationale for this change

Linux, Spark SQL and Iceberg currently build the same default Linux native library separately. For a shared-source change the current queue selects three native consumers, nightly selects seven, and manual dispatch can select ten. A single producer removes two duplicate builds from the queue and shares its artifact with every selected consumer. This PR also keeps independent lint/compile checks off the native-build critical path.

After #5976 lands, the remaining benefit will be avoiding duplicate warm builds for native-changing runs and duplicate library restores for matching runs. Current-head end-to-end savings have not been measured. The shared producer must be changed to use that PR's composite action on the dependency rebase before merge.

What changes are included in this PR?

• Build and publish native-lib-linux once, then require every Linux, Spark SQL and Iceberg consumer, including Spark 4.2, to depend on it. Required Checks includes the producer.
• Move the current Linux lint, strict Scala warnings, Celeborn compatibility and Rust tests into an independent reusable workflow. Preserve main's full Java profile matrix, Maven retry/bootstrap, main-only cache publication and uncached Rust debug tests.
• Keep current push/nightly/label/manual policy. Pushes refresh Linux native/Maven/dataset caches and the separate macOS cache while skipping independent checks. Configuration guards cover each workflow and its mode inputs.
• Keep JVM compilation separate per Spark/JDK. The Java-only Spark compilation job explicitly runs Maven bootstrap; jobs using setup-builder inherit it without an extra step.
• Derive native selection after all consumer path/event decisions, including Hive-only and Spark 4.2 routes. Tests cover native-only push inputs and a future contrib-manifest filter extension from ci: reuse Linux native libraries across workflow runs #5976.
• Update contributor and configuration pointers for moved jobs.

How are these changes tested?

Rebased onto main 0ac4dadae70838d8675bfd846c2d1c7617c25c78, including #6646 (8a7e664). That upstream fix addresses the exact prior-head Spark 3.4 failure: DynamicPartitionPruningV1SuiteAEOn / SPARK-34637: DPP side broadcast query stage is created firstly. Refreshing stale shuffle scans during AQE replanning allowed the join's broadcast build side to change and broke DPP broadcast reuse; #6646 moves that refresh to stage/final-plan preparation.

At b4ef9529491908cfa4091878dba6501286d85baa:

• The exact upstream Spark 3.4.3 test passes with Comet extensions, plugin, native scan and shuffle enabled, including its broadcast-reuse assertion. Five related Comet DPP/stale-shuffle regressions also pass on Spark 3.4.3.
• Full Spark 3.4 reactor test compilation, Spotless and Scalastyle pass. The native/protobuf inputs match the verified main 74257803 CI artifact used by these runtime tests; no local native rebuild was needed.
• CI configuration checker, 42 configuration mutation tests, 24 native-selection tests, actionlint 1.7.12 (ShellCheck disabled), and whitespace checks pass. Independent rebase review confirms shared-producer, independent-check, cache and routing contracts are preserved alongside main's new checks and suite inventory.

Broader fresh hosted CI is pending. Current-head producer/artifact integration and push/nightly execution still require hosted verification. The prior-head assertion is not classified as a flake. After artifact expiry, rerun the full workflow.

[rich7420]

LGTM

[andygrove]

LGTM but I would like to merge #5842 first so we can unblock setting up the merge queue to reduce CI resource usage. I reviewed this PR assuming that it will go in after #5842.

Once #5842 is in, two things will need to happen on the rebase here:

1. ci.yml merges cleanly between the two PRs, but #5842 adds a required_checks aggregator whose needs: must list every ci.yml job, and a preflight guard in check-ci-config.py that enforces it. The merged file fails that guard until pr_build_linux_checks and build_linux_native are added to required_checks.needs. The guard message names them, so the rebase will tell you. build_linux_native in particular has to be there and cannot be exempted: if the producer fails, GitHub marks all nine consumers skipped, and the aggregator treats skipped as pass, so without the producer in needs a broken native build would produce a green Required Checks.

2. #5842 hardens Lint Scala (syntactic) against Maven Central connection resets by splitting it into a retried cs launch scalafix:0.14.6 -- --version warm-up followed by the real check under cs launch --mode offline. Since this PR moves that job into pr_build_linux_checks.yml, that split needs to come along. The iceberg_spark_test_reusable.yml change in #5842 (routing the shard-inventory upload through upload-artifact-retry) merges cleanly onto your prepare-shards version.

After both land, check-ci-config.py will carry two hand-rolled YAML readers (block_mapping here and the line regexes in #5842), both justified by PyYAML not being on the preflight runner. I will open a follow-up issue to install PyYAML in preflight and collapse them rather than leave both in place.

I verified the two-way compatibility locally: your checker passes against a ci.yml that includes the required_checks job, and the #5842 checker passes against your ci.yml once the two ids are added.

[andygrove]

One more heads-up from the merge-queue side, following on from my review above.

#5843 (the PR that actually enables the queue, stacked on #5842) changes POLICY in dev/ci/compute-changes.py so that a push to main selects only docs. Every test job moves to the merge_group event instead, because the queue has already tested the exact tree that lands.

That will break test_main_and_manual_runs_include_legacy_consumers in dev/ci/test-native-build-selection.py here, which asserts event="push" selects every native consumer. Whichever of #5841 and #5843 lands second needs to update that test (the workflow_dispatch half stays true). If this PR lands first, #5843 will pick it up on its rebase along with adding pr_build_linux_checks and build_linux_native to required_checks.needs. If #5843 lands first, the push assertion here should become merge_group.

Also FYI, run-spark-3.4-tests and run-spark-3.5-tests now exist as labels, so the label-gated selection tests here have real labels behind them.

[andygrove]

Three things, all about the rebase rather than the design. The producer and consumer wiring itself looks right to me, and I checked that build_linux_native is the exact union of the nine consumer outputs on this head.

#5852 landed on main after this branch was last pushed, adding a Bootstrap Maven step calling ./.github/actions/maven-bootstrap to five jobs in pr_build_linux.yml. Three of those five are the jobs this PR moves into pr_build_linux_checks.yml, namely lint-java, build-spark-4-1 and celeborn-reflection-compatibility. A rebase cannot carry that step into a file this branch creates from scratch, and pr_build_linux.yml conflicts, so the natural resolution is to take the deletion and lose the retry for those three. The two TPC jobs stay behind and keep theirs. Nothing in check-ci-config.py verifies that a job calling a bare ./mvnw has the bootstrap step ahead of it, so this would be silent until the next Maven Central blip takes out a queue-gating job. Could you add it back to those three after the rebase, and update the ROUTING_CASES comment that says the composite is called only from pr_build_linux.yml? Given how easily it drops out in a file move, is it worth a guard for it?

NATIVE_CONSUMERS is a flat list of output keys, one per consumer job, and #5871 has already broken that assumption on main. It added spark_4_1_hive as a second output feeding the same spark_4_1 job, which is now gated on needs.changes.outputs.spark_4_1 == 'true' || needs.changes.outputs.spark_4_1_hive == 'true'. On a labeled run with run-spark-4.1-hive-tests, spark_4_1 is false and spark_4_1_hive is true, so the union computes build_linux_native as false, the producer is skipped, and spark_4_1 is then skipped through its needs even though its if is true. The label would quietly do nothing and Required Checks would still go green. compute-changes.py is the one file that auto-merges cleanly here, so the union will miss the new key silently. native_selection_failures will fail preflight on the if: string mismatch, but the tempting fix there is to relax the comparison, which leaves the hole. Would it make sense to let a consumer map to a set of output keys rather than one?

Last one is coverage. The hosted run only selected pr_build_linux, spark_4_1 and iceberg_1_11, so six of the nine rewired consumers have not run against the shared artifact. spark_3_4 is the one I would most want to see, because its build job in spark_sql_test_reusable.yml is the only consumer whose toolchain setup actually changed, swapping ./.github/actions/setup-builder for a bare actions/setup-java@v4 at JDK 11, and it now consumes a libcomet.so linked against JDK 17. The linux-test Spark 3.4 lane already proves the library loads under JDK 11, but the new setup path has no coverage. These are queue-tier jobs, so a break blocks every merge rather than one PR. Could you apply run-spark-3.4-tests, run-spark-3.5-tests, run-spark-4.0-tests and run-iceberg-tests once on the rebased head?

[sunchao]

@andygrove, addressed the three items from your latest review and rebased onto main at 451c999632; the PR is now at 79c161b293 with no merge conflicts.

1. Maven bootstrap: preserved the bootstrap in all three moved jobs and both TPC jobs, and corrected the routing comment. The new guard requires an earlier unconditional bootstrap with failures propagated before direct ./mvnw commands in both Linux workflows. Regression mutations remove, move, condition, or ignore the bootstrap's failure in each of the five jobs.
2. Hive routing: NATIVE_CONSUMERS now maps caller IDs to all selecting outputs. spark_4_1 includes both core and Hive, so a Hive-only label starts the native producer. Exact caller, OR-gate, export, and dependency validation remains enforced. Compute and CLI tests cover core-only, Hive-only, both, queue, and manual runs; upstream's module selection is preserved.
3. Coverage: applied all four requested labels after pushing this head: run-spark-3.4-tests, run-spark-3.5-tests, run-spark-4.0-tests, and run-iceberg-tests. The regular PR run and four additional opt-in runs (1, 2, 3, 4) are pending. This requests coverage of all nine rewired consumers, including the Spark 3.4/JDK 11 setup path; runtime results are not yet confirmed.

Local validation passed: 36 configuration regression tests, 21 native-selection tests (299,008 combinations), actionlint, suite/benchmark inventory checks, 15 Iceberg shard tests, 4 PR-label tests, Markdown formatting, and whitespace checks. The PR description now reflects the implementation and pending hosted coverage.

[andygrove]

Happy to approve once the following issues are resolved.

Thanks for the quick turnaround on the last round. All three items are addressed: the Maven bootstrap is back in the three moved jobs and guarded, NATIVE_CONSUMERS maps callers to tuples so the Hive-only label starts the producer, and all four label runs came back green, so every one of the nine consumers has now run against the shared artifact, including the Spark 3.4 setup-java-only build path. I also ran the three Python checks locally on 79c161b and they pass in about five seconds total, so the preflight cost is fine.

1. Rebase hazard from #5897. chore: drop support for JDK 11 landed on main after this head was pushed. It edits the lint-java matrix in pr_build_linux.yml (the Spark 3.4 entry becomes JDK 17 and JAVA_TOOL_OPTIONS drops the version conditional), and flips java: 11 to 17 for spark_3_4 and iceberg_1_8 in ci.yml and for preflight. The ci.yml and linux-test hunks should auto-merge, but the lint-java hunk conflicts with this PR's deletion of that block, and pr_build_linux_checks.yml is a new file, so the natural resolution drops the change and the new file keeps JDK 11. Same shape as the bootstrap step last time. It fails loudly this time because the new Maven enforcer rejects JDK 11, so no guard is needed, but could you carry those edits into pr_build_linux_checks.yml on the rebase?

2. Contributor doc pointer. docs/source/contributor-guide/adding_a_new_spark_version.md (around line 112) tells authors to add the compile-only job to pr_build_linux.yml, but build-spark-4-1 now lives in pr_build_linux_checks.yml. Could you update the file name there? The suite-matrix instructions in development.md and check-suites.py still point at pr_build_linux.yml correctly since linux-test stays put.

Nothing else from me. The wiring, the main-only cache write on the single producer, artifact retention, the permissions blocks, and the README rewrite all look right.

[sunchao]

@andygrove, both items from your latest review are addressed. Rebased onto main at 39da85081; the new head is a126925b7 and GitHub reports no merge conflicts.

1. JDK 17: carried the Spark 3.4/JDK 17 lint matrix entry and unconditional JAVA_TOOL_OPTIONS into pr_build_linux_checks.yml. The preflight, Spark 3.4, Iceberg 1.8, and Linux test updates from chore: drop support for JDK 11 #5897 are also preserved. All six moved jobs match the current upstream jobs.
2. Contributor guide: the compile-only instructions now point to pr_build_linux_checks.yml. The suite-matrix instructions still point to pr_build_linux.yml, where linux-test remains.

The rebase also preserves Spark 3.4's new label/manual-only policy. Updated the selection regression to verify that queue runs select the other eight consumers, that a Spark 3.4-only queue change does not start native compilation, and that labels cannot opt Spark 3.4 back into a queue run. Its label and manual paths remain covered through both Python and the CLI.

Local checks passed: 36 configuration tests, 21 selection tests (including 299,008 combinations), actionlint, suite/benchmark checks, 15 Iceberg shard tests, 4 PR-label tests, Markdown formatting, and whitespace checks. Updated the PR description with the previous green coverage and the fresh CI run. The four opt-in labels remain applied; results on this rebased head are pending.

[andygrove]

One rebase item, and this time only half of it is loud.

#5930 landed on main after this head. It adds a cache-refresh-only input to pr_build_linux.yml so that push to main runs only the jobs that own an actions/cache entry, and check-ci-config.py grew a sixth invariant pinning which jobs survive it. Three of the five jobs it names, lint, build-native and linux-test-rust, are jobs this PR moves into the two new workflows.

I merged main into a126925b7 locally. Four files conflict and compute-changes.py auto-merges correctly, the FILTERS["build_linux_full"] = FILTERS["build_linux"] alias picks up your two new paths for free. Preflight then fails on the three stale CACHE_REFRESH_JOBS names, on linux-test having lost its guard in the move, and on two ROUTING_CASES entries needing build_linux_full. All of that is loud and the messages say what to do.

The silent part is what is left after you do it. I dropped the three stale names, put the guard back on linux-test, and check-ci-config.py prints CI config checks passed. Both of your test suites pass too, 21 and 36. But pr_build_linux_checks.yml is gated on build_linux, which has a push tier, and it has no cache-refresh-only input, so the whole thing runs on every push to main. I confirmed the selection with EVENT_NAME=push python3 dev/ci/compute-changes.py on the merged tree: build_linux=true, build_linux_full=false. Using this PR's own durations that is lint-java x4 at 19m10s, Celeborn x2 at 12m03s, build-spark-4-1 at 4m21s and scalafix-syntactic at 25s, about 36 runner-minutes a push. #5930 took the push tier from 587 to about 73, so this puts it back to roughly 109.

Could you carry the input into pr_build_linux_checks.yml on the rebase, wired from ci.yml the same way as pr_build_linux with ${{ needs.changes.outputs.build_linux_full != 'true' }}, and guard those four jobs? lint and linux-test-rust should stay unguarded, since linux-test-rust owns the cargo-debug cache and lint gates it, which is the same reasoning #5930 used for the originals. Passing an input does not change the caller's if:, so your linux_checks_failures invariant that the two callers share a condition still holds.

The guard itself also needs to become multi-workflow. CACHE_REFRESH_WORKFLOW is a single Path today, and once the cache writers live in three files a table of {workflow: {job: reason}} is what actually expresses the invariant. Would it be worth extending the CACHE_REFRESH_INPUT check the same way, so a caller that forgets the input for either workflow fails preflight rather than only the pr_build_linux one? Your test-ci-config.py mutation harness is the natural place to pin it, a mutation that drops the guard from a job in pr_build_linux_checks.yml would have caught exactly what I hit.

Two smaller pieces of the same rebase. docs/source/contributor-guide/ci.md lines 55-60 and 214 name the five cache-writing jobs and tell contributors to add the guard when they add a job to pr_build_linux.yml. That file is not in this PR and merges clean, so it goes stale without saying anything. And the build_linux comment in POLICY still says the push tier is there for "main's cargo-ci, cargo-debug, Maven and TPC-H/TPC-DS caches", but cargo-ci now lives in build_linux_native.yml and cargo-debug in pr_build_linux_checks.yml, so that one output is load-bearing for three workflows now rather than one. Worth spelling out, because it is the thing that keeps the shared producer running on push at all.

Nothing else from me. The producer and consumer wiring, the union over NATIVE_CONSUMERS, the Maven bootstrap guard and the JDK 17 carry-over all look right on this head, and all nine consumers came back green.

[sunchao]

@andygrove, all items in your latest review are addressed in 3c31e5aa8. I checked the current head against the review and reran the configuration and routing tests.

• Both Linux callers pass the same cache-refresh-only and profiles inputs. In cache-refresh mode, the four moved Java/Scala lint, compile-only, and Celeborn jobs are skipped, while Rust formatting and the Rust debug cache writer remain active. The Linux JVM test matrix is also guarded. The expressions account for the newer all-profile/nightly route.
• CACHE_REFRESH_JOBS now maps each of the three workflows to its retained jobs and reasons. Each Linux caller's inputs are checked independently. Mutation tests cover a dropped guard in the checks workflow, missing/miswired inputs on either caller, and removing or guarding off the cache writers and their prerequisites.
• The contributor CI guide now explains cache ownership across the three files and where new jobs need guards. The build_linux policy comment explicitly explains why push must select all three workflows. The earlier Maven bootstrap, Hive routing, JDK 17, and compile-job documentation fixes remain present.

The configuration checker, all 41 configuration tests, and all 23 selection tests pass. Both commit CI and Spark 4.1 opt-in CI passed on this exact head. Together they exercised all nine native consumers, including every Spark 4.1 Hive shard. Hosted push/nightly execution remains separate from this PR coverage; their selection and guards are covered locally.

[rich7420]

@sunchao thanks for the patch

[sunchao]

Summary

• Prior state and problem: Linux, Spark SQL, and Iceberg workflows separately compiled the same native library.
• Design approach: Share one Linux native producer across nine callers while running independent Linux checks alongside it.
• Correctness / compatibility analysis: Base comparisons confirm preservation of native build settings, moved checks, test commands, and suite matrices. Checked relevant Spark build/test sources for 3.4.3, 3.5.9, 4.0.4, and 4.1.3. No SQL execution semantics change.
• Key design decisions: Selection stays in the existing router, including Hive-only and all-profile routes. Explicit artifact inputs preserve the producer dependency. Both new callers feed Required Checks.
• Implementation sketch: Extract two reusable workflows, update artifact downloads, and extend configuration guards and mutation tests across the Linux workflows.
• Behavioral changes worth calling out: Duplicate native compilations are removed, and push cache-refresh and nightly scopes are preserved. Elapsed-time savings were not measured. Artifact retention remains one day, requiring a full workflow rerun after expiry.
• Suggested improvements: None at P1/P2 severity. No introduced P1/P2 issues found within this review. Earlier substantive review concerns are addressed.

Reviewed all 13 changed files from base 23f5b63b14a09802a9c8b98858b84f320bd517fb to full head 0c5bf4c25648521a31af6ce821113b3fc6654aa7. Routed skill: review-comet-pr. No sibling skill applies to this CI-only scope. Read the supplied discussion and resolved threads, excluding Copilot.

Exact-head CI is not green: run 35054382827 has 83 successful jobs, 10 skipped jobs, one cancelled Spark 3.4 sql_core-2 shard, and failed Required Checks. GitHub confirms that shard exceeded six hours. The shared producer, moved checks, and other selected tests passed. CodeQL passed. The timeout’s cause was not established or attributed to this PR.

Local validation passed: configuration checker, 41 configuration tests, 23 routing tests, actionlint 1.7.12 with ShellCheck disabled, structural comparisons against base, all 2,250 tracked paths across 17 routes, and whitespace checks. No local native/Spark build was run. Hosted push/nightly behavior and reruns after artifact expiry remain unverified.

[andygrove]

This should wait for #5976, as you planned, and I think the rebase onto it is more than mechanical because the two PRs route the native build differently. #5976 extends every Spark and Iceberg consumer's filter with NATIVE_CACHE_RECIPES, since each of them runs build-native-ci itself, and adds a push-only override to compute() so main warms the library cache. Here the consumers stop building and build_linux_native is derived from their outputs. After the rebase the recipes only concern the shared producer, and build_linux_native.yml should call the composite rather than keep its own restore, build and save steps. Its if: github.event_name == 'push' save no longer passes #5973's main-only check anyway. The override also has to run before your union, which I've left inline, along with one silent hazard from #5881 in spark_sql_test_reusable.yml.

The four points from my last review are all addressed. I checked the cache-refresh-only input and guards in pr_build_linux_checks.yml, the per-workflow CACHE_REFRESH_JOBS table, the ci.md text and the POLICY comment, and test-ci-config.py, test-native-build-selection.py and check-ci-config.py pass at 0c5bf4c25. I don't think the six-hour Spark 3.4 sql_core-2 cancellation is this PR. Every test in that shard had passed by 05:22, and the job printed nothing after that until the timeout. The same shard passed in 35 minutes at 3c31e5aa8 a few hours earlier, and the main merge between those two runs changed only Comet test files and the 4.1 diff.

After the rebase, could the description be re-based on what is left to save? The queue has three Linux native producers today, and in recent queue runs the Linux and Iceberg 1.11 builds took 3 to 4 minutes each. With #5976 those become library restores whenever the native inputs match main, and about 60% of the commits that landed in the last two weeks didn't touch native/. What this PR adds on top is mostly two warm builds per native-changing queue run, and I'd like to see that number next to the size of the change before this goes in.

[sunchao]

Summary

• Prior state and problem: Linux, Spark SQL, and Iceberg workflows independently compiled the same native library.
• Design approach: Share one Linux native producer across nine callers, with independent Linux checks running alongside it.
• Correctness / compatibility analysis: Base comparisons confirm preserved native build settings, moved checks, test commands, and suite matrices. Checked relevant upstream Spark build sources for 3.4.3, 3.5.9, 4.0.4, and 4.1.3. No SQL execution semantics change.
• Key design decisions: Routing remains in compute-changes.py. The producer includes Hive-only and all-profile routes. Explicit dependencies and Required Checks coverage ensure producer failures block the aggregate verdict.
• Implementation sketch: Extract the producer and independent checks into reusable workflows, pass the shared artifact explicitly, and extend preflight guards and regression tests. Version-specific JVM artifacts remain separate.
• Behavioral changes worth calling out: Duplicate native compilations are removed while push cache-refresh and nightly scopes remain intact. Elapsed-time savings are unmeasured. Artifact retention remains one day, after which failed-job reruns require a full workflow rerun.
• Suggested improvements: No introduced P1/P2 issues found within this review. Earlier current-head concerns are addressed. The two unresolved inline threads describe future rebase hazards involving changes absent from this base/head pair.

Reviewed all 13 changed files from base 23f5b63b14a09802a9c8b98858b84f320bd517fb to full head 0c5bf4c25648521a31af6ce821113b3fc6654aa7. Confirmed the PR is not a draft. Routed skill: review-comet-pr; no sibling skill applies to this CI-only scope. Read existing reviews, issue comments, inline comments, and threads, excluding Copilot.

Exact-head CI is not green: run 35054382827 has 83 successful jobs, 10 skipped jobs, one cancelled Spark 3.4 sql_core-2 shard, and failed Required Checks. GitHub confirms the shard exceeded six hours. Its log stops producing test output around 05:22 until cancellation at 10:51. The producer, moved checks, and remaining selected tests passed. CodeQL passed. The timeout’s cause was not established or attributed to this PR.

Local validation passed: configuration checker, 41 configuration tests, 23 routing tests, actionlint 1.7.12 with ShellCheck disabled, suite inventory, 15 Iceberg shard tests, structural comparisons, and whitespace checks. Comparing 2,246 base paths across 19 event cases found no existing-output routing differences. No local native/Spark build was run. Hosted push/nightly behavior, all-profile runtime coverage at this head, and reruns after artifact expiry remain unverified.

[andygrove]

This is a light fully automated review since there are so many PRs open.

I found three comments outside the changed files that still describe the pre-split layout, so they go stale once this lands. .scalafix-syntactic.conf:20 sends readers to the scalafix-syntactic job in pr_build_linux.yml, and .github/workflows/pr_benchmark_check.yml:75 points at pr_build_linux.yml for the same semanticdb exclusion in the main lint matrix. Both scalafix-syntactic and lint-java now live in pr_build_linux_checks.yml. dev/ci/linux-test-profiles.py:26 says the lint job runs the script and publishes the matrix that linux-test reads. At this head that is prepare-matrix in pr_build_linux.yml, and lint only runs cargo fmt in the checks workflow. Could these three pointers be updated in this PR, so the next person chasing a syntactic scalafix failure or editing the profile rows lands in the right file?

[sunchao]

Summary

• Prior state and problem: Linux, Spark SQL, and Iceberg workflows independently compiled the same native library.
• Design approach: Share one Linux native producer across nine callers, with independent Linux checks running alongside it.
• Correctness / compatibility analysis: Base comparisons confirm preserved native build settings, moved checks, test commands, and suite matrices. Checked upstream Spark build sources for 3.4.3, 3.5.9, 4.0.4, and 4.1.3. No SQL execution semantics change.
• Key design decisions: Selection remains in compute-changes.py, including Hive-only and all-profile routes. Explicit artifact dependencies and Required Checks coverage ensure producer failures block the aggregate verdict.
• Implementation sketch: Extract two reusable workflows, pass the shared artifact explicitly, and extend configuration guards and mutation tests. Spark/JDK-specific JVM artifacts remain separate.
• Behavioral changes worth calling out: Duplicate native compilations are removed while push cache-refresh and nightly scopes remain intact. Elapsed-time savings are unmeasured. Artifact retention remains one day, after which consumers require a full workflow rerun.
• Suggested improvements: No introduced P1/P2 issues found within this review. Earlier substantive concerns are addressed. The unresolved inline threads describe future rebase hazards involving changes absent from this base/head pair.

Reviewed the full 13-file diff from base 23f5b63b14a09802a9c8b98858b84f320bd517fb to head 0c5bf4c25648521a31af6ce821113b3fc6654aa7. Confirmed the PR is not a draft. Routed skill: review-comet-pr. No sibling skill applies to this CI-only scope. Read existing reviews, issue comments, inline comments, and threads, excluding Copilot.

Exact-head CI is not green: run 35054382827 has 83 successful jobs, 10 skipped jobs, one cancelled Spark 3.4 sql_core-2 shard, and failed Required Checks. GitHub confirms the shard exceeded six hours. The shared producer, moved checks, and remaining selected tests passed. CodeQL completed successfully with permissions annotations. No evidence establishes that this PR caused the timeout.

Local validation passed: configuration checker, 41 configuration tests, 23 routing tests, actionlint 1.7.12 with ShellCheck disabled, structural comparisons, and whitespace checks. Comparing 2,246 base paths across 22 event/label cases found no existing-output routing differences. No local native/Spark build was run. Hosted push/nightly behavior, all-profile runtime coverage at this head, and reruns after artifact expiry remain unverified. The checkout is unchanged.

[sunchao]

Rebased onto 9866221 and pushed eae558b. The moved jobs come from current main, preserving Spark 4.1/4.2 compilation, strict Scala warnings, main-only cache writes, uncached Rust debug tests, and macOS cache-refresh routing. Spark 4.2 now participates in the shared native producer’s selection and artifact dependency.

Updated the scalafix-syntactic and linux-test-profiles pointers. The old semanticdb pointer in pr_benchmark_check.yml was already removed on main; the rebased file has no stale pointer. Updated the compile-only contributor guidance and PR description, including the remaining #5976 dependency and the absence of current end-to-end timing measurements.

CI configuration, 42 mutation tests, 24 selection tests, actionlint, suite inventory, 15 Iceberg tests, benchmark inventory, Markdown formatting and whitespace checks pass locally. Fresh hosted validation is pending.

[sunchao]

Summary

• Prior state and problem: Linux, Spark SQL, and Iceberg workflows separately compiled the same native library.
• Design approach: Share one Linux native producer across ten consumer callers, while independent checks run alongside it.
• Correctness / compatibility analysis: Native build settings, moved checks, suite matrices, and test commands are preserved. Checked upstream Spark build sources for 3.4.3, 3.5.9, 4.0.4, 4.1.3, and 4.2.0 against the Java-only compilation setup. No SQL execution semantics change.
• Key design decisions: compute-changes.py owns producer selection, including Hive-only and all-profile routes. Explicit dependencies and Required Checks coverage ensure producer failures block the aggregate verdict.
• Implementation sketch: Extract reusable producer/check workflows, pass the native artifact explicitly, retain Spark/JDK-specific JVM artifacts, and extend configuration guards and mutation tests. This keeps scheduling policy in the existing router.
• Behavioral changes worth calling out: Duplicate native compilations are removed while push and nightly scopes remain intact. End-to-end savings are unmeasured. Artifact retention remains one day, after which consumers need a full workflow rerun.
• Suggested improvements: None at P1/P2 severity. No introduced P1/P2 issues found within this review. Earlier substantiated bootstrap, routing, and cache-scope concerns are addressed.

Reviewed the entire 15-file diff from base ac9ae94d057013095fdf1b70e2d4bc5b24a58d90 to head 765545d451ed2bd279bf838589653ed070f94488, including all three commits. Confirmed the PR is not a draft. Read existing reviews, issue comments, inline comments, and threads, excluding Copilot. Routed skill: review-comet-pr. No sibling skill applies to this CI-only scope.

Compared affected paths with branch-1.1. The PR introduces intended CI reuse and scheduling changes, with unchanged native profile/flags and no additional user-facing runtime behavior. The declared, unmerged #5976 dependency is absent from this reviewed tree.

Exact-head CI remains pending: Comet CI run 37222066341. At inspection, checks showed 26 successes, 31 in progress, six skipped, and no failures. The shared producer, moved Java checks, and CodeQL passed. All 34 started native-artifact downloads succeeded. Spark 4.2 runtime coverage was skipped, and Required Checks has no final verdict yet.

Local validation passed: configuration checker, 42 mutation tests, 24 selection tests, actionlint 1.7.12 with ShellCheck disabled, suite inventory, 15 Iceberg-shard tests, benchmark inventory, structural comparisons, and whitespace checks. Comparing 2,494 existing paths across 34 event cases found no routing differences. No local native/Spark runtime build was run. Hosted push/nightly execution and reruns after artifact expiry remain unverified. The checkout is unchanged.

[sunchao]

Rebased on main 0ac4dadae7, bringing in #6646 (8a7e664), which fixes the exact Spark 3.4 SPARK-34637 broadcast-reuse failure from the prior head. The exact upstream test now passes locally with Comet enabled, alongside five related DPP/stale-shuffle regressions. All 66 CI configuration/selection tests and actionlint pass; the CI split itself remains intact. Fresh hosted validation is pending.

[sunchao]

Summary

• Prior state and problem: Linux, Spark SQL, and Iceberg workflows separately compiled the same native library.
• Design approach: Share one Linux native producer across ten consumer callers, with independent checks running alongside it.
• Correctness / compatibility analysis: Native build settings, moved checks, suite matrices, and test commands are preserved. Checked upstream Spark build sources for 3.4.3, 3.5.9, 4.0.4, 4.1.3, and 4.2.0 against the Java-only compilation setup. No SQL execution semantics change.
• Key design decisions: compute-changes.py retains routing policy, including Hive-only and all-profile selection. Producer dependencies and Required Checks coverage prevent failed native builds from yielding a clean aggregate verdict.
• Implementation sketch: Extract producer/check workflows, pass the native artifact explicitly, retain version-specific JVM artifacts, and extend configuration guards. Scheduling remains in the existing router.
• Behavioral changes worth calling out: Compared affected paths with branch-1.1. The PR adds intended CI reuse and scheduling changes while retaining native profile/flags. Duplicate native builds are removed, but end-to-end savings remain unmeasured. Artifact retention remains one day.
• Suggested improvements: No introduced P1/P2 issues found within this review. Earlier substantiated concerns are addressed.

Reviewed all 15 changed files and all three commits from base 0ac4dadae70838d8675bfd846c2d1c7617c25c78 to full head b4ef9529491908cfa4091878dba6501286d85baa. Confirmed non-draft status and read existing reviews, issue comments, inline comments, and threads, excluding Copilot. Routed skill: review-comet-pr. No sibling skill applies to this CI-only scope. Unmerged #5976 is absent from this reviewed tree.

Exact-head CI passed: Comet CI run 37377680995 completed with 95 successful jobs and 11 skipped. The producer, moved checks, Spark 3.4–4.1 suites, and all four Iceberg versions passed. All 70 native-artifact downloads succeeded. CodeQL also passed. Spark 4.2 runtime tests were skipped.

Local validation passed: configuration checker, 42 mutation tests, 24 selection tests, actionlint 1.7.12 with ShellCheck disabled, structural comparisons, and whitespace checks. Comparing 2,542 existing paths across 35 event cases found no existing-output routing differences. No local native/Spark build was run. Hosted push/nightly execution and reruns after artifact expiry remain unverified. The checkout is unchanged.