[GLUTEN-12807][CORE] Deprecate and remove Spark 3.3 support - #12902
[GLUTEN-12807][CORE] Deprecate and remove Spark 3.3 support#12902LuciferYang wants to merge 10 commits into
Conversation
Also removes spark-3.3 from the format script's profile list first, since every later step runs it and the enforcer's requireActiveProfile would reject an unknown profile once the poms no longer define it.
Removes shims/spark33, gluten-ut/spark33, and the delta 2.3 / iceberg 1.5 source directories that only the spark-3.3 profile selected. Also drops gluten-iceberg/src-iceberg3, which was already dead: iceberg.binary.version was never set to 3 and the file was a byte-identical copy of the src-iceberg5 one. LICENSE loses the 12 entries that pointed into the removed directories.
Drops the two spark-test-spark33 jobs and the shims33 change-detection flag they were the only consumers of, removes 3.3 from the two TPC matrices, and moves the four Spark-3.3-only jobs (OOM, random-kill, uniffle, celeborn) to 3.5 so those scenarios keep their only coverage. Their step names said spark3.3 too.
…ing chain The ClickHouse release chain hardcoded 3.3: package.sh built only that version and gluten.sh only recognised jars/spark33, so it would have broken outright. Moves both to 3.5 and settles the 'TODO: support spark 3.5 later' next to it.
Merges the two version-split sections in velox-backend-limitations.md, keeping the 3.4+ behaviour that is now the floor.
Six places updated a version marker without the thing it governed: the m2-cache Dockerfile's dangling continuation swallowed WORKDIR into the RUN, package-release.sh still demanded a 3.3 bundle jar that can no longer be built, build-release.sh kept a Java-8 gate with no Java-8 build left, ClickHouse.md paired Spark 3.5.5 with Delta 2.2.0, clickhouse-backend-debug.md kept spark.test.home at spark33, and the nightly bundle jobs built 3.5 twice.
|
Run Gluten Clickhouse CI on x86 |
github-actions
Bot
added
CORE
|
|
||
| # Spark 4.0 | ||
| $MVN_CMD -Pjava-17,scala-2.13,spark-4,backends-velox,hadoop-3.3,spark-ut \ | ||
| $MVN_CMD -Pjava-17,scala-2.13,spark-4.0,backends-velox,hadoop-3.3,spark-ut \ |
There was a problem hiding this comment.
Drive-by fix, unrelated to Spark 3.3, flagged here because it sits in the same RUN as the continuation fix below.
spark-4 is not a profile id. The root pom defines spark-4.0 and spark-4.1, so Maven printed The requested profile "spark-4" could not be activated because it does not exist. and carried on with the root defaults. enforce-spark-profile does not catch it either: dependency:go-offline invokes the plugin goal directly rather than running a lifecycle, and that execution binds to validate, so it never fires. I confirmed the command exits 0 with only the warning.
Falling back to the defaults gives a mix that is mostly 3.5: sparkshim.artifactId resolves to spark-sql-columnar-shims-spark35, sparkbundle.version to 3.5, delta.version to 3.3.2. Only spark.version happens to match, since the root default is already 4.0.2, so the Spark 4.0 jars did land in the cache while the Delta 4.0.1 and spark40 shim coordinates did not. That is the whole cost: no red build, just a cache image that does not hold what this stanza was meant to pre-populate for the 4.0 jobs.
Introduced in #11655. Happy to pull it into its own PR if you would rather keep this one to the removal.
There was a problem hiding this comment.
Split out to #12903 and reverted here, so this PR's only change to this file is the removal of the Spark 3.3 stanza plus the trailing ; \ that removal left dangling.
It is pre-existing (apache#11655) and adds behavior rather than removing it, so it does not belong in a removal PR. Split into its own MINOR PR.
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
| # Run test to generate input data files. If you are using spark 3.4, replace -Pspark-3.5 with -Pspark-3.4. | ||
| mvn test -Pspark-3.5 -Pbackends-velox -pl backends-velox -am \ | ||
| -DtagsToInclude="org.apache.gluten.tags.GenerateExample" -Dtest=none -DfailIfNoTests=false -Dexec.skip |
Both carried the short Apache notice instead of the ASF header. dev/check.py only inspects files a PR changes, so touching these two for the profile removal is what surfaced it. Replaced the short block rather than stacking the ASF one on top of it, which is what the checker's own fix does.
|
Run Gluten Clickhouse CI on x86 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 161 out of 369 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
ep/build-clickhouse/src/package.sh:41
- This script calls Maven via the system
mvn, which can pick up an unexpected Maven/JVM configuration and diverge from the repo’s pinned build wrapper. Use the repo wrapper (./build/mvn) here to make the packaging build reproducible and consistent with the rest of Gluten’s build tooling.
|
The ClickHouse CI failure here is not from this change's content. That Jenkins job invokes The command line lives in the job configuration rather than in this repository. I grepped for One detail for whoever picks this up: the reactor in that log already lists @jackylee-ch could you help find someone with access to update that job, or point me at where it is configured? |
|
@zzcclp PTAL. We are removing Spark 3.3 supported in Gluten, but there are still some ClickHouse Spark 3.3 CI exists. Can we remove them in Gluten ClickHouse CI? |
OK, I will remove them later |
|
Run Gluten Clickhouse CI on x86 |
# Conflicts: # docs/developers/dev-container.md
|
Run Gluten Clickhouse CI on x86 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 161 out of 369 changed files in this pull request and generated no new comments.
Suppressed comments (1)
ep/build-clickhouse/src/package.sh:41
- This script invokes Maven directly (e.g.,
mvn ...in get_project_version and build_gluten_by_spark_version). Repository guidance expects using the./build/mvnwrapper to ensure the pinned Maven version and JVM flags are applied; using plainmvncan make packaging results non-reproducible across environments.
|
@jackylee-ch @zhouyuan @philo-he Could you please help review this PR? The failed GHA task is unrelated to the current PR, but I do not have permission to trigger it manually. Thanks |
What changes are proposed in this pull request?
Removes the Spark 3.3 build surface. This covers three items in #12807: the Spark-33 unit tests, the Spark-33 shim source tree, and the building scripts. Two follow-ups remain, described at the end.
Seven directories go, 328 files and about 41k lines:
shims/spark33,gluten-ut/spark33,backends-clickhouse/src-delta23,gluten-delta/src-delta23,backends-clickhouse/src-iceberg-spark33,gluten-iceberg/src-iceberg5,gluten-iceberg/src-iceberg3. Thespark-3.3profile is gone from the six poms that declared one, and the root pom'srequireActiveProfilelist now names only the four surviving versions, so-Pspark-3.3fails withMissing spark version profileinstead of quietly resolving to whatever the defaults are.gluten-iceberg/src-iceberg3needs a word since its name does not mention 3.3: no profile ever seticeberg.binary.version=3, so no build could reach it, and at the base commit its two files were byte-identical tosrc-iceberg5's. It was already dead, andsrc-iceberg5was reachable only from the 3.3 profile, so both go together.LICENSEcarried twelve entries pointing intoshims/spark33/andsrc-delta23/. Those are removed, and the remaining 24 paths were checked to still resolve.Four CI jobs ran only on Spark 3.3 and are the sole coverage for what they exercise, so they are retargeted to 3.5 rather than deleted:
tpc-test-ubuntu-oom,tpc-test-ubuntu-randomkill,tpc-test-centos8-uniffle,tpc-test-ubuntu-2204-celeborn. For celeborn the gluten-it install command moved with it, and it now takes the profile frommatrix.sparklike the Gluten build two steps above it.spark-test-spark33andspark-test-spark33-sloware deleted outright: 3.4 and 3.5 run the same profile set (-Piceberg -Pdelta -Phudi -Ppaimon), so no combination is orphaned. Theshims33change-detection flag and its path filter go with them.The ClickHouse release chain hardcoded 3.3 end to end:
ep/build-clickhouse/src/package.shbuilt only that version andbin/gluten.shonly recognisedjars/spark33, so leaving it would have shipped a tarball whose launcher cannot find its own jars. Both move to 3.5, which also settles the# TODO: support spark 3.5 latersitting next to it. I have no ClickHouse environment, so that part rests on reading andbash -n. The ClickHouse CI triggers itself here (this PR touchespom.xml,backends-clickhouse/**,shims/**and more, which the trigger workflow's path filters match), but a CH maintainer's eyes on the packaging change would still help.Four things reviewers should know.
install-spark-resources.shno longer installs the 3.3 distribution, sodocker_image.ymlwill rebuild the images and/opt/shims/spark33/spark_homedisappears from them. Nothing in the repo references that path, but a branch that still does will break once the new image is published.The ClickHouse CI is red on this PR for a reason that cannot be fixed from here. That job is a Jenkins build outside this repository (its workspace is
.../gluten/gluten-ci/, and its parent pom isio.kyligence:devopslib), and it invokesmvn clean test -Pbackends-clickhouse -Pspark-3.3 -Pjava-8 ... -Dspark.test.home=/tmp/spark33. With the profile gone,enforce-spark-profilestops it before anything compiles:Missing spark version profile: -Pspark-<version>. Nothing in apache/gluten generates that command line, so whoever owns the job needs to move it to-Pspark-3.5and a Spark 3.5spark.test.home, which is the only version the ClickHouse backend has a profile for anyway. Worth noting that its reactor listing already showsGluten Shims for Spark 3.5, so that environment appears to activatespark-3.5from its own settings rather than from the command line. Because it fails at the first enforcer rule, that run says nothing yet about whether the ClickHouse changes in this PR are sound.dev/release/build-release.shhad a "Java 8 is required" gate whose only purpose was the Spark 3.3 build directly below it. With that build gone nothing in the script needs Java 8 (the four remaining builds all pass-Pjava-17, and the script installs Corretto 17 a few lines later), while the gate still rejects a release manager on JDK 17, and it does so after the longest step in the script. The design note for this work listed it as out of scope; I removed it here instead, since this change is what made it vestigial.dev/release/package-release.shneeded the same treatment for a different reason: itsSPARK_VERSIONSlist still demanded a 3.3 bundle jar that can no longer be built, and it hard-fails on a missing jar before it packages anything, so a release cut would have produced zero artifacts.Five test cases are pinned to 3.3 with
testWithMaxSparkVersion/testWithSpecifiedSparkVersion, and those helpers skip thetest()call entirely when no version matches, so from this PR until PR 3 they run nowhere and leave no skip marker:VeloxParquetWriteForHiveSuite'shive.exec.compress.outputcase,ClickhouseGlutenKafkaScanSuite's only case, and three inGlutenClickHouseNativeWriteTableSuite. Two carry TODOs saying they fail on 3.5, so each needs a decision (port or delete plus an issue) rather than a substitution, which is why they belong to PR 3.Deliberately left alone: the nineteen
ignoreClassentries inpackage/pom.xmlunder "The overridden class list by Gluten" are now inert, because all five classes they name were overridden only inshims/spark33, butbanDuplicateClassesruns withfail=trueand dropping the entries could surface a duplicate from a dependency jar, so that is its own call.tools/gluten-it/pom.xmlis a standalone pom withoutrequireActiveProfile, so-Pspark-3.3there now warns and builds 3.5 rather than failing; adding the same gate would also outlaw the profile-less build that works today. Andtools/workload/benchmark_velox/initialize.ipynbstill provisions Spark 3.3.1 across five coupled lines including a symlink; I cannot test that notebook, so I left it rather than half-update it.Follow-ups under #12807: PR 2 lifts the shim methods that existed only for 3.3, PR 3 removes the residual version checks (
eqSpark33,AlignExpandOutputTypes, theNativeWritePostRuleregistration, thespark33helper in the CH tests) and handles the five pinned cases above.How was this patch tested?
Local verification, all against this branch:
mvn validate -Pspark-3.3Missing spark version profile, as intendedtest-compilefor 3.4, 3.5, 4.0, 4.1-Pceleborn,uniffle -Piceberg,delta,hudi,paimon)-Pbackends-clickhousebuildsrc-deltadoes not reference anything that lived only insrc-delta23dev/format-scala-code.sh --checkMissing spark version profiledocker buildx build --checkonDockerfile.ubuntu22-m2-cacheNoEmptyContinuationwarning, byte-for-byte the same warning the pre-change file producesbash -non every changed scriptOne caveat on the local runs: the 3.5 builds needed
-Pscala-2.13to get around corruptedspark-*_2.12/3.5.5jars in my local~/.m2, and that combination has to skip-Ppaimonbecause upstream never publishedpaimon-spark-3.5_2.13. The 2.12 path is what CI exercises.The license header check needs a note, because I got it wrong at first. It inspects every file a PR changes, not only the files it adds, so removing the
spark-3.3profile fromshims/pom.xmland the stale clean exclude frompackage/pom.xmlput both under the check, and both carried the short Apache notice rather than the ASF header. They now carry the ASF header, matchinggluten-ut/pom.xmland the root pom. I replaced the short block instead of stacking the ASF one above it, which is whatcheck.py's own fix mode does and would have left two license blocks in each file.bash .github/workflows/util/check.sh <base>exits 0 locally after the change (it needspip install regex).Unit tests are what CI settles, not a local run. The ClickHouse packaging changes are the part I am least able to verify myself.
Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude claude-opus-5