Skip to content

test: [branch-1.1] backport the Iceberg write report and the mid-write retry test (#6155, #6111) - #6307

Draft
andygrove wants to merge 2 commits into
apache:branch-1.1from
andygrove:backport-6155-6111-branch-1.1
Draft

andygrove wants to merge 2 commits into
apache:branch-1.1from
andygrove:backport-6155-6111-branch-1.1

Conversation

@andygrove

Copy link
Copy Markdown
Member

Backport of #6155 and #6111 to branch-1.1, as two commits.

Both were cherry-picked in merge order, without conflicts, from 14f0f59f74dbcf273ff3cbdb1721a5ee535bbe84 and 634e37d08b02da4f9fcbe9dea3606dfd884644c7. #6111 does not apply on its own, because its imports and helper sit on top of #6155's changes to CometIcebergWriteActionSuite. That is why the two share a pull request.

Every changed line is identical to upstream:

Which issue does this PR close?

Closes #6148 on branch-1.1. #6155 already closed it on main. #6111 covers part of #5646.

Rationale for this change

Since #6284, a code pull request against branch-1.1 runs the Iceberg Spark test jobs with the native writer enabled, and so will the dispatch before each release candidate. As #6148 describes, a passing job can't tell a native write from a silent fallback. With #6155, each job's summary page counts the writes that ran natively, through the JVM writer, or through Spark's own V2 write, and lists why Comet declined. So the release branch's Iceberg runs will show how much of 1.1.0's native writer they exercised.

#6111 adds a test for a failed native write attempt. It checks that the task retries and that the failed attempt leaves no committed or orphaned data files.

Neither changes what a user's job does. The listener is registered only when the internal spark.comet.testing.icebergWriteReport.dir is set.

What changes are included in this PR?

The original changes, so see #6155 and #6111 for the details. No adaptations were needed.

#6155 adds:

  • The internal test config spark.comet.testing.icebergWriteReport.dir, which defaults to COMET_ICEBERG_WRITE_REPORT_DIR.
  • IcebergWriteReportListener, and the driver plugin hook that registers it.
  • IcebergTableAsSelectShim, which recognizes Spark 3.4's CTAS and RTAS execs.
  • dev/ci/summarize-iceberg-writes.py, and its test, which runs in Preflight.
  • Steps in the Iceberg workflow and dev/local-ci.sh.
  • A section in the contributor guide's Iceberg Spark tests page.

#6111 adds the test native acceleration: a mid-write failure retries without orphan files to CometIcebergWriteActionSuite, plus a parquet-file walker shared by the suite and the test. The suite now runs on local[5,2] so that a task can retry.

The new config is .internal(), so configs.md needs no row.

How are these changes tested?

Run locally on branch-1.1 with the default profile (Spark 4.1, Scala 2.13, JDK 17):

Upstream, #6155's report tests also passed on Spark 3.4 and 3.5. Against branch-1.1, the changed paths route this pull request to every suite except Spark 3.4's SQL job and the benchmark check. That includes every Spark profile, macOS, PyArrow and all four Iceberg versions. The Iceberg jobs should also publish this branch's first write report.

andygrove and others added 2 commits September 28, 2026 07:30
…bs (apache#6155)

* test: report which writer ran each Iceberg write in the Iceberg CI jobs

The Iceberg Spark test jobs run with the native Iceberg writer enabled, but
a passing job cannot tell a native write from a silent fallback: the
fallback warnings never reach the job log and no upstream test asserts
which writer ran.

Add a test-only config, spark.comet.testing.icebergWriteReport.dir, backed
by the COMET_ICEBERG_WRITE_REPORT_DIR environment variable. When it is set,
the driver plugin registers IcebergWriteReportListener, which appends one
JSON line per Iceberg write: native, JVM under the split operator (with
Comet's fallback reasons), or Spark's own V2 write. The core and extensions
jobs set the variable, so the Iceberg diffs are unchanged, and
dev/ci/summarize-iceberg-writes.py renders a per-job and all-shards table
on the job summary page. dev/local-ci.sh prints the same summary.

Closes apache#6148.

* test: report streaming and Spark 3.4 CTAS/RTAS Iceberg writes, and count only each shard's latest attempt

The write report missed two kinds of Iceberg write that Spark runs itself:

- A streaming micro-batch runs as `WriteToDataSourceV2Exec` over a `MicroBatchWrite`, not as a
  `V2ExistingTableWriteExec`.
- On Spark 3.4, CTAS and RTAS write the table from the create or replace exec through
  `TableWriteExecHelper.writeWithV2`. Spark 3.5+ runs that write as a nested append or overwrite,
  which the listener already sees, so `IcebergTableAsSelectShim` recognizes the execs on 3.4 only.

Both went unreported, so the Spark count and the total left them out and the native share read
high.

The summarizer took each shard's latest attempt from its report files, so a retry that recorded no
writes was hidden behind the earlier attempt's records. It now takes the latest attempt from the
shard artifact directories and names a shard whose latest attempt recorded nothing.
`dev/ci/test-summarize-iceberg-writes.py` covers that and runs in Preflight.

(cherry picked from commit 14f0f59)
* test: cover native Iceberg mid-write retry cleanup

* test: harden native Iceberg retry cleanup coverage

* test: handle Spark 3.4 decimal sum without codegen

* Revert "test: handle Spark 3.4 decimal sum without codegen"

This reverts commit 42ea18b.

(cherry picked from commit 634e37d)
@github-actions github-actions Bot added enhancement New feature or request test Testing related area:Iceberg labels Sep 28, 2026

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

Summary

  • Prior state and problem: Passing Iceberg CI jobs did not reveal whether writes ran natively or fell back. Mid-write retry cleanup also needed explicit coverage.
  • Design approach: Backports an opt-in query listener, JSONL reporting and CI summaries, plus a native-write retry test.
  • Correctness / compatibility analysis: No introduced P1/P2 issues found within this review. Checked Spark sources for all five supported profiles, including command-result deduplication, CTAS/RTAS differences and listener registration. No existing review concerns remain unresolved.
  • Key design decisions: Reporting defaults off, preserving normal query execution without listener overhead. Small version-specific shims isolate Spark 3.4 behavior. The shared registration helper preserves existing metrics-listener behavior without unnecessary abstraction.
  • Implementation sketch: The listener classifies native, JVM and Spark writes. The summarizer selects each shard’s latest attempt. The retry test blocks a later output path and verifies retry, manifest contents and absence of orphan files.
  • Behavioral changes worth calling out: Reporting includes failed queries. The test suite permits one task retry through local[5,2]. Writer eligibility and production write behavior remain unchanged.
  • Suggested improvements: None meeting the P1/P2 reporting threshold.

Reviewed both commits and all 14 files in the full diff from 6811b607b74e4a52fe539eec3b4cb8122ca13145 to 6d799f6c6fd7f593e9bbcebe5d90e03cee4174ab. The PR is open and not a draft. The snapshot and live discussion contain no reviews or comments. Routed skills: review-comet-pr and review-comet-iceberg-write-pr.

Exact-head CI at 2026-09-28 14:23 UTC: 75 successful, 25 running, 3 skipped and 2 failed checks. The failures occurred before tests: Spark 4.2 expressions encountered ECONNRESET during toolchain setup, and Iceberg 1.9 extensions encountered Maven Central HTTP 429 resolving the parent POM. CI is therefore not fully green.

Validation: All six Python summary tests, CI configuration checks and suite-registration checks passed locally. Exact-head artifacts confirm the four new Iceberg tests passed on Spark 3.4, 3.5, 4.0 and 4.1, and all six plugin tests passed on Spark 4.1. Real Iceberg 1.8 and 1.11 reports also summarized successfully. Native/JVM suites were not run locally. Spark 4.2 skipped the new Iceberg tests because Iceberg was unavailable on its test classpath, and the remaining CI jobs were still running.

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

Labels

area:Iceberg enhancement New feature or request test Testing related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants