Skip to content

fix: fall back for brotli Iceberg writes when Hadoop's BrotliCodec cannot be loaded - #6996

Open
andygrove wants to merge 2 commits into
apache:mainfrom
andygrove:fix-6960-brotli-codec-fallback
Open

andygrove wants to merge 2 commits into
apache:mainfrom
andygrove:fix-6960-brotli-codec-fallback

Conversation

@andygrove

Copy link
Copy Markdown
Member

Which issue does this PR close?

Closes #6960.

Rationale for this change

parquet-mr compresses and decompresses brotli pages with Hadoop's
org.apache.hadoop.io.compress.BrotliCodec, which it loads by name. Neither Spark nor Iceberg
ships that class, and the Iceberg runtime's bundled copy of Parquet looks up the same one. Without
a brotli codec jar, iceberg-java fails a brotli write. The eligibility gate admitted brotli
anyway, and parquet-rs compresses it without the class, so a native write committed a table that
Spark without Comet cannot read. It fails with
BadConfigurationException: Class org.apache.hadoop.io.compress.BrotliCodec was not found, and so
would Comet whenever its scan falls back to Spark's reader.

#6960 also reported that iceberg-java fails a gzip write with a numeric
write.parquet.compression-level, which the native writer accepts. That case is different: Hadoop
reads zlib.compress.level as the name of a ZlibCompressor.CompressionLevel constant, so 5
throws, but the native files are ordinary gzip pages that every reader can read. Spark 4.1.1's
parquet-mr reads a gzip level 5 file without trouble. This PR documents that difference instead of
making the native path fail as well.

What changes are included in this PR?

  • A new eligibility rule, requireLoadableBrotliCodec, declines a brotli write unless
    org.apache.hadoop.io.compress.BrotliCodec loads through the thread context class loader, which
    Spark adds application jars to. It loads the class without initializing it, so the check runs no
    codec code. It reads the effective codec, so spark.sql.iceberg.compression-codec and the
    compression-codec write option count too. It runs after the compression level rule, so an
    out-of-range brotli level still reports the level.
  • IcebergWriteProtoTranslation.compressesWithBrotli reads the codec name as resolveCompression
    does, but never throws, so an unknown codec such as lzo still fails where it did before.
  • User guide (iceberg-writes.md): the eligibility table's compression row and a new paragraph
    describe the brotli rule, and the accepted-divergences entry on codec settings now covers a
    numeric gzip level. The table cell keeps its width, so the rest of the table is not re-padded.

#6961 adds a list of plans for each fallback to the same page. Whichever of the two merges second
should add this rule to its "Not planned" list, beside the other writes iceberg-java itself
rejects.

How are these changes tested?

A new CometIcebergWriteDetectionSuite case plans two brotli writes without running them, one
set by a table property and one by spark.sql.iceberg.compression-codec, and checks that each
falls back with the new reason. Under a context class loader that resolves the class name, as one
with a brotli codec jar would, both are Compatible. Breaking the rule either way, never declining
or always declining, fails the test.

Locally, CometIcebergWriteDetectionSuite and IcebergWriteProtoTranslationSuite pass on the
default Spark 4.1 profile (104 tests) and on the Spark 3.4 profile with Iceberg 1.5.2 (100 passed,
4 canceled by their version checks).

An end-to-end check, not committed, ran a brotli and a zstd INSERT with
spark.comet.write.iceberg.enabled=true and spark.comet.exec.localTableScan.enabled=true:

  • With the rule disabled, as on main: the brotli plan shows CometIcebergWrite and the insert
    succeeds, and reading the table with spark.comet.enabled=false fails with
    BadConfigurationException: Class org.apache.hadoop.io.compress.BrotliCodec was not found.
  • With this PR: the brotli plan shows IcebergWrite, and the insert fails with that same error,
    as it does without Comet.
  • The zstd table is written natively in both runs, and Spark without Comet reads it.

The Iceberg Spark test suites are unaffected: none of them writes brotli (checked on a recent
Iceberg main), and Iceberg's build adds no brotli codec.

…nnot be loaded

parquet-mr compresses and decompresses brotli pages with Hadoop's
org.apache.hadoop.io.compress.BrotliCodec, which neither Spark nor Iceberg
ships. Without it, iceberg-java fails a brotli write, but the native writer
wrote the files anyway, and Spark without Comet could not read them back.
The eligibility gate now declines brotli unless that class loads.

