Skip to content

fix: fall back to Spark for native Iceberg writes to gs:// through HadoopFileIO - #5935

Open
zhangfengcdt wants to merge 4 commits into
apache:mainfrom
zhangfengcdt:fix/iceberg-native-write-gcs-hadoop-fileio
Open

zhangfengcdt wants to merge 4 commits into
apache:mainfrom
zhangfengcdt:fix/iceberg-native-write-gcs-hadoop-fileio

Conversation

@zhangfengcdt

@zhangfengcdt zhangfengcdt commented Sep 14, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

Closes #5637.

Rationale for this change

The native Iceberg write path accepts gs:// data locations, but the only bridge from the Hadoop Configuration into the native FileIO translates fs.s3a.* keys. A HadoopFileIO takes its GCS credentials, endpoint and project from fs.gs.*, so none of that reached the native writer and it could resolve a different storage identity or endpoint than the JVM writer would. Following the issue's recommendation, this change fails closed for that combination rather than attempting an fs.gs.* to gcs.* bridge, which is not a simple key rename.

What changes are included in this PR?

  1. CometIcebergNativeWrite: a new trigger rule that declines a gs:// data location unless the FileIO opening it is a GCSFileIO. The decision is a small package-visible function so it can be unit-tested directly.
  2. IcebergReflection: a helper that returns the effective FileIO class for a location. For a ResolvingFileIO it invokes io(location) and inspects the delegate Iceberg actually instantiates, including the fallback to HadoopFileIO when the scheme's FileIO cannot be loaded or initialized, so a table that resolves to GCSFileIO stays eligible. Reflection failures fail closed.
  3. iceberg-writes.md: documents the new condition.

Spark fallback message

When the gate declines, the write runs on Spark's Iceberg writer and Comet's extended EXPLAIN reports one of these reasons:

  • The effective FileIO is not a GCSFileIO (typically a HadoopFileIO):

    gs:// data location gs://bucket/warehouse/db/t/data is written through org.apache.iceberg.hadoop.HadoopFileIO, whose fs.gs.* Hadoop configuration is not forwarded to the native writer

  • The delegate could not be resolved (for example, a ResolvingFileIO whose GCSFileIO fails to construct):

    could not resolve the FileIO for the gs:// data location gs://bucket/warehouse/db/t/data

How are these changes tested?

New and updated tests in CometIcebergWriteDetectionSuite:

  • The test that expected a gs:// table under the Hadoop catalog to be Compatible now covers only memory; that expectation was the bug.
  • A fallback test for HadoopFileIO + gs://. The write is planned but not executed, so the test does not touch the network.
  • A unit test of the gate covering all three branches: GCSFileIO accepted, HadoopFileIO rejected, unresolved rejected.
  • Two end-to-end tests of ResolvingFileIO delegate resolution and write planning. One uses an unparseable gcs.channel.read.chunk-size-bytes, which fails GCSFileIO.initialize on Iceberg versions before 1.10 and is ignored at initialization on 1.10+; the expectation is version-aware. In CI the pre-1.10 profiles lack the Google authentication classes, so GCSFileIO never constructs there and the test passes through the class-load fallback. Direct coverage of the initialization-failure path therefore remains unverified in CI; it holds by source inspection of 1.5.2 and 1.8.1.

@github-actions github-actions Bot added bug Something isn't working area:writer Native Parquet writer area:Iceberg labels Sep 14, 2026
@zhangfengcdt
zhangfengcdt force-pushed the fix/iceberg-native-write-gcs-hadoop-fileio branch from 455bffb to f014058 Compare September 14, 2026 21:38

@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.

Correctness

This addresses #5637: the native Iceberg writer accepts GCS data locations but its Hadoop configuration bridge translates only S3 settings. The new trigger correctly rejects a direct HadoopFileIO at a gs:// data location before native conversion, leaving the JVM write operator in place. It uses the table's data-location provider, so a separate write.data.path is covered; other schemes retain their existing behavior.

One P2 remains: ResolvingFileIO.ioClass(location) reports the scheme-mapped class rather than the instantiated delegate. Initialization can select HadoopFileIO while this helper reports GCSFileIO, admitting the same unsupported Hadoop GCS configuration. The inline comment gives a concrete Iceberg 1.8.1 case. This distinction also exists in the inspected 1.5.2, 1.10.0 and 1.11.0 resolver implementations.

Validation

