Skip to content

feat: [branch-1.2] support the S3A magic committer in native Parquet writes (#6940) - #7281

Merged
andygrove merged 1 commit into
apache:branch-1.2from
andygrove:backport-6940-branch-1.2
Oct 28, 2026
Merged

andygrove merged 1 commit into
apache:branch-1.2from
andygrove:backport-6940-branch-1.2

Conversation

@andygrove

Copy link
Copy Markdown
Member

Which issue does this PR close?

Backport of #6940, which closes #6929.

Rationale for this change

Native Parquet writes cannot run where S3A's magic committer commits the write, and Spark 4.1 makes it the default whenever spark-hadoop-cloud is on the classpath (SPARK-47618). #6940 lets the native writer produce such a write by having the JVM create each file through S3A and the native writer supply only its bytes.

This is a feature rather than a bug fix. branch-1.2 has not shipped its .0 release, so it is still taking what main gains after the cut (see Which Branches Take Backports), and the release manager decides whether it stays. It is not going to branch-1.1 or branch-1.0, which take regression fixes only. There is no newer release branch.

What changes are included in this PR?

The cherry-pick of f1121ca (git cherry-pick -x), with one adaptation:

  • native/core/src/execution/jni_api.rs: Native.setOutputStream calls get_execution_context(exec_context) without the ?. On main that function returns a CometResult (it rejects a null handle with an Internal error), but on branch-1.2 it returns the &mut ExecutionContext directly. setOutputStream already rejects a zero handle with a NullPointer error before the call, so no check is lost. The change that made the function fallible is not backported.

Everything else is identical to the source commit.

How are these changes tested?

Built on branch-1.2 with the cherry-pick, because a clean pick can still depend on code that landed after the cut:

  • cargo check --all-targets --workspace, cargo clippy --all-targets --workspace -- -D warnings and cargo fmt --all --check in native/.
  • ./mvnw test-compile on the default profile (Spark 4.1) and on -Pspark-3.5.
  • The Rust tests for the stream writer, the sink and the planner: 142 passed in datafusion-comet.

CometParquetWriterSuite, CometParquetWriterRetrySuite and the other suites the source PR changed run in CI, which runs every tier on a pull request against a release branch. ParquetWriteWithMagicCommitterSuite needs MinIO and is manual, as on main; I have not run it on this branch.

…e#6940)

* feat: support the S3A magic committer in native Parquet writes

On Spark 4.0+, a task file that is neither local nor on HDFS is created by
the JVM through Hadoop's FileSystem API, as Spark's Parquet writer creates it,
and the native writer streams the file's bytes into that stream over JNI. S3A
therefore creates the magic committer's files itself and records their pending
uploads. A failed write aborts the stream.

Planning admits an S3 destination when the S3A magic committer commits it,
through PathOutputCommitProtocol or BindingParquetOutputCommitter. Other S3
writes still fall back to Spark.

* test: make the magic committer's failed-write check isolate the writer's abort

S3A's job abort also aborts pending uploads under the destination, through
core-default.xml's fs.s3a.committer.abort.pending.uploads=true, which hid a
writer that never aborted its file. Turn that off in the test, use one task so
job abort cannot race other tasks, and run each committer binding alone. Add a
unit test that the output stream writer sends nothing after a failed write.

* feat: support the S3A magic committer in Spark 3.4 and 3.5 native writes

The Spark 3.x writer runs the session's commit protocol with the write's
Hadoop configuration since apache#4746, so it reaches S3A's committers as Spark's
writer does. Both writers now create task files through one helper,
NativeWriteUtils.writeFile, which creates any file that is neither local nor on
HDFS through Hadoop and closes or aborts it, and both serdes share one file
system gate. The magic committer tests run on every supported Spark version;
in-memory commit tracking is skipped on Hadoop 3.3.4, which lacks it.

* refactor: simplify the native write path through the JVM

The native writer now fails a task with the output stream's own error, which
keeps the Java exception for the JNI boundary to rethrow as it was, so
HadoopFileOutput no longer records failures and the JVM no longer swaps
exceptions. Both writers write a task file through one helper,
CometWriteFilesExec.writeFile, which sets the file and stream flag, runs the
plan, and closes or aborts the file.

Also: the pusher and output stream registrations share their rules, the
writer takes an optional stream, local writes report their counted bytes,
planning skips object store options and Hadoop configurations it does not
need, the committer factory keys come from Hadoop's constants, and the tests
share their setup and read S3A's success manifest through SuccessData.

* address review: drop the unreachable bucket committer fallback and say once why S3A creates the file

* address review: enrich write errors as Spark does, test the Hadoop route's failure paths

Fail a native write's task with TASK_WRITE_FAILED as Spark's writer does,
keeping fetch failures and fast-failing on an existing file. Admit
subclasses of PathOutputCommitProtocol at planning. Build each task's
plan once and create its iterator through CometExec, register the output
stream through register_before_execution directly, and move the
supported destinations to the Parquet page's Writing section.

Test a file of several MiB, a failure partway through it, a failed close,
a stream that cannot abort, a retried task and a path with special
characters, and against MinIO, a failure after S3A uploaded a part and
failed task and job commits.

* test: compare what a failed job commit leaves pending with Spark's writer

S3A's magic committer leaves the uploads of committed tasks pending when
job commit fails, for Spark's own writer as well, so check that the
native write leaves the same and nothing visible. Scope each pending
upload check to its table, and abort what a test leaves behind.

* address review: test the fast-fail and commit paths, nested and temporal columns, and the sink's guards

Cover the branches of the Hadoop route that had no test: a task file that already exists, a file that exists at task commit (with and without fastFailOnFileFormatOutput), nested and temporal columns compared with Spark's writer through chunked writes and the abort path, an INSERT into a table of the V2 file source, and the output stream sink's oversize and null checks.

* fix: handle the fallible execution context lookup in setOutputStream

* test: write complex and temporal types through the Hadoop route

* test: leave the non-ASCII character out of the output path when the JVM cannot encode it

(cherry picked from commit f1121ca)
@github-actions github-actions Bot added enhancement New feature or request area:writer Native Parquet writer area:scan Parquet scan / data reading area:ffi Arrow FFI / JNI boundary labels Oct 28, 2026

@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 @andygrove lets check it out

@andygrove
andygrove merged commit cc6535c into apache:branch-1.2 Oct 28, 2026
164 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:ffi Arrow FFI / JNI boundary area:scan Parquet scan / data reading area:writer Native Parquet writer enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants