ci: shard Iceberg Spark tests across four runners

[sunchao]

Why are the changes needed?

The main Iceberg Spark test task is one of Comet CI's longest paths. In a recent successful job, test execution consumed about 67 minutes, including 18.7 minutes in TestStructuredStreamingRead3 and 10.7 minutes in TestRewriteDataFilesAction. The job finished after 78 minutes including setup and compilation.

Which issue does this PR close? None. This is a CI scheduling improvement that retains the existing supported versions and test coverage.

What changes were proposed in this PR?

Split the main Iceberg Spark test task across four isolated workers. Keep the long structured-streaming family together and assign all remaining classes automatically by a stable hash. New classes therefore receive a worker without maintaining a test allowlist. Nested classes stay with their enclosing class, and all classes remain available on the test classpath for inherited fixtures and framework discovery.

A workflow-side Gradle init script adds an exclusion predicate to the existing candidate selection. It does not replace Gradle's existing includes/excludes or JUnit's tag and parameterized-test handling. Each worker uploads both its unsharded and selected candidate inventories alongside the JUnit reports, with distinct names for reruns. A dependent job checks the complete index set, identical unsharded baselines, and selected sets whose disjoint union equals the baseline. Matrix indices and the Gradle partition count now come from one definition, independent of strategy.job-total.

Run a lightweight coverage-equivalence fixture once per Iceberg version in the existing short runtime job. Native compilation, extensions, runtime-test commands, and existing Comet/Gradle build flags are otherwise unchanged. This PR intentionally leaves repeated JVM compilation for separate work.

How was this PR tested?

• The original fixture passed with Gradle 8.14.4/JDK 17 and Gradle 8.12.1/JDK 11. For this review follow-up, reran the updated fixture with Gradle 8.14.4/JDK 17: the four workers match the unsharded set of 10 candidate classes and 12 executed cases exactly once, and each captured unsharded inventory matches the independently executed baseline.
• Added 15 fast regression tests for real-inventory validation, including missing/extra/duplicate classes, missing/extra indices, mismatched counts/tasks/baselines, malformed metadata, and failed-job reruns. All pass and run in preflight.
• Covered nested, parameterized, inherited, dynamic, and newly named tests; existing include/exclude patterns, exclusion predicates, and JUnit tags; single-worker equivalence; and isolation from another Test task.
• Verified that a failing test still fails Gradle, and invalid worker indices or task names fail explicitly.
• Checked the streaming family in all four configured Iceberg releases, the change filters, and the unchanged native/extension jobs and existing test flags. Actionlint, Python syntax, Apache RAT, and git diff --check pass on the review follow-up. Documentation formatting was checked on the original revision but was not rerun locally for this follow-up.

A complete successful four-shard Iceberg run and its total elapsed time remain unmeasured. The previous unsharded job spent about 67 minutes testing and 11 minutes on setup/compilation. Assuming the same test work and 11 minutes of repeated setup per worker, four shards would use about 111 runner-minutes per version (67 + 4 × 11), compared with 78 previously: 33 extra minutes, or roughly 42%. This is an estimate, not a measured after result, and excludes the small inventory-check job. The existing successful shards 2/3/4 took 26m54s, 28m08s, and 25m05s, but shard 1 did not complete, so these cannot establish a full-run total or whole-CI critical-path improvement. Compilation sharing or fewer shards can be evaluated against a complete run; neither benefit is assumed here. The main test workers have new shard-suffixed check names, so any external requirements that refer to the old individual check should be checked.

[andygrove]

Note on this review: this was generated by an LLM (Claude Code) at my request while I worked through a review backlog. I have not verified the individual findings myself. Please treat everything below as suggestions to evaluate rather than as authoritative review feedback, and push back on anything that is wrong or already handled.

Cutting the Iceberg path from 78 minutes is worth doing, and I like that the split is a Gradle exclusion predicate layered on top of the existing candidate set rather than an allowlist someone has to maintain. Keeping @Nested classes with their enclosing class and leaving everything on the classpath are the right details to get right.

Three things I would want addressed.

