Skip to content

Enable iceberg write by default, and disable dml iceberg operation on mor table by default.#13891

Merged
liurenjie1024 merged 6 commits into
NVIDIA:release/25.12from
liurenjie1024:ray/13520-1
Dec 1, 2025
Merged

Enable iceberg write by default, and disable dml iceberg operation on mor table by default.#13891
liurenjie1024 merged 6 commits into
NVIDIA:release/25.12from
liurenjie1024:ray/13520-1

Conversation

@liurenjie1024
Copy link
Copy Markdown
Collaborator

Fixes #13520.

Description

We shared out local microbenchmark internally, and our append and dml operation on copy on write table met goal. But dml operation on merge on read table didn't met goal due to table scan fallback to cpu, which requires #12298 to be implemented. So in this pr:

  1. Enable iceberg write by default
  2. Disable dml operation on merge on read table by default
  3. Add related it tests.

Checklists

  • This PR has added documentation for new or modified features or behaviors.
  • This PR has added new tests or modified existing tests to cover new code paths.
    (Please explain in the PR description how the new code paths are tested, such as names of the new/existing tests that cover them.)
  • Performance testing has been performed and its results are added in the PR description. Or, an issue has been filed with a link in the PR description.

@liurenjie1024 liurenjie1024 marked this pull request as draft November 27, 2025 11:10
@liurenjie1024 liurenjie1024 changed the title Ray/13520 1 Enable iceberg write by default, and disable dml iceberg operation on mor table by default. Nov 27, 2025
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Nov 27, 2025

Greptile Overview

Greptile Summary

This PR enables Iceberg write acceleration by default while keeping merge-on-read (MOR) DML operations disabled by default as experimental.

Key Changes

  • Enabled Iceberg write by default: Changed spark.rapids.sql.format.iceberg.write.enabled default from false to true (RapidsConf.scala:1699)
  • Disabled MOR DML by default: Added .disabledByDefault() to WriteDeltaExec to prevent merge-on-read UPDATE/DELETE/MERGE operations from running on GPU unless explicitly enabled (Spark350PlusNonDBShims.scala:228)
  • Comprehensive test coverage: Added three new fallback tests verifying that MOR DML operations correctly fall back to CPU when WriteDeltaExec is disabled
  • Documentation updated: Reflected config default changes in advanced_configs.md
  • CSV files synchronized: Updated all 10 supportedExecs.csv files across Spark versions (350-401) to mark WriteDeltaExec as NS with experimental warning

Rationale

According to the PR description, microbenchmarks showed that:

This phased approach allows users to benefit from proven COW acceleration while keeping experimental MOR DML disabled until performance issues are resolved.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The changes are well-structured with proper feature flagging, comprehensive test coverage for fallback scenarios, and consistent updates across all affected files. The phased enablement approach (COW enabled, MOR disabled) aligns with performance benchmarking results and provides safe defaults.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
sql-plugin/src/main/scala/com/nvidia/spark/rapids/RapidsConf.scala 5/5 Changed default value of ENABLE_ICEBERG_WRITE from false to true, enabling Iceberg write acceleration by default
sql-plugin/src/main/spark350/scala/com/nvidia/spark/rapids/shims/Spark350PlusNonDBShims.scala 5/5 Added .disabledByDefault() to WriteDeltaExec to disable merge-on-read (MOR) DML operations by default
integration_tests/src/main/python/iceberg/init.py 5/5 Added spark.rapids.sql.exec.WriteDeltaExec: "true" to test config with comment explaining it's needed for MOR DML operations
integration_tests/src/main/python/iceberg/iceberg_delete_test.py 5/5 Added test test_iceberg_delete_mor_fallback_writedelta_disabled to verify MOR DELETE falls back to CPU when WriteDeltaExec is disabled
integration_tests/src/main/python/iceberg/iceberg_update_test.py 5/5 Added test test_iceberg_update_mor_fallback_writedelta_disabled to verify MOR UPDATE falls back to CPU when WriteDeltaExec is disabled
integration_tests/src/main/python/iceberg/iceberg_merge_test.py 5/5 Added test test_iceberg_merge_mor_fallback_writedelta_disabled to verify MOR MERGE falls back to CPU when WriteDeltaExec is disabled

Sequence Diagram

sequenceDiagram
    participant User
    participant RapidsConf
    participant Spark350PlusNonDBShims
    participant IcebergProvider
    participant WriteDeltaExec
    
    Note over RapidsConf: ENABLE_ICEBERG_WRITE<br/>default: false → true
    
    User->>RapidsConf: Check isIcebergWriteEnabled
    RapidsConf-->>User: true (new default)
    
    Note over User: Copy-on-Write (COW) Operations
    User->>IcebergProvider: INSERT/APPEND on COW table
    IcebergProvider->>RapidsConf: Check isIcebergWriteEnabled
    RapidsConf-->>IcebergProvider: true
    IcebergProvider-->>User: Execute on GPU ✓
    
    Note over User: Merge-on-Read (MOR) DML Operations
    User->>Spark350PlusNonDBShims: UPDATE/DELETE/MERGE on MOR table
    Spark350PlusNonDBShims->>WriteDeltaExec: Check if enabled
    Note over WriteDeltaExec: disabledByDefault()<br/>(experimental)
    WriteDeltaExec-->>Spark350PlusNonDBShims: Disabled by default
    Spark350PlusNonDBShims-->>User: Fallback to CPU ✓
    
    Note over User: Enable MOR DML (Advanced)
    User->>RapidsConf: Set WriteDeltaExec=true
    User->>Spark350PlusNonDBShims: UPDATE/DELETE/MERGE on MOR table
    Spark350PlusNonDBShims->>WriteDeltaExec: Check if enabled
    WriteDeltaExec-->>Spark350PlusNonDBShims: Enabled by user
    Spark350PlusNonDBShims-->>User: Execute on GPU ✓
Loading

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@liurenjie1024 liurenjie1024 marked this pull request as ready for review December 1, 2025 05:38
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Signed-off-by: liurenjie1024 <liurenjie2008@gmail.com>
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Signed-off-by: liurenjie1024 <liurenjie2008@gmail.com>
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Copy Markdown
Collaborator

@res-life res-life left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@liurenjie1024
Copy link
Copy Markdown
Collaborator Author

build

@liurenjie1024 liurenjie1024 merged commit 9863ea8 into NVIDIA:release/25.12 Dec 1, 2025
62 checks passed
@sameerz sameerz added the task Work required that improves the product but is not user facing label Dec 1, 2025
@liurenjie1024 liurenjie1024 deleted the ray/13520-1 branch December 2, 2025 03:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

task Work required that improves the product but is not user facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants