andygrove opened a new pull request, #6155:
URL: https://github.com/apache/datafusion-comet/pull/6155
## Which issue does this PR close?
Closes #6148.
## Rationale for this change
Since #5677 the Iceberg Spark test jobs run with the native Iceberg writer
enabled. A passing job still can't tell a native write from a silent fallback.
`CometIcebergNativeWrite` declines writes without failing them, Gradle does not
copy the `explainFallback` warnings into the job log, and no upstream Iceberg
test checks which writer ran. That makes the Iceberg jobs weak evidence for
#5644.
## What changes are included in this PR?
- A test-only config, `spark.comet.testing.icebergWriteReport.dir`. Its
default comes from the `COMET_ICEBERG_WRITE_REPORT_DIR` environment variable.
When it is set, `CometDriverPlugin` registers `IcebergWriteReportListener`, a
`QueryExecutionListener` that writes one JSON line for each Iceberg write in
the executed plan, labeled with one of three writers:
- `native`: `CometIcebergWriteExec`.
- `jvm`: Comet's split operator kept Iceberg's JVM writer
(`IcebergWriteExec`). The line includes the fallback reasons Comet recorded on
the operator.
- `spark`: Spark's own V2 write, for example `WriteDelta`, which Comet's
split operator never plans.
The listener skips `CommandResultExec`, so a command that is collected
afterwards is counted once.
- `dev/ci/summarize-iceberg-writes.py` builds a Markdown table for the job
summary page. It shows the count and share of each writer, the most common
fallback reasons, and the Spark write operators. It counts only the latest
attempt of each shard and never fails a job.
- `iceberg_spark_test_reusable.yml`: the core shards and the extensions job
set the environment variable, print the summary, and upload the raw records.
The shard coverage job adds a summary for all shards together. The Iceberg
diffs are unchanged because the setting reaches the test JVMs through the
environment.
- `dev/local-ci.sh iceberg` sets the same variable and prints the summary
after each shard and after the extensions target.
- A new section in the contributor guide's Iceberg Spark tests page.
## How are these changes tested?
- A new test in `CometIcebergWriteActionSuite` runs one native write, one
ORC write that falls back to the JVM writer, and one write with the split
operator off. It checks that the report records `native`, `jvm` (with the "only
parquet" reason) and `spark` (`AppendData`), and that collecting a command's
result does not count the write twice.
- A new test in `CometPluginsSuite` checks that the listener is registered
only when the directory is set, and only once.
- Both run with Iceberg 1.11.0 on Spark 4.1, using the 1.11.0 diff and the
environment variable as the workflow sets it:
- `TestPartitionedWrites` and `TestUnpartitionedWrites` recorded 112
writes, all native.
- `TestMergeOnReadDelete` (294 tests) recorded 800 writes: 162 native, 294
JVM and 344 Spark `WriteDelta`. Every JVM write carried a reason: Avro 138, ORC
76, format-version 3 69, non-Arrow input 11.
- `dev/ci/check-ci-config.py` passes.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
