Repository navigation

[MINOR][VL] Configure Gluten for all Delta Spark test contexts - #13131

Open

felipepessoto wants to merge 9 commits into

apache:mainfrom

felipepessoto:felipepessoto-delta-task-resource-failures

Open

[MINOR][VL] Configure Gluten for all Delta Spark test contexts#13131
felipepessoto wants to merge 9 commits into
apache:mainfrom
felipepessoto:felipepessoto-delta-task-resource-failures

felipepessoto commented Sep 25, 2026 •

edited

Loading

Contributor

What changes are proposed in this pull request?

Configure Gluten consistently across Delta test Spark contexts. Suites outside DeltaSQLCommandTest could reuse Gluten's JVM-wide cache serializer without the executor plugin, causing TaskResourceRegistry is not initialized.

Move the intended Gluten defaults into java-test-args.sh and remove the DeltaSQLCommandTest replacement and associated license-check exemption. Delta extensions and catalog settings remain suite-specific.
Use Spark's writer only to prepare the Hadoop mock-S3 fixture; keep Gluten enabled for its conversion and assertions.
Remove 19 now-passing baseline entries relative to main, including conversion tests fixed by consistent plugin initialization. Tests and failure-gate enforcement remain enabled.

The two catalog writes previously tracked under #13138 now pass because #13271 added fallback for writes requiring partition columns inside Parquet files—the Delta 4.4.1 default. This is not a DOUBLE-specific fallback or native partition-type support. The case with delta.writePartitionColumnsToParquet=false, without features forcing those columns into the files, still needs verification. #13138 remains open.

How was this patch tested?

Rebased Delta CI completed the native/bundle builds and all eight test shards. Only the gate for four now-passing DeltaUpdateCatalogSuite entries failed; there were no new regressions.

After pruning those entries, the recorded results pass the shard gate and full aggregation: 22,571 tests ran, 510 expected failures, and zero regressions, now-passing entries, or stale entries. Latest-head CI is running for 95e93654e.

Also verified Delta 4.4.1 setup and patch application, shared defaults, shell/workflow syntax, and license headers. Known failures still execute; a green gate does not mean those bugs are fixed.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: GitHub Copilot 1.0.99-2 (GPT-6 Astra)

github-actions Bot added CORE INFRA DOCS labels

github-actions Bot commented Sep 25, 2026

Run Gluten Clickhouse CI on x86

felipepessoto closed this

felipepessoto reopened this

github-actions Bot commented Sep 25, 2026

Run Gluten Clickhouse CI on x86

1 similar comment

github-actions Bot commented Sep 25, 2026

Run Gluten Clickhouse CI on x86

github-actions Bot added BUILD VELOX labels

github-actions Bot commented Sep 25, 2026

Run Gluten Clickhouse CI on x86

github-actions Bot removed BUILD VELOX labels

github-actions Bot commented Sep 25, 2026

Run Gluten Clickhouse CI on x86

felipepessoto mentioned this pull request

[VL] Native Delta writes fail on DOUBLE partition columns instead of falling back #13138

Open

github-actions Bot commented Sep 25, 2026

Run Gluten Clickhouse CI on x86

1 similar comment

github-actions Bot commented Sep 25, 2026

Run Gluten Clickhouse CI on x86

github-actions Bot removed the CORE label

felipepessoto marked this pull request as ready for review

September 25, 2026 22:25

Copilot AI lite review requested due to automatic review settings

September 25, 2026 22:25

felipepessoto commented Sep 25, 2026

Contributor Author

@philo-he @zhouyuan @zhztheplayer could you please review this PR? Thank you!

Copilot started reviewing on behalf of felipepessoto

September 25, 2026 22:26

View session

Copilot AI reviewed

View reviewed changes

Copilot AI left a comment

Contributor

Copilot review overview

🔵 Needs a closer look

Two DOUBLE partition-write failures remain deferred, and the PR is pending latest-head Delta CI.

Review effort: Lite
Findings: None

What changed in this PR

Configures Gluten consistently across Delta Spark test contexts and removes the copied DeltaSQLCommandTest workaround.

Changes:

Centralizes shared Gluten and Spark defaults.
Simplifies Delta setup while preserving upstream fixtures.
Updates mock-S3 handling and known-failure baselines.

