[WIP][SPARK-54571][CORE] Use LZ4 safeDecompressor

[dbtsai]

What changes were proposed in this pull request?

In recent LZ4 versions, safeDecompressor has become highly optimized and can be as fast, or even sometimes faster, than fasterDecompressor. So it does make sense to switch to safeDecompressor.

Why are the changes needed?

It is recommended to switch to .safeDecompressor(), which is not vulnerable and provides better performance per https://sites.google.com/sonatype.com/vulnerabilities/cve-2025-12183

Does this PR introduce any user-facing change?

No

How was this patch tested?

Unit tests

Was this patch authored or co-authored using generative AI tooling?

No

[dbtsai]

It's more involving than I thought as LZ4BlockInputStream doesn't take safeDecompressor. I will take a deeper look tomorrow.

[dongjoon-hyun]

Ya, please make the CI happy. Thank you, @dbtsai .

[LuciferYang]

It seems that the fix for CVE‐2025‐12183 wasn't implemented until version 1.8.1, but Spark is still using version 1.8.0.

https://github.com/yawkat/lz4-java/releases

Spark still use 1.8.0

[yawkat]

Note that LZ4BlockInputStream does not support safeDecompressor in lz4-java 1.8.1. If you upgrade to that version, it will still work and be secure, but performance will be much worse than in 1.8.0.

lz4-java 1.10.0 introduces a new builder for LZ4BlockInputStream that accepts a safeDecompressor.

[dongjoon-hyun]

To reviewers, just for the record, LZ4 1.10.0 is not published to the Maven Central yet.

[yawkat]

It is published, but only under the new group id

[SteNicholas]

@yawkat, which group id does 1.10.0 publish?

[LuciferYang]

we need change to use

<dependency>
    <groupId>at.yawk.lz4</groupId>
    <artifactId>lz4-java</artifactId>
    <version>1.10.0</version>
</dependency>

https://repo.maven.apache.org/maven2/at/yawk/lz4/lz4-java/

[dongjoon-hyun]

Thank you for the updated info, @yawkat , @LuciferYang .

Could you update this PR, @dbtsai ?

[dongjoon-hyun]

To all, in order to help this PR, I made an independent PR for dependency upgrade.

• [SPARK-54597][BUILD] Upgrade lz4-java to 1.10.0 #53327

[dongjoon-hyun]

We upgraded to 1.10.0 a few minutes ago. Could you rebase this PR to master branch once more, @dbtsai ?

• [SPARK-54597][BUILD] Upgrade lz4-java to 1.10.0 #53327

[yawkat]

I recommend you wait a few hours with releasing this. Another (smaller, unrelated) CVE has been found in lz4-java.

[yawkat]

CVE-2025-66566 has been published and fixed in 1.10.1. I suggest you move to that version. Though cloudflare seems to be having some trouble that breaks maven central at the moment.

[dongjoon-hyun]

I recommend you wait a few hours with releasing this. Another (smaller, unrelated) CVE has been found in lz4-java.

Just FYI, we have no intention to hurry this, @yawkat . To be safe, this will be tested in master branch for next few months in Apache Spark 4.2.0 timeframe. Currently, we are voting on Apache Spark 4.1.0, it's too late for this kind of risky dependency change.

That's the main reason why LZ4 1.10.0 PR is only in master branch. Dependency change and code change is only building a ground to test further in various workloads.

• [SPARK-54597][BUILD] Upgrade lz4-java to 1.10.0 #53327

[dongjoon-hyun]

Gentle ping once more, @dbtsai .

[dongjoon-hyun]

Gentle ping, @dbtsai .

[mridulm]

The PR description mentions that safeDecompressor is fast enough.
While the CVE is a good enough reason to switch - can we link benchmark results to the PR description to point to this ?

[yawkat]

I am not aware of spark benchmarks, but as of 1.8.1, safeDecompressor is substantially faster than fastDecompressor. In earlier versions, the difference was minor.

[SteNicholas]

@yawkat, how about the performance for 1.10.0? @dongjoon-hyun has already bumped to 1.10.0.

[yawkat]

Performance between 1.8.1 and 1.10.1 has not changed substantially.

[mridulm]

@yawkat I am not questioning whether it is faster or not - I am sure it is, else @dbtsai wont be raising this PR :)
Given the PR description, adding a reference, which makes this clear would help.

[pan3793]

@mridulm I found a perf report at yawkat/lz4-java#3 (comment), but without providing the data.

[pan3793]

@mridulm @yawkat @dbtsai @dongjoon-hyun @SteNicholas I created #53453 to add an lz4 benchmark based on TPCDS catalog_sales.dat (SF1), the size is about 283M. My test results show lz4-java 1.10.1 is about 10~15% slower on lz4 compression than 1.8.0, and is about 5% slower on lz4 decompression even with migrating to suggested safeDecompressor (#53454)

[yawkat]

The underlying lz4 library was updated in 1.9.0 so a performance difference is possible.

[Dzeri96]

While this is being merged, would you consider "setting spark.io.compression.codec to something other than lz4" as good advice for users potentially affected by the vulnerabilities?

As far as I saw, only this config key has LZ4 as default. Also, it seems like spark.eventLog.compression.codec is its own setting, even though the former config key mentions it being used for writing event logs. I guess this is historic and an error in the docs?

[pan3793]

@Dzeri96, setting spark.io.compression.codec=zstd could be an option. Generally, with default spark.io.compression.zstd.level=1, for shuffle cases, compression ratio zstd:lz4 is about 2:1, which means you are expected to save 50% shuffle IO and disk space with zstd, but note that zstd will use more CPUs, so if you run micro benchmark where IO is not a bottleneck, you will see performance drop after switching to zstd, but for large scale production cluster, I suppose zstd is a better option, for both performance and bills.

[Yicong-Huang]

Had a discussion with @dbtsai, I am picking this up in #54496.

[dbtsai]

Closing in favor of #54496