Reviewed head 44b6a9bdf77b3a19a532b7d34178247236b7aa32 against base 683c7219e39928d267dc45c664d8656a4157da78, including all four changed files and current public discussion. CI checked out merge 298abf56e26de79dd99e65973d4d374eb6a22af6, with identical changed files and relevant dependency/dispatch sources. The new GCS tests passed on Spark 3.4, 3.5, 4.0 and 4.1; Spark 4.2 canceled them because Iceberg is unavailable on that profile. The checks report 53 successes and 10 skips.

A local component probe against the actual Iceberg 1.8.1 runtime confirmed that the invalid chunk-size property throws NumberFormatException, which is an IllegalArgumentException. The resulting delegate selection and native eligibility were traced through source; no local end-to-end GCS write was executed. The planning-only test helper agrees with CommandExecutionMode.SKIP in the maintained Spark 3.5 and 4.0 branches. Maintained 3.4, 4.1 and 4.2 source branches were unavailable, so CI evidence is separate from canonical source coverage.

Performance

The new work occurs during write planning: another data-location lookup, class-hierarchy checks, and reflective class resolution for GCS locations. The existing method cache avoids repeated method discovery; there is no new per-row or per-file data-path work. No material performance claim or measured regression is established here, so a throughput microbenchmark is not needed for this guard.

Design

Adding the check to the existing eligibility rules is a small, appropriate way to preserve JVM storage behavior without implementing an incomplete credential translation. Earlier storage-scheme and FileIO checks still fail closed. The resolver helper needs to inspect the effective delegate before this design fulfills its stated guarantee. The regression should compare the helper with that actual delegate and exercise its initialization fallback.

Abstraction & complexity

The shared scheme parser and package-visible decision function are modest additions that fit the existing structure. The helper's contract currently promises more than the Iceberg API provides; correcting that contract and testing it independently is the substantive simplification needed. No additional abstraction or broader property translation is required.

Comment thread spark/src/main/scala/org/apache/comet/iceberg/IcebergReflection.scala Outdated
@zhangfengcdt zhangfengcdt changed the title fix: Fall back to Spark for native Iceberg writes to gs:// through HadoopFileIO fix: fall back to Spark for native Iceberg writes to gs:// through HadoopFileIO Sep 15, 2026
@zhangfengcdt
zhangfengcdt force-pushed the fix/iceberg-native-write-gcs-hadoop-fileio branch 2 times, most recently from 142db33 to f12167c Compare September 15, 2026 16:11

@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.

The previous delegate-selection finding is fixed. The gate now inspects the delegate instantiated by ResolvingFileIO.io(location), and reflection failures still fail closed.

One new P2 remains in the regression test. The invalid chunk-size property does not make GCSFileIO.initialize fail on Iceberg 1.10/1.11. I independently reproduced this with both real runtimes and the exact current Comet helper. It returns GCSFileIO, making the new HadoopFileIO assertion false. Details are inline.

The focused GCS cases passed in the current Spark 3.4/3.5/4.0/4.1 CI profiles. Spark 4.2 canceled them because Iceberg was unavailable. Current CI has finished with 49 successful and 12 skipped checks, but three failed: the Maven-bootstrap HTTP 403, the resulting missing shard evidence, and aggregate Required Checks. No full local Spark suite or native GCS write was run.

Comment thread spark/src/test/scala/org/apache/comet/CometIcebergWriteDetectionSuite.scala Outdated
…doopFileIO

HadoopFileIO takes its GCS configuration from fs.gs.*, which is not
forwarded to the native writer, so a native write could run with a
different storage identity or endpoint than the JVM writer. Decline a
gs:// data location unless the FileIO Iceberg resolves for it is a
GCSFileIO.

Closes apache#5637
ResolvingFileIO.ioClass throws a NoClassDefFoundError when the GCS
client libraries are absent, which aborted the suite on the Spark 3.4
and 3.5 CI jobs. Resolve the delegate the way the gate does and expect a
decline when it cannot be resolved.
ResolvingFileIO.ioClass only maps the scheme to a class; io(location)
instantiates it and falls back to HadoopFileIO when that fails, so the
gate must inspect the instantiated delegate.
Iceberg 1.10+ defers gcs.* parsing to client construction, so an
unparseable chunk size no longer fails GCSFileIO.initialize there.
@zhangfengcdt
zhangfengcdt force-pushed the fix/iceberg-native-write-gcs-hadoop-fileio branch from 3c6e055 to 251fb7d Compare September 15, 2026 18:38

@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.