File	Description
`.github/workflows/util/delta-spark-ut/setup-delta.sh`	Simplifies Delta setup and removes fixture replacement.
`.github/workflows/util/delta-spark-ut/run-delta-tests.sh`	Updates shared JVM configuration documentation.
`.github/workflows/util/delta-spark-ut/README.md`	Documents configuration and usage.
`.github/workflows/util/delta-spark-ut/known-failures.txt`	Refreshes expected failures.
`.github/workflows/util/delta-spark-ut/java-test-args.sh`	Adds centralized Gluten and Spark defaults.
`.github/workflows/util/delta-spark-ut/apply-delta-test-patches.sh`	Scopes mock-S3 preparation to Spark’s writer.
`.github/workflows/delta_spark_ut.yml`	Updates workflow setup and documentation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

zhztheplayer reviewed

View reviewed changes

zhztheplayer left a comment

Member

Thanks.

.github/workflows/util/delta-spark-ut/apply-delta-test-patches.sh

Comment on lines +89 to +111

+              if ! sed 's/^__BLANK_CONTEXT__$/ /' <<'PATCH' | git -C "$DELTA_DIR" apply -
+              diff --git a/spark/src/test/scala/org/apache/spark/sql/delta/ConvertToDeltaSuiteBase.scala b/spark/src/test/scala/org/apache/spark/sql/delta/ConvertToDeltaSuiteBase.scala
+              --- a/spark/src/test/scala/org/apache/spark/sql/delta/ConvertToDeltaSuiteBase.scala
+              +++ b/spark/src/test/scala/org/apache/spark/sql/delta/ConvertToDeltaSuiteBase.scala
+              @@ -1322,7 +1322,9 @@
+                   withTempDir { dir =>
+                     withTable("externalTable") {
+                       withSQLConf(("fs.s3.impl", classOf[S3LikeLocalFileSystem].getCanonicalName)) {
+              -          sql(s"CREATE TABLE externalTable USING parquet LOCATION 's3://$dir' AS SELECT 1")
+              +          withSQLConf("spark.gluten.sql.native.writer.enabled" -> "false") {
+              +            sql(s"CREATE TABLE externalTable USING parquet LOCATION 's3://$dir' AS SELECT 1")
+              +          }
+              __BLANK_CONTEXT__
+                         // Ideally we would test a successful conversion with a remote filesystem, but there's
+                         // no good way to set one up in unit tests. So instead we delete the data, and let the
+              PATCH
+              then
+                echo "ERROR: mock S3 fixture patch did not apply." >&2
+                echo "The patch expects the Delta v4.2.0 path-scheme fixture;" \
+                  "ref '${DELTA_REF}' must remain source-compatible." >&2
+                exit 1
+              fi
+              echo "::endgroup::"

zhztheplayer Sep 28, 2026

Member

Same comment as #13129 (comment).

If possible, can we open a PR to add the patch list, which can be reviewed first?

felipepessoto Sep 28, 2026

Contributor Author

Thanks @zhztheplayer. I’ll be offline for 3 weeks. I plan to address the comments once I’m back.

felipepessoto and others added 6 commits

October 19, 2026 13:44


          [MINOR][VL] Configure Gluten for all Delta Spark test contexts

1c6e6f7

Set shared JVM defaults for the Gluten plugin, columnar shuffle manager, and off-heap memory so custom Delta test contexts initialize task resources. Preserve suite-specific Delta extension and catalog settings, add JVM bootstrap regression coverage, and remove the 35 confirmed-fixed baseline failures.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>


          [MINOR][VL] Centralize Delta CI defaults without replacing its test f…

e426732

…ixture

Move the remaining native, ANSI, parallelism, and safety defaults into shared JVM options. Leave upstream DeltaSQLCommandTest and license-header checking intact, update setup callers and coverage, and scope the Hadoop-only mock-S3 fixture write to Spark.

Keep the upstream Velox PR override unset for the requested CI reproduction of the existing AdaptivePrefetch zero-duration division. The native blocker is documented in the draft PR rather than hidden by changing parallelism or baselining the crash.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>


          [VL] Test the upstream adaptive prefetch timing fix

59a09e8

Apply facebookincubator/velox#19208 through UPSTREAM_VELOX_PR_ID after preserving the completed unpatched Delta CI run 36165436135.

The unpatched CI passed all 12 bitmap tests and did not reproduce SIGFPE; its only new failures were two DeltaUpdateCatalogSuite cases. The upstream guard is supported by deterministic zero-duration regression coverage and the isolated native crash evidence.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>


          [VL] Keep Delta CI on the pinned Velox dependency

Remove the temporary UPSTREAM_VELOX_PR_ID=19208 override as requested. The completed unpatched CI passed all 12 bitmap tests without reproducing SIGFPE, so keep the upstream fix independent of this Delta configuration migration.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>


          [GLUTEN-13138][VL] Track DOUBLE partition Delta CI failures

7449f99

Record exactly the two DeltaUpdateCatalogSuite cases that fail on unsupported DOUBLE partition writes. Link their deferred fallback fix in the baseline; keep the tests and native execution enabled.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>


          [MINOR][DOC] Simplify Delta test configuration documentation

644a6c1

Keep the lasting rule that Delta extensions and catalog settings are suite-specific. Remove the duplicated mock-S3 rationale and fixture-replacement history from the README.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>


          [MINOR][VL] Remove the auxiliary Delta JVM-options test

87d30d5

Remove the Python property-presence test, its workflow step and path filter, and README instructions. Keep the shared configuration defaults, existing Delta integration suites, and known-failure gate unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI lite review requested due to automatic review settings

October 19, 2026 20:54

felipepessoto force-pushed the felipepessoto-delta-task-resource-failures branch from f98cb65 to 87d30d5 Compare

October 19, 2026 20:54

Copilot AI reviewed

View reviewed changes

Copilot AI left a comment

Contributor

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

2 open issues

_{Review effort: Lite}

.github/workflows/util/delta-spark-ut/apply-delta-test-patches.sh

.github/workflows/util/delta-spark-ut/apply-delta-test-patches.sh Outdated

Copilot stopped reviewing on behalf of felipepessoto due to an error

October 19, 2026 21:57

The job was not acquired by a runner even after multiple attempts.


          [MINOR][VL] Clarify the mock-S3 fixture patch diagnostic

c2161f6

Describe the expected ConvertToDeltaSuiteBase.scala fixture instead of hard-coding a Delta release. Retain the selected Delta ref and the unified-diff context marker behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: cf944462-056c-4604-886a-a69045318c7d

Copilot AI lite review requested due to automatic review settings

October 19, 2026 22:05

Copilot AI reviewed

View reviewed changes

Copilot AI left a comment

Contributor

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

2 open issues

2 resolved since last review

_{Review effort: Lite}

.github/workflows/util/delta-spark-ut/apply-delta-test-patches.sh

.github/workflows/util/delta-spark-ut/apply-delta-test-patches.sh

Copilot stopped reviewing on behalf of felipepessoto due to an error

October 19, 2026 22:33

The job was not acquired by a runner even after multiple attempts.


          [MINOR][VL] Remove passing Delta catalog tests from the baseline

95e9365

Drop four DeltaUpdateCatalogSuite entries that pass in the rebased CI run, along with their obsolete issue-reference comment. Keep the tests, native settings, and fail-on-fixed enforcement unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: cf944462-056c-4604-886a-a69045318c7d

Copilot AI lite review requested due to automatic review settings

October 20, 2026 02:13

Copilot started reviewing on behalf of felipepessoto

October 20, 2026 02:22

View session

Copilot AI reviewed

View reviewed changes

Copilot AI left a comment

Contributor

🟢 Approval recommended

No unresolved review issues were identified.

No open issues 🎉

2 resolved since last review

_{Review effort: Lite}

philo-he reviewed

View reviewed changes

philo-he left a comment

Member

Thanks for the PR. Could you check whether the test failure is related?

felipepessoto commented Oct 28, 2026

Contributor Author

Thanks for the PR. Could you check whether the test failure is related?

@philo-he I’ll check, but before merging this PR we need to first merge the #13129 (review)

This branch has not been deployed

No deployments

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

Reviewers

At least 1 approving review is required to merge this pull request.

Labels