strategy.job-total will silently drop tests if the matrix ever gains a dimension

The workflow passes -PcometShardCount=${{ strategy.job-total }}, which today equals 4 because shard is the only matrix key. If anyone later adds a second dimension, say jdk: [11, 17], job-total becomes 8 while matrix.shard still ranges 1 through 4. Classes hashing to shards 5 through 8 would then be excluded from every worker and simply never run, with a green build. Could this pass 4 literally, or derive the count from the length of the shard list? A silent coverage hole in the Iceberg suite is exactly the kind of thing nobody notices for months.

Nothing verifies that the four real shards cover the real test set

check-iceberg-shards.py is a nice piece of work, but it validates the init script against a synthetic 10-class fixture in a different job. The actual Iceberg run uploads four comet-shards/*.json manifests as artifacts and then nothing reads them. The property we care about is "the union of the four real shards equals what the unsharded run would have selected, with no class appearing twice," and that is currently unchecked.

Would you add a small follow-up job that depends on the four shards, downloads the manifests, and asserts the union has no duplicates and matches the expected class count? That is cheap and it turns the artifact upload from something you look at during a postmortem into something that fails the build. Without it, the failure mode of a bad hash change is silently running fewer tests.

The runner-minute cost is not stated

The description quotes 67 minutes of test execution inside a 78-minute job, so roughly 11 minutes of setup, container pull, and compilation. That 11 minutes is now paid four times per Iceberg version, and the description explicitly defers the repeated-compilation work. Wall clock clearly improves. Total consumption on shared ASF runners goes up, and by how much is the number that matters for whether this is a net win. Could you put the before and after total runner minutes in the description? If the increase is significant it might be worth landing the compilation sharing first, or starting with two shards instead of four.

One smaller thing

The comment in CometIcebergTestShard.owner records concrete minute figures from a 1.11 run. Those will go stale and there is nothing that notices when the buckets drift apart. Since each shard's duration is visible in the Actions UI anyway, would it be better to say "sized against a 1.11 run in , recheck if a shard becomes the critical path" rather than baking in numbers that will silently become wrong?

[sunchao]

Updated in 913354ca0.

Addressed the count and real-inventory validation gaps in this update.

The workflow matrix and Gradle partition count now come from the same SHARD_COUNT definition. Neither uses strategy.job-total, so adding another matrix dimension cannot silently change ownership to nonexistent shard indices.

Each real Iceberg worker now captures the unsharded Gradle candidate set with only our predicate temporarily disabled, restores the predicate, and records the selected set before running tests. A dependent job downloads the matching configuration's artifacts, requires every shard index, checks that the independently captured baselines agree, and verifies that the selected sets are disjoint and cover the baseline exactly once. It selects each shard's latest available attempt so failed-job reruns can retain earlier successful workers. There is no fixed release-specific class count. JUnit execution is still checked separately by the synthetic fixture, since candidate classes are not executed test cases.

Validation: all 15 inventory-validator regressions pass; the updated Gradle 8.14.4/JDK 17 fixture verifies 10 candidate classes and 12 executed cases exactly once, including nested/parameterized/inherited/dynamic cases, upstream exclusions, another task's isolation, and failure propagation. Actionlint 1.7.12, Python syntax, Apache RAT, and git diff --check pass. The real Iceberg artifact check is wired into CI but awaits a complete hosted run; I have not represented it as a locally completed full Iceberg suite.

I also made the cost estimate explicit in the description: using the previous 67 test minutes plus 11 setup/compile minutes, four workers are approximately 111 runner-minutes versus 78 per version, or +33 minutes / +42%, excluding the small inventory-check job. That assumes unchanged total test work and repeated setup, and is not a measured after result. Shards 2/3/4 completed in 26m54s, 28m08s, and 25m05s, but shard 1 did not finish, so those partial results cannot establish the full total or whole-CI critical-path improvement. Compilation sharing or fewer shards should be judged against a complete run.

The balancing comment remains explicitly historical: it describes the Iceberg 1.11 run used to size the shards, not a current timing guarantee.