ci: publish nightly SNAPSHOT jars to repository.apache.org - #5902
Conversation
github-actions
Bot
added
build
| fi | ||
| for jar in jars/*.jar; do | ||
| for lib in linux/amd64 linux/aarch64; do | ||
| if ! unzip -l "$jar" | grep -q "org/apache/comet/$lib/libcomet.so"; then |
There was a problem hiding this comment.
This can report a missing library when the entry is present. I reran this step against the four jars from your dry run in an Ubuntu 24.04 amd64 container: all eight checks failed with PIPESTATUS=141 0. grep -q exits after its match, leaving unzip to receive SIGPIPE, which pipefail treats as failure. The hosted run passed; this depends on pipe/consumer scheduling.
Letting grep consume the full listing passes all four jars and still rejects a copy with the aarch64 library removed:
| if ! unzip -l "$jar" | grep -q "org/apache/comet/$lib/libcomet.so"; then | |
| if ! unzip -l "$jar" | grep -F "org/apache/comet/$lib/libcomet.so" > /dev/null; then |
There was a problem hiding this comment.
Good catch, and thanks for running it down. Reproduced locally against one of those dry-run jars: the old form failed 100 out of 100 runs with PIPESTATUS=141 0, so the hosted run passing was luck. Took your suggestion — grep now consumes the whole listing, 0 failures out of 100, and it still returns 1 for a library that is genuinely absent.
| # property overrides every module's setting. $profiles is a | ||
| # space-separated list of -P flags, so it must word-split. | ||
| # shellcheck disable=SC2086 | ||
| JAVA_HOME="$jdk" ./mvnw -B "$GOAL" -DskipTests -Dmaven.deploy.skip=false $profiles < /dev/null |
There was a problem hiding this comment.
Could we validate the packaged jars before deploying them? With GOAL=deploy, this call uploads each snapshot before the native-library check below runs. I confirmed the upload order by deploying the Spark 4.1 reactor to a local file repository.
If that check catches an incomplete jar, the job fails after the jar is already available to consumers. Please move validation ahead of publication so the deployed artifacts have passed the check.
There was a problem hiding this comment.
You are right, the check was useless where it was. The loop now packages a variant, checks that jar, and only then runs the publishing goal for it, so nothing reaches Nexus before it has passed. The extra Maven invocation reuses the target directory the check ran against, and the parent pom pins project.build.outputTimestamp, so it re-creates the same jar — identical sha256 locally, and it cost 20s on top of a 75s package. I also dropped the separate count assertion in favour of failing when the jar glob does not match exactly one file.
One thing I decided not to do: variants are still published one at a time, so if the fourth fails its check the first three are already up. Making it all-or-nothing means packaging all four, then rebuilding each from clean to deploy it, and I did not think ~15 extra minutes a night was worth it for a snapshot that gets overwritten tomorrow. Happy to change it if you disagree.
| echo "::endgroup::" | ||
| done <<EOF | ||
| $JAVA_HOME_11_X64 -Pspark-3.4 -Pscala-2.12 | ||
| $JAVA_HOME_17_X64 -Pspark-3.5 -Pscala-2.12 |
There was a problem hiding this comment.
Just making sure, are AWS Labs folks are using Scala 2.12 for benchmarking?
There was a problem hiding this comment.
Just making sure, are AWS Labs folks are using Scala 2.12 for benchmarking?
Yes, according to https://awslabs.github.io/data-on-eks/docs/benchmarks/spark-gluten-velox-comet-benchmark
Add a scheduled workflow that builds the Comet native library for linux/amd64 and linux/aarch64 inside an Ubuntu 20.04 container, matching the glibc baseline of the release builder, and deploys SNAPSHOT jars for Spark 3.4, 3.5, 4.0 and 4.1 to the ASF snapshot repository. The run skips when main has not changed since the previous night, and a dry_run dispatch builds and verifies the jars without touching Nexus so the workflow can be exercised on a fork. Document how to consume the snapshots in the installation guide.
apache#5897 dropped JDK 11 support: the Spark 3.4 profile now targets Java 17 and a Maven enforcer rule rejects anything older, so the Spark 3.4 build in this workflow would fail on the JDK 11 it was pinned to.
The native-library check ran after the whole build loop, so with GOAL=deploy every jar was already uploaded by the time it ran. Package each variant, check it, and only then run the publishing goal for that variant. The second Maven invocation reuses the same target directory, and the parent pom pins project.build.outputTimestamp, so it re-creates a byte-identical jar; locally it took 20 seconds against a 75-second packaging run. The check itself could also report a missing library for a complete jar: `unzip -l | grep -q` leaves unzip killed by SIGPIPE once grep exits on the match, and `pipefail` turns that into a failure. Letting grep read the whole listing fixes it. Against a real 13 MB jar the old form failed 100 times out of 100 locally with PIPESTATUS=141 0, the new form none, and the new form still reports a library that is genuinely absent. Also guard the jar glob so a build that produces no plugin jar, or more than one, fails with a clear message instead of publishing something unchecked. That subsumes the separate count assertion.
53d87c8 to
6d74353
Compare
|
Rebased on main and pushed the review fixes. Since @kazuyukitanimura approved, one thing changed beyond the review comments: #5897 dropped JDK 11, so the Spark 3.4 build in this workflow would now hit the enforcer rule. Every variant is built with JDK 17 and the second JDK install is gone. |
|
@rich7420 I addressed feedback - could you take another look? |
sunchao
left a comment
sunchao
left a comment
There was a problem hiding this comment.
Correctness
This adds a nightly path for testing the development version without building Comet locally. It builds Linux amd64 and aarch64 libraries, bundles both into four Spark/Scala variants, and publishes SNAPSHOT coordinates to the ASF repository. I found no new or remaining P1/P2 issue in the three-file change at 6d7435344a69 against 0ea7bdb0c9af.
The workflow uses the release builder's Ubuntu 20.04 baseline and CPU targets. Each variant starts with a Maven clean, then receives both native libraries before packaging. The SNAPSHOT version guard prevents this path from deploying a release version. The inherited Apache parent POM supplies the snapshot repository and matching server ID. Overriding maven.deploy.skip also publishes the parent POM needed to resolve the child artifacts.
The two issues already discussed are addressed: the library check consumes the full listing, and packaging and validation precede each deploy invocation. In a local check using the exact Bash function and large synthetic jars, the current function accepted 25/25 repeated checks of a valid jar and rejected either missing architecture. Restoring the old early-exit grep caused 25/25 false rejections.
This PR changes publishing and installation instructions, with no expression, operator, or runtime fallback change. It does not alter Spark's null, overflow, ANSI, or type semantics. The four coordinates match the selected Maven profiles. I make no new Spark semantic-compatibility claim, and the maintained Spark 3.4 and 4.1 sources were unavailable for comparison.
Validation
The fork dry-run checked out this exact head in all four jobs. Both native builds succeeded and reported a maximum glibc symbol requirement of 2.30. Producer and consumer artifact IDs and SHA-256 digests match. All four jar variants completed packaging and local installation, and the checked jars were uploaded.
The main PR checks show 9 successes and 12 skips. Preflight ran actionlint on merge 249bae0bfa63, whose parents are the assigned base and head. Its tree differs from the head only in two unrelated rounding test files. The usual Linux, macOS, SQL, and Iceberg jobs were skipped. Seven multiline shell blocks also passed local syntax checks.
The dry-run used install, skipped credential configuration, and skipped tests. It does not validate Nexus authentication, remote deployment, or Spark/JNI execution. The author's package/deploy byte-identity check remains author-reported.
Performance
Native compilation runs once per architecture, and the four JVM variants reuse those libraries. Dependency caching avoids repeated downloads, while cleaning between profiles prevents stale classes from leaking across variants. The second Maven invocation reports 45.2–47.4 seconds per variant in the dry-run. That cost is confined to publishing and supports the pre-publication check. No query-runtime overhead or speedup is introduced, so a Spark microbenchmark would not test this change.
Design
The three-stage flow is easy to follow: decide whether to build, produce the two libraries, then package and publish the variants. Repository gating allows fork dry-runs while excluding fork publishing. There is no pull-request trigger. Nexus credentials are scoped to the Maven step and are referenced through environment variables in the server settings. The workflow token has read-only contents permission.
The schedule uses the default branch, and manual dispatch can select a ref, consistent with GitHub's event model. The 24.5-hour age check is a freshness heuristic, so it does not track the last successful publication. A failed nightly without newer commits needs a manual retry, as the workflow documents. Workflow-wide concurrency prevents simultaneous runs. Publishing the four variants remains sequential and non-atomic, which the existing discussion explicitly acknowledges. These are bounded operational choices rather than a new release-management mechanism.
Abstraction & complexity
The workflow reuses the existing Maven bootstrap and artifact retry actions, the release CPU targets, and Maven's inherited repository configuration. The architecture matrix and four-profile loop avoid duplicating the build logic. The small library-check helper serves both required architectures without adding reusable infrastructure for a single workflow. I found no additional actionable simplification.
|
Thanks for the reviews @kazuyukitanimura @sunchao @rich7420 |
4 participants
Which issue does this PR close?
Closes #5899.
Rationale for this change
Anyone who wants to try a fix or feature before the next release currently has to build Comet from source, including the Rust native library. Publishing nightly SNAPSHOT jars to the ASF snapshot repository gives users and downstream projects a ready-made artifact for the current development version, the same way Spark and Iceberg publish their snapshots.
The repository previously had a tag-triggered GHCR Docker publish workflow, removed in #4241. Its runs had failed for months, at first within a minute of starting and later as
startup_failurefrom unpinned third-party actions, and it built arm64 under QEMU with caching disabled. This workflow avoids all three problems: it only usesactions/*and the repo's own composite actions, builds each architecture on a native runner, and caches the Cargo registry and Maven repository.What changes are included in this PR?
A new scheduled workflow,
.github/workflows/publish_snapshot.yml, with three jobs:changesskips the nightly run whenmainhas had no commits since the previous night, so Nexus does not accumulate identical snapshots. A manual dispatch always runs.nativebuildslibcomet.sofor linux/amd64 onubuntu-24.04and linux/aarch64 onubuntu-24.04-arm. Both build inside anubuntu:20.04container so the library links against glibc 2.31, the same baseline as the release builder indev/release/comet-rm/Dockerfile, and a step fails the job if the library ever requires a newer glibc. The samemake core-*-libstargets as the release are used, so the baseline CPU targets match released jars.deployplaces both libraries underspark/target/classes/org/apache/comet/linux/the waybuild-release-comet.shdoes, then runs./mvnw deployfor the four default variants: Spark 3.4 and 3.5 with Scala 2.12, Spark 4.0 and 4.1 with Scala 2.13. Every variant is built with JDK 17, matchingpr_build_linux.yml.-Dmaven.deploy.skip=falseoverrides the root pom so the parent pom is deployed too, since consumers need it to resolve the child poms and the release publishes it. Each variant is packaged and checked for both bundled native libraries before the publishing goal runs for it, so an incomplete jar fails the job instead of reaching consumers.Credentials come from the
NEXUS_USERandNEXUS_PWrepository secrets that ASF Infra provisions for snapshot publishing, read into asettings.xmlthrough${env.*}rather than written to disk. Theorg.apache:apacheparent pom already maps SNAPSHOT deploys toapache.snapshots.https, so no pom changes are needed. If the secrets are not yet configured on this repository, the deploy step fails at upload and an INFRA ticket is needed.A
dry_runinput onworkflow_dispatchruns the whole pipeline but ends withinstallinstead ofdeployand uploads the jars as workflow artifacts. It is also allowed on forks so the workflow can be exercised before a change lands.The installation guide's snapshot-only section now explains where the snapshots live, which artifacts exist, that they are unreleased builds for testing only, and how to use one with
spark-shelleither by downloading the jar or via--packageswith the snapshot repository. The workflows README lists the new standalone workflow.How are these changes tested?
actionlint,prettier --checkanddev/ci/check-ci-config.pypass locally../mvnw deploy -Dmaven.deploy.skip=false -Pspark-4.1 -DaltDeploymentRepository=...into a file repository confirmed that the parent pom,comet-common,comet-sparkandcomet-spark-integrationare all deployed with timestamped snapshot names.dry_runof the workflow on my fork built both native libraries in the Ubuntu 20.04 containers, passed the glibc check, built all four variants and verified that each jar bundles both libraries before publishing it: https://github.com/andygrove/datafusion-comet/actions/runs/34873305385Cost per night from that run: 20 minutes for the amd64 native build, 17 for aarch64, and 18 minutes for the deploy job's four Maven builds, so roughly 55 runner-minutes and 39 minutes of wall clock. Packaging each variant before publishing it accounts for about 2 minutes of the deploy job; the second Maven invocation reuses the target directory the check ran against.
The first real publish will be a manual
workflow_dispatchafter this merges, followed by checking that the coordinates resolve from the snapshot repository.