voonhous commented on PR #19674:
URL: https://github.com/apache/hudi/pull/19674#issuecomment-5350976850

   Reviewed the pom, CI wiring, release scripts and validation. Shading deltas, 
comet coordinates, LICENSE/NOTICE, and the skip/deploy mechanics all check out. 
One real bug and a few gaps:
   
   1. **Blocker:** the inherited `com.codahale.metrics.` relocation rewrites 
Comet's `org.apache.spark.CometSource$` (implements Spark's `Source`), so 
`spark.comet.metrics.enabled=true` fails with `AbstractMethodError` at 
SparkContext init (verified via javap on the shaded output signature vs Spark's 
interface). Same bug class as the flink dropwizard fix (#18982). Drop the 
metrics include+relocation in this module, or document metrics as unsupported 
and pin the behavior in validate.sh.
   2. **Release risk:** native 3.4/4.0/4.1 bundles are never built by any CI 
lane -- the `-pl` add in `validate-bundle-spark4` is a no-op (its only live 
matrix entry is spark4.2, where the module skips). First build is release day 
(cf. HUDI-7766). Add build-only matrix entries.
   3. **Release validation:** the 5 staged coordinates are existence-checked 
only (`validate_staged_bundles.sh`); no workflow runs them from staging, so 4 
of 5 published bundles never execute anywhere.
   4. **Coverage:** the module README documents MOR bridging 
(`CometSparkRowToColumnar`) but validation is COW-only. Make one table MOR and 
assert the plan, like `CometSortMergeJoin` is asserted today.
   5. **CI cost:** the module shades on every full-reactor lane (~88MB comet 
download + ~200MB jar per PR run); the Azure jacoco module list also lacks the 
exclusion.
   6. **CI cost:** per-run 400MB Spark tarball from archive.apache.org. Use a 
pushed glibc base image like `bundle-validation/base`, or dlcdn + Spark 3.5.9 
(what comet 1.0.0 compiles against; dlcdn dropped 3.5.5).
   
   Minor: no scala-2.13 branch in the Dockerfile/jar glob; README should warn 
against cluster-provided Comet jars (first-jar-wins plus libcomet 
classpath-resource resolution); `-Dspark3` alias row missing from the README 
table (it activates no comet profile); `packaging/bundle-validation/README.md` 
not updated; `SPARK_RUNTIME` value grammar and `CONTAINER_NAME` naming nits; 
Comet's arrow/guava `META-INF/maven` `pom.properties` ship in the jar while the 
classes are relocated.
   


-- 
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]

Reply via email to