Repository navigation
Conversation
…ncy to fix SparkR docs build
### What changes were proposed in this pull request?
Declare `org.objenesis:objenesis` explicitly at version `3.3` in the parent POM's
`dependencyManagement` and add it as a direct dependency of `core`. This backports
the objenesis-handling portion of SPARK-51806 (present on master/branch-4.0) to
branch-3.5. kryo-shaded is left at 4.0.2 and the runtime dependency manifest is
unchanged (it already lists `objenesis/3.3` and `kryo-shaded/4.0.2`).
### Why are the changes needed?
The release documentation build fails during SparkR vignette rendering
(`R/create-docs.sh` -> `pkgdown::build_site`) with:
java.lang.NoClassDefFoundError: org/objenesis/strategy/InstantiatorStrategy
at org.apache.spark.serializer.KryoSerializer.newKryo(KryoSerializer.scala:133)
objenesis is only pulled in transitively via `kryo-shaded`, which declares
objenesis `2.5.1`, whereas Spark resolves objenesis `3.3` through Maven. Because
the version is supplied inconsistently and only transitively, the SBT/coursier
build used by the docs step does not reliably copy objenesis into
`assembly/target/scala-2.12/jars`, so the SparkR-launched JVM cannot load it and
the first Kryo-serialized typed aggregate fails, aborting the docs build.
Making objenesis an explicit, version-pinned direct dependency of `core`
guarantees a single resolved version (3.3) that is always placed on the runtime
classpath by the `assembly` copyDeps step.
### Does this PR introduce any user-facing change?
No. Dev/build-only.
### How was this patch tested?
Confirmed that `build/sbt -Phive "show assembly/dependencyClasspath"` now resolves
`objenesis 3.3` (previously `objenesis 2.5.1`), so the jar is included in the
assembled jars directory that SparkR uses. The checked-in dependency manifest is
unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013cdRUhJPqTt9C1uJFgrx9d
holdenk
changed the title
…uffle fetch decoders FetchShuffleBlocks and FetchShuffleBlockChunks decode an outer element count and eagerly allocate a 2D array of that size. Check the count against the bytes actually received before allocating: each inner group carries its own 4-byte length prefix, so a valid count never exceeds readableBytes() / 4. Uses an explicit comparison since Objects.checkFromIndexSize is unavailable on Java 8. Backport of 9687793 Closes apache#58617 from holdenk/17-validate-fetch-counts-3.5. Lead-authored-by: Holden Karau <holden@pigscanfly.ca> Co-authored-by: Holden Karau <holden.karau@snowflake.com> Signed-off-by: Holden Karau <holden.karau@snowflake.com>
…Delegation Improve Transport Context Stream Manager Delegation by requiring authentication when enabled This is a backport of 3b47eee behind a feature flag since 3.5 is in extended LTS mode. Generated-By: Claude (multiple models) and Cursor with Kimi (K3) Closes apache#58612 from holdenk/f003-r2-branch-3.5. Lead-authored-by: Holden Karau <holden@pigscanfly.ca> Co-authored-by: Holden Karau <holden.karau@snowflake.com> Signed-off-by: Holden Karau <holden.karau@snowflake.com>
String index values are copied verbatim into store keys, so an embedded key separator (0x00) corrupts the index key structure and mixes entries across range scans and deletes. Fail fast with IllegalArgumentException in LevelDBTypeInfo and RocksDBTypeInfo instead. This is a backport of 23fdef7 Closes apache#58616 from holdenk/111-kvstore-index-validation-branch-3.5. Lead-authored-by: Holden Karau <holden@pigscanfly.ca> Co-authored-by: Holden Karau <holden.karau@snowflake.com> Signed-off-by: Holden Karau <holden.karau@snowflake.com>
Better handling for custom log names and URLs Backport of b3149c6 (Closes apache#58699) to branch-3.5. Two deviations from the master commit: the ui-test jest module does not exist on branch-3.5 so the JS unit test is not included, and the escapeHtml helper (added on master by 432adc7, not backported) is included here since formatLogsCells now depends on it. Custom log names can be weird, so let's validate that they're displayable before we display them. Invalid log URLs will be replaced with plain text. New unit tests Generated-by: Claude (mixture of models) and Cursor (Kimi K3) Closes apache#59026 from holdenk/f026-validate-custom-log-names-3.5-backport. Authored-by: Holden Karau <holden@pigscanfly.ca> Co-Authored-by: Holden Karau <holden.karau@snowflake.com> Signed-off-by: Holden Karau <holden.karau@snowflake.com>
…ory server ### What changes were proposed in this pull request? Backport of SPARK-59407 to branch-3.5, based on master commit 38258cb (apache#58700). Skip excessively long event log lines during History Server replay: `ReplayListenerBus` reads lines with a per-line byte cap (`spark.history.fs.eventLog.maxLineLength`, default 512m) instead of materializing unbounded lines; over-long lines are drained and skipped with a warning. Branch adaptations vs master: the config declares `.version("3.5.10")` and drops `.withBindingPolicy(...)` (ConfigBindingPolicy exists only on 4.2+); the new warning uses plain string interpolation since branch-3.5 predates the structured logging framework. ### Why are the changes needed? Individual corrupt or unexpectedly large event log lines can make loading a previous Spark job in the History Server slow or exhaust memory; bounding the per-line length keeps replay memory bounded. ### Does this PR introduce _any_ user-facing change? Yes: new configuration `spark.history.fs.eventLog.maxLineLength` (default 512m). Event log lines longer than the cap are skipped with a warning instead of being read into memory during replay; setting it to 0 or a negative value disables the limit. ### How was this patch tested? New test in `ReplayListenerSuite` covering that an over-long line is skipped while neighboring events still replay. Full core module test suite run locally on this branch, green. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude and Cursor Made with [Cursor](https://cursor.com) Closes apache#59058 from holdenk/f040-compressed-file-handling-branch-3.5. Lead-authored-by: Holden Karau <holden@pigscanfly.ca> Co-authored-by: Holden Karau <holden.karau@snowflake.com> Signed-off-by: Holden Karau <holden.karau@snowflake.com>
…TA_BYTE_ARRAY reader ### What changes were proposed in this pull request? Validate and fast fail invalid prefix lengths while reading parquet data. Backport 08d777a (note: upstream includes geo fix, but there is no geo read path in 3.5) ### Why are the changes needed? The prefix length in a DELTA_BYTE_ARRAY page is read from the file and was used to copy bytes out of the previous value without validation. Reject negative prefix lengths and prefix lengths larger than the previous value with a ParquetDecodingException, so corrupt pages fail the read deterministically instead of producing malformed values or hard-to-diagnose low-level exceptions. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? New unit test ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude (mixture of version) & Cursor (with Kimi K3). Closes apache#59050 from holdenk/33-parquet-delta-page-validation-branch-3.5. Lead-authored-by: Holden Karau <holden@pigscanfly.ca> Co-authored-by: Holden Karau <holden.karau@snowflake.com> Signed-off-by: Holden Karau <holden.karau@snowflake.com>
…s in UI ### What changes were proposed in this pull request? Make more side-effect causing behavior in the Spark Web UI POST v.s. GET/HEAD when not deployed on YARN. Also validate that the requests have a CSRF token. Even in YARN deployments GETs which have pre-fetch headers (`x-moz`: `prefecth`) will be rejected with 403. `app/kill`, `driver/kill`, `app/hold`, `app/resume` all require CSRF tokens now, however `worker/kill` does not as it is an exposed API for operators. Missing CSRF tokens will 403 & a HEAD/GET request will 405 unless deployed on YARN or fallback behavior is explicitly enabled. Backport of c4ef971 ### Why are the changes needed? Browser pre-fetch and agents making assumptions on POST v.s. GET "verbs." In general GETs should be side-effect free which is why things like crawlers browsers and agents may choose to prefect them which when the GET is not side-effect free causes undesired behavior. ### Does this PR introduce _any_ user-facing change? Yes new config flag, also pressing back on POST pages often behaves differently in browsers. ### How was this patch tested? New UI test, both "regular" in MasterWebUISuite and UISuite but also added additional selenium tests. Existing UI tests still function :) ### Was this patch authored or co-authored using generative AI tooling? Generated-By: Claude (mixutre of models) and Cursor (Kimi K3) Closes apache#59030 from holdenk/42-62-super-branch-3.5. Lead-authored-by: Holden Karau <holden@pigscanfly.ca> Co-authored-by: Holden Karau <holden.karau@snowflake.com> Signed-off-by: Holden Karau <holden.karau@snowflake.com>
Drop invalid links rather than break rendering Application and worker UI addresses come from external registrants and could contain bad/invalid info. Filter it out to prevent the UI from breaking. No New UIUtilSuite test Yes Generated-by: Cursor (Kimi K3) Closes apache#58622 from holdenk/improve-ui-link-rendering-part2. Lead-authored-by: Holden Karau <holden@pigscanfly.ca> Co-authored-by: Holden Karau <holden.karau@snowflake.com> Signed-off-by: Holden Karau <holden.karau@snowflake.com> (cherry picked from commit e91d22d) Signed-off-by: Holden Karau <holden.karau@snowflake.com>
Co-authored-by: Holden Karau <holden@pigscanfly.ca> Co-authored-by: Holden Karau <holden.karau@snowflake.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR explicitly declares the
objenesisdependency in both the parent POM and the core module to ensure it is reliably present on the runtime classpath. This fixes an issue where the SBT/coursier build could drop objenesis from assembled jars, breaking Kryo functionality (particularly during SparkR vignette rendering in the release docs build).Changes
pom.xml): Added explicitobjenesis3.3 dependency declaration with version management. Includes detailed comment explaining the version mismatch between Kryo 4.0.x (which depends on objenesis 2.5.1) and Spark's dependency on objenesis 3.3.core/pom.xml): Added explicitobjenesisdependency (version inherited from parent) to ensure it is reliably resolved and copied onto the runtime classpath by build tools like SBT'sassemblycopyDeps.Details
The root cause is a transitive dependency version conflict: Kryo 4.0.x pulls in objenesis 2.5.1, while Spark explicitly depends on objenesis 3.3. The SBT/coursier build can drop the lower-priority transitive dependency from the assembled jars directory, which breaks Kryo since it cannot find objenesis at runtime. By explicitly declaring objenesis as a direct dependency in the core module with the correct version (3.3), we ensure Maven/SBT properly includes it in the classpath.
Fixes SPARK-51806.
https://claude.ai/code/session_013cdRUhJPqTt9C1uJFgrx9d