A gzip write with a numeric level, which iceberg-java also fails, stays
native: its files are ordinary gzip pages that every reader can read. The
user guide lists it as an accepted divergence.

Closes apache#6960.
@andygrove
andygrove force-pushed the fix-6960-brotli-codec-fallback branch from 4c2788b to 0eb9d0c Compare October 18, 2026 17:45

@sunchao sunchao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the full four-file diff from d0f367dc02520fe68f524db22083cc0af6e58bca to f7d2593b8eca6987f36c3a7fa2ece7c1ce63812a. The PR is not a draft. The snapshot and live discussion contain no reviews, conversation comments, or inline comments.

Summary

  • Prior state and problem: Native Iceberg writes admitted Brotli without Hadoop’s BrotliCodec, allowing successful commits that the same installation’s JVM readers could not read.
  • Design approach: Add a driver-side eligibility rule that declines effective Brotli writes when the codec class cannot load, leaving iceberg-java to handle the write.
  • Correctness: Checked Spark’s codec mapping and application class-loader setup, Iceberg’s effective write properties, and Parquet’s codec loading. The new rule uses the resolved configuration, preserves compression-level rejection precedence, and catches missing-class and linkage failures. No introduced P1/P2 correctness issue was identified.
  • Compatibility analysis: Compared relevant sources for Spark 3.4.3, 3.5.9, 4.0.4, 4.1.3, and 4.2.0, with Iceberg 1.5.2, 1.8.1, 1.10.0, and 1.11.0. The Brotli class name and option/session/table precedence support this shared implementation. No version-specific shim or wire-format change is needed.
  • Key design decisions: Checking class availability without initialization avoids constructing a codec during planning. Looking through the context class loader accommodates Spark application jars. Running the rule after level validation preserves the more specific diagnostic.
  • Implementation sketch: CometIcebergNativeWrite adds the rule, IcebergWriteProtoTranslation shares codec-name normalization, the detection suite checks table/session settings and both availability outcomes, and the user guide documents the fallback and existing gzip divergence.
  • Performance: Additional work is confined to eligibility checks: codec-name normalization and, only for Brotli, class lookup. There is no added per-row or per-batch work. No evidence-backed performance regression was found.
  • Design: The change fits the existing ordered eligibility rules and prevents incompatible output before writing starts. Its scope is appropriately small.
  • Abstraction & complexity: Sharing codecName prevents the gate and serializer from interpreting codec names differently. Reusing ClassLoaders.contextOrDefault avoids another class-loader policy. No unnecessary abstraction was identified.
  • Behavioral changes worth calling out: Compared with latest release branch branch-1.1 at 89c729a9697467e8f8d89e3eadf1797a75e2328c, Brotli writes without the codec now fall back instead of writing natively. This is the intended compatibility correction. Numeric gzip-level behavior is unchanged and newly documented.
  • Suggested improvements: No additional change meeting the P1/P2 reporting bar was identified.

Routed skills: review-comet-pr, review-comet-iceberg-write-pr, and review-comet-expression-pr for the serde paths. No scalar expression implementation changed.

Exact-head CI: 25 checks succeeded, 18 were skipped, and none failed or remained running. The CI merge commit 29cae167d6a33f8ff0ccbbf3a067e00914ed3a0e has the same tree as the reviewed head. The Spark 4.1 scans job explicitly ran the new Brotli case and both affected suites, with 738 tests passing overall. Upstream Iceberg/Spark SQL suites and macOS were skipped.

Validation: Bounded local probes passed for the extracted new helper methods, including normalization, unknown/default codecs, missing classes, linkage errors, null context loaders, and loading without initialization. Actual Parquet 1.16.0 and Iceberg 1.11.0 shaded Parquet reproduced the missing-Brotli failures. Hadoop 3.4.2 reproduced numeric gzip-level rejection.

Limits: No local full Comet build, end-to-end native write, or upstream Iceberg/Spark SQL suite was run. The local helper checks were isolated probes, and no real external Brotli codec jar was tested. Older-profile runtime results reported by the author were not independently rerun. Project files remain unchanged.

No introduced P1/P2 issues found within this review. No substantiated unresolved existing P1/P2 blockers were identified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Iceberg area:writer Native Parquet writer bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Native Iceberg writes commit brotli files that no JVM reader can read without Hadoop's BrotliCodec

2 participants