mattcasters opened a new pull request, #7482: URL: https://github.com/apache/hop/pull/7482
# Code review report: issue #7476 — Spark / Beam validation **Branch:** `issue-7476` **Issue:** [#7476 — Validate Spark functionality on Beam](https://github.com/apache/hop/issues/7476) **Base:** `main` @ `02b72d78af` **Commits (oldest → newest):** | SHA | Summary | | --- | --- | | `a33b663f6c` | Re-enable Spark IT, pin 3.5.7, matrix scaffolding | | `d3cb207381` | First matrix report + matrix script hardening | | `670ba47518` | Versioned Spark client packs + fat-jar selection | | `fce6f6dd84` | Java 21 / SPARK-42369 conclusion; classpath fixes | | `ccc2cb5052` | Apache RAT exclusion for matrix markdown | **Diff size:** ~41 files, +2212 / −972 lines --- ## 1. Executive summary for reviewers This work answers: *What Spark versions actually work with current Hop (Java 21) + Beam 2.74 client-mode submit?* ### Measured conclusion | Finding | Detail | | --- | --- | | **Supported** | **Spark 3.5.x** (validated **3.5.7**) | | **Not supported on Java 21** | Spark **3.4.x and earlier** — `NoSuchMethodException: DirectByteBuffer.<init>(long,int)` ([SPARK-42369](https://issues.apache.org/jira/browse/SPARK-42369), fixed in Spark **3.5.0**) | | **Client/cluster match** | Driver + fat jar Spark client libraries must match cluster minor; skew → `InvalidClassException` / serialVersionUID | | **Matching packs cannot save 3.4** | Matched client 3.4.4 + cluster 3.4.4 still fails on Java 21 | Hop requires Java 21 for the driver **and** for workers (fat-jar bytecode). The Spark cluster must therefore also run Java 21 → **Spark ≥ 3.5.0 is the platform floor**. ### What changed in product code 1. **Re-enabled** `integration-tests/spark` smoke (removed `disabled.txt`). 2. **Shipped Spark runtime** (`spark-core` / `spark-streaming` + network jars) as a **default client pack** under `lib/spark-client/`. 3. **Driver classpath** includes `lib/spark-client/*` in all hop launch scripts. 4. **Optional versioned packs** (`lib/spark-clients/<V>/`) + fat-jar flag `--spark-client-version` for 3.5.x alignment (and future matrix cells). 5. **Docs + MATRIX-REPORT\*.md** document measured support (not aspirational version tables). 6. **Unit test** `BeamSparkPipelineEngineTest` re-enabled (`local[2]`, UI disabled). --- ## 2. Commit walkthrough ### `a33b663f6c` — Spark IT revived, pin 3.5.7 **Intent:** Get a single green path and a way to measure other cluster versions. | Area | Change | | --- | --- | | Beam POM | `spark-core` / `spark-streaming` as **compile** (not test-only); exclusions for log4j / jackson-scala preserved | | Docker Spark images | Temurin **21**, `ARG SPARK_VERSION`, `archive.apache.org`, healthcheck | | Compose | Slim stack (master + worker only); service name `hop-spark-test` (no underscores — Spark RPC rejects them); `spark.ui.enabled=false`; `SPARK_MASTER_HOST=spark` | | IT project | Remove `disabled.txt`; fix `tempLocation`; defer Hive workflows to `optional-*` (not `main*.hwf`) | | Scripts | `run-spark-matrix.sh`; `SPARK_VERSION` plumbing in `run-tests-docker.sh` | | Unit test | Re-enable with `local[2]` + Spark UI off | ### `d3cb207381` — MATRIX-REPORT + matrix hardening - Curl retries on Spark download - Hadoop binary suffix map (3.2.x → `bin-hadoop3.2`) - Per-version timeout; better surefire PASS/FAIL detection (`<failure>` tags) - First empirical report artifact ### `670ba47518` — Versioned Spark client packs | Area | Change | | --- | --- | | Assembly | Spark runtime jars → `lib/spark-client/` (not flat `lib/beam/`) | | Launch scripts | Classpath selection via `HOP_SPARK_CLIENT_VERSION` | | `HopBeamGuiPlugin` | Pack-aware jar discovery for fat-jar generation | | `GenerateFatJarConfigPlugin` | `--spark-client-version` / `-scv` | | Tooling | `tools/spark-client-pack/` (Maven POM + `materialize-pack.sh`) | | IT | `run-spark-matched-matrix.sh`; beam image build-arg for pack-aware fat jar | ### `fce6f6dd84` — Java 21 conclusion + driver classpath reliability Critical runtime fixes after matrix failures: - **Root cause of `JavaStreamingListener` / `Source` CNFEs:** packaged `hop-run.sh` never put `lib/spark-client` on the main `CLASSPATH`; versioned pack path was also wiped by unzip-before-docker. - **Fix:** bake `lib/spark-client/*` into primary classpath; after unzip, rematerialise pack and **copy into `lib/spark-client`**; invalidate base/beam images when pack version set. - Docs + reports: **3.5.x only** on Java 21; cite SPARK-42369. - Default matched matrix versions → `3.5.7` only. ### `ccc2cb5052` — RAT - Exclude matrix markdown from Apache RAT (`pom.xml`). --- ## 3. Architecture notes (review focus) ### Install layout ```text lib/beam/ # Beam runners (+ Beam’s spark-common-utils / sql-api fragments) lib/spark-client/ # DEFAULT runtime pack (3.5.7) lib/spark-clients/V/ # Optional packs (materialised on demand) ``` **Driver:** `hop-run` / `hop-gui` classpath includes `lib/spark-client/*` (or versioned pack if present). **Workers:** fat jar from `hop-conf --generate-fat-jar` with optional `--spark-client-version=V`. **Rule:** driver pack + fat jar pack + cluster minor should match. ### Why not rebuild all of Hop with `-Dspark.version=X`? Packs allow one Hop build + optional client sets. Full multi-version Maven rebuilds are expensive and don’t help a released tarball. Packs still matter for **3.5.x patch** alignment; they do **not** restore 3.4 on Java 21. ### IT flow (matched) 1. Unzip client zip 2. `materialize-pack.sh V` → `lib/spark-clients/V` 3. Copy pack → `lib/spark-client` (driver) 4. Rebuild `hop-base-image` + `hop-beam-image` (fat jar with `--spark-client-version=V`) 5. Start cluster `SPARK_VERSION=V` 6. Smoke `main-0001-test-spark-cluster` --- ## 4. Test evidence | Test | Result | | --- | --- | | `BeamSparkPipelineEngineTest` (`local[2]`) | PASS (with `spark.ui.enabled=false`) | | IT smoke, cluster **3.5.7**, default/matched client | PASS | | Cluster 3.2.4 on Java 21 | FAIL — master `DirectByteBuffer` | | Cluster 3.3.4 vs client 3.5.7 | FAIL — serialVersionUID skew | | Cluster 3.4.4 on Java 21 | FAIL — executor `DirectByteBuffer` | | Matched client **3.4.4** + cluster **3.4.4** | FAIL — same Java 21 `DirectByteBuffer` ([SPARK-42369](https://issues.apache.org/jira/browse/SPARK-42369)) | Artifacts: `integration-tests/spark/MATRIX-REPORT.md`, `MATRIX-REPORT-MATCHED.md`. **Reproduce smoke:** ```bash ./mvnw -pl plugins/engines/beam,assemblies/static,assemblies/client -am package -DskipTests # install beam zip into m2 before client if needed: # ./mvnw -pl plugins/engines/beam install -DskipTests SPARK_VERSION=3.5.7 ./integration-tests/scripts/run-tests-docker.sh \ PROJECT_NAME=spark KEEP_IMAGES=true ``` --- ## 5. Review checklist ### Correctness / design - [ ] **Agree** that Java 21 + SPARK-42369 makes Spark **3.5.0+** the support floor for Hop Beam client-mode. - [ ] **Classpath:** shipping Spark in `lib/spark-client` and always adding it to hop launch scripts is acceptable (size / dependency footprint). - [ ] **Fat-jar selection:** versioned packs correctly exclude `lib/beam/spark-*.jar` fragments so they don’t mix with an alternate pack (see `HopBeamGuiPlugin.findInstalledJarFilenames`). - [ ] **Hive ITs** renamed to `optional-*` (not run by `main*.hwf` discovery) — intentional deferral, not silent delete. - [ ] **Docker service rename** `integration_test_spark` → `hop-spark-test` (underscores invalid in Spark RPC hostnames). ### Security / supply chain - [ ] Spark distros downloaded from `archive.apache.org` in Dockerfiles (checksums **not** pinned — optional follow-up). - [ ] Pack materializer resolves Maven Central with existing project exclusions (log4j, etc.). ### Ops / maintainability - [ ] `run-spark-matrix.sh` vs `run-spark-matched-matrix.sh` naming/docs clear enough. - [ ] Default CI path should stay **one** version (`3.5.7`) — full matrix is manual/expensive. - [ ] Large `.bat` diffs are mostly line-ending / block rewrite noise — focus review on the **classpath logic**, not every bat line. ### Docs - [ ] Supported-versions table now says **3.5.x (3.5.7 PASS)** for Hop 2.19 / Beam 2.74. - [ ] Engine adoc no longer implies pre-3.5 Spark works with Hop on Java 21. ### Gaps / follow-ups (out of this PR unless reviewers want them) - Checksums for Spark tarball downloads in Dockerfiles. - GUI fat-jar “Spark client version” picker (CLI/env only today). - Thin fat jar / spark-submit mode (`--exclude-spark-client`) — related to #2612. - Pin CI daily job to `PROJECT_NAME=spark` + `SPARK_VERSION=3.5.7` if not already enabled. - Flink re-enable remains separate (`integration-tests/flink/disabled.txt`). --- ## 6. Key files to read first | Priority | Path | Why | | --- | --- | --- | | P0 | `integration-tests/spark/MATRIX-REPORT.md` | Measured conclusions | | P0 | `plugins/engines/beam/pom.xml` | Spark as runtime dependency | | P0 | `plugins/engines/beam/src/assembly/assembly.xml` | `lib/spark-client` packing | | P0 | `assemblies/static/.../hop-run.sh` | Driver classpath | | P1 | `plugins/engines/beam/.../HopBeamGuiPlugin.java` | Fat-jar jar selection | | P1 | `plugins/engines/beam/.../GenerateFatJarConfigPlugin.java` | CLI flag | | P1 | `tools/spark-client-pack/*` | Pack materializer | | P1 | `docker/integration-tests/integration-tests-spark.yaml` | IT topology | | P1 | `integration-tests/scripts/run-tests-docker.sh` | Pack rehydrate after unzip | | P2 | Docs under `docs/hop-user-manual/.../beam*` | User-facing claims | --- ## 7. Risk summary | Risk | Severity | Mitigation in PR | | --- | --- | --- | | Larger install (Spark jars ~15MB+ network deps) | Medium | Required for client-mode; was missing before and broke Spark | | Classpath / pack wiring regressions | Medium | Smoke IT on 3.5.7; materialise+activate pack in docker path | | Over-claiming Spark support | Low | Docs + matrix reports explicitly limit to 3.5.x / Java 21 | | Bat file noise | Low | Behavioural change is classpath only | --- *Report generated for branch `issue-7476` / commits `a33b663f6c`…`ccc2cb5052`.* -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