The fixture P2 is fixed in 251fb7d4a69f0602b86f1ac816868f202233a092 (reviewed against 4479e722efebd33620f29a246f7863bd04f5df1a). On Iceberg 1.10+, it now compares the delegate with an unaffected control and expects compatibility when Iceberg retains GCSFileIO. The previous production delegate-selection fix is unchanged. No new or remaining P1/P2 findings.

I reran the exact helper, fixture and guard component checks with real Iceberg 1.5.2, 1.8.1, 1.10.0 and 1.11.0 runtimes. All four revised assertions pass. The original 1.10/1.11 reproduction still makes the old assertion false and now passes the revised one. Older runtimes resolve to HadoopFileIO on this classpath. Missing Google authentication classes prevent crediting direct eager-GCS-initialization coverage.

The focused GCS cases passed in current Spark 4.0 and 4.1 CI. Spark 4.2 skipped them because Iceberg is unavailable. Executed merge ab0aa7bd has parents [base, head] and an identical tree to this head. Native artifact upload/download digests match. CI still has 14 running checks, alongside 36 successful and 11 skipped, at September 15, 19:37 UTC. No full local Spark suite or native GCS write was run.

@comphead comphead left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @zhangfengcdt looks good, it would be beneficial to update PR description to include Spark fallback message when dealing with GS

@zhangfengcdt

Copy link
Copy Markdown
Member Author

Thanks @zhangfengcdt looks good, it would be beneficial to update PR description to include Spark fallback message when dealing with GS

Sure, thanks for the review and I have updated the pr description to include the fallback message.

@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.

The added fallback examples match the current gate at 251fb7d4. My existing approval stands. The source is unchanged, and the resolved fixture P2 remains fixed.

Please align two description details with the reviewed behavior: the helper invokes io(location) to inspect the instantiated delegate, and the resolver tests exercise delegate resolution and write planning. On pre-1.10 runtimes, their passing result alone does not prove that GCSFileIO.initialize ran. The prior 1.5.2/1.8.1 component controls lacked Google authentication classes, so that direct initialization coverage remains unverified.

Required Checks is now green: 53 successful checks and 12 skips. The completed Spark 3.4/3.5 scan jobs also passed the focused GCS cases on merge ab0aa7bd, whose full tree matches this head. No unchanged broad suites were rerun locally, and no native GCS write is claimed.

@zhangfengcdt

Copy link
Copy Markdown
Member Author

The added fallback examples match the current gate at 251fb7d4. My existing approval stands. The source is unchanged, and the resolved fixture P2 remains fixed.

Please align two description details with the reviewed behavior: the helper invokes io(location) to inspect the instantiated delegate, and the resolver tests exercise delegate resolution and write planning. On pre-1.10 runtimes, their passing result alone does not prove that GCSFileIO.initialize ran. The prior 1.5.2/1.8.1 component controls lacked Google authentication classes, so that direct initialization coverage remains unverified.

Required Checks is now green: 53 successful checks and 12 skips. The completed Spark 3.4/3.5 scan jobs also passed the focused GCS cases on merge ab0aa7bd, whose full tree matches this head. No unchanged broad suites were rerun locally, and no native GCS write is claimed.

I have updated the following to align with the reviewed behavior.

  1. IcebergReflection: a helper that returns the effective FileIO class for a location. For a ResolvingFileIO it invokes io(location) and inspects the delegate Iceberg actually instantiates, including the fallback to HadoopFileIO when the scheme's FileIO cannot be loaded or initialized, so a table that resolves to GCSFileIO stays eligible. Reflection failures fail closed.

AND

  • Two end-to-end tests of ResolvingFileIO delegate resolution and write planning. One uses an unparseable gcs.channel.read.chunk-size-bytes, which fails GCSFileIO.initialize on Iceberg versions before 1.10 and is ignored at initialization on 1.10+; the expectation is version-aware. In CI the pre-1.10 profiles lack the Google authentication classes, so GCSFileIO never constructs there and the test passes through the class-load fallback. Direct coverage of the initialization-failure path therefore remains unverified in CI; it holds by source inspection of 1.5.2 and 1.8.1.

@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.

The io(location) correction matches the implementation, and the description now acknowledges that direct pre-1.10 initialization-failure coverage remains unverified. My existing approval stands on unchanged head 251fb7d4. No new or remaining P1/P2 findings.

Missing Google authentication classes were directly observed in the component controls. I have not independently verified that cause in the hosted CI classpath, and passing resolver tests alone do not establish which failure path ran. Required Checks remains green. No unchanged broad suites were rerun.

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 drop Hadoop GCS configuration for HadoopFileIO + gs://

3 participants