Spark 4.1: Add rewrite option to enable executor cache for delete files by anuragmantri · Pull Request #17868 · apache/iceberg

anuragmantri · 2026-08-28T20:26:04Z

Related to #11648.

RewriteDataFilesSparkAction has disabled the executor cache for delete files unconditionally since #13820. As noted in the discussion on #11648, that is the right default for position deletes but costly for equality deletes, which cannot be narrowed to a single data file while reading and are therefore re-read in full for every data file whose bounds overlap.

This adds a rewrite option so users can opt back in, where a Spark executor cache setting is exposed as a rewrite option rather than a session property.

Existing Default behavior is unchanged.

Usage from SQL:

CALL catalog_name.system.rewrite_data_files(
  table => 'db.sample',
  options => map('executor-cache.delete-files.enabled', 'true')
);

Usage from the action API:

SparkActions.get(spark)
    .rewriteDataFiles(table)
    .option(RewriteDataFilesSparkAction.EXECUTOR_CACHE_DELETE_FILES_ENABLED, "true")
    .execute();

Tests added to TestSparkExecutorCache assert the number of times each delete file is opened during a rewrite: 2 per delete file with the option at its default, once per data file, and 1 with the option enabled.

AI Disclosure

Model: Claude Opus 5
Platform/Tool: Claude Code
Human Oversight: fully reviewed by me after the changes were made.
Prompt Summary: Make the executor cache optional during RewriteDataFiles via a new rewrite option, keeping the existing default, then review the change against AGENTS.md and add tests.

anuragmantri · 2026-08-28T20:27:01Z

@RussellSpitzer @kinolaev - Could you take a look? Thanks!

RussellSpitzer · 2026-08-28T20:47:43Z

 | `output-spec-id` | current partition spec id | Identifier of the output partition spec. Data will be reorganized during the rewrite to align with the output partitioning. |
 | `remove-dangling-deletes` | false | Remove dangling position and equality deletes after rewriting. A delete file is considered dangling if it does not apply to any live data files. Enabling this will generate an additional commit for the removal. |
 | `max-files-to-rewrite` | null | This option sets an upper limit on the number of eligible files that will be rewritten. If this option is not specified, all eligible files will be rewritten. |
+| `executor-cache.delete-files.enabled` | false | Use the executor cache for delete files while rewriting. Enable this when the same delete file applies to many data files, which is common with equality deletes |


let's try to keep this to one snake case thing.
cache-delete-files?

Given that the Spark property is called spark.sql.iceberg.executor-cache.enabled and we already have the partial-progress.enabled option on this procedure, my preference would be executor-cache.enabled.

I'd rather we not use a "." property if we can help. The partial progress ones are not a good example based on all the other procedure options we have. All the others use the kabob thing and I think it's because only partial progress was originally defined in the "action".

enable-executor-cache is also fine, but I don't think it really explains what the option is doing. It only effects deletes and specifically delete files so I'd try to keep the name tied to that functionality

I kept it to cache-delete-files. This is concise and conveys the intention.

RussellSpitzer · 2026-08-28T20:48:52Z

+   *
+   * <p>Defaults to false.
+   */
+  public static final String EXECUTOR_CACHE_DELETE_FILES_ENABLED =


As I mentioned above, lets just drop the other parts here, I think just CACHE_DELETE_FILES is fine

RussellSpitzer · 2026-08-28T20:50:01Z

  public void testExecutorCacheForDeleteFilesDisabled() {
    Table table = createTablePartitioned(1, 1);
    RewriteDataFilesSparkAction action = SparkActions.get(spark).rewriteDataFiles(table);
+    action.execute();


Ok I can see this is because we moved it into "validate"

Since we are in the same package can we just call validate rather than actually executing?

Ok we can't do that. Maybe we should move this back to the constructor?

We can't, because.option() is called after the constructor

That's too bad. i'm trying to figure out how we can avoid running 3 additional spark rewrites in the tests here when all we really care about is the conf being on or off.

Okay, I looked at all the other options tests. All test behavior. This one is different. IMO, we have covered the behavior tests in TestSparkExecutorCache so we probably don't need these tests here. I removed them. Let me know if you feel otherwise.

Can we make the init method package-private? I checked, and the removed tests pass when calling init(0L) instead of execute.
I am also fine with the current tests in TestSparkExecutorCache.

kinolaev · 2026-08-28T21:43:28Z

LGTM. Although I prefer a slightly shorter name (see above), the current option is also fine with me. Thanks @anuragmantri for the PR!

…ests

anuragmantri

Thanks for the quick reviews.

I also thought about the three options we have. It can be confusing but I kept all three. My rationale is that these serve slightly different purposes.

executor-cache.enabled - Main switch to control caching, also gates locality. Predates my PRs to add ability to disable delete files only.
executor-cache.delete-files.enabled - A scan option narrowed down for delete files cache onl.
cache-delete-files this is a rewrite option that controls 2 for scans during rewrites.

anuragmantri · 2026-08-28T22:11:55Z

 | `output-spec-id` | current partition spec id | Identifier of the output partition spec. Data will be reorganized during the rewrite to align with the output partitioning. |
 | `remove-dangling-deletes` | false | Remove dangling position and equality deletes after rewriting. A delete file is considered dangling if it does not apply to any live data files. Enabling this will generate an additional commit for the removal. |
 | `max-files-to-rewrite` | null | This option sets an upper limit on the number of eligible files that will be rewritten. If this option is not specified, all eligible files will be rewritten. |
+| `executor-cache.delete-files.enabled` | false | Use the executor cache for delete files while rewriting. Enable this when the same delete file applies to many data files, which is common with equality deletes |


I kept it to cache-delete-files. This is concise and conveys the intention.

anuragmantri · 2026-08-28T22:12:05Z

+   *
+   * <p>Defaults to false.
+   */
+  public static final String EXECUTOR_CACHE_DELETE_FILES_ENABLED =


anuragmantri · 2026-08-28T22:23:49Z

  public void testExecutorCacheForDeleteFilesDisabled() {
    Table table = createTablePartitioned(1, 1);
    RewriteDataFilesSparkAction action = SparkActions.get(spark).rewriteDataFiles(table);
+    action.execute();


Okay, I looked at all the other options tests. All test behavior. This one is different. IMO, we have covered the behavior tests in TestSparkExecutorCache so we probably don't need these tests here. I removed them. Let me know if you feel otherwise.

Spark 4.1: Add rewrite option to enable executor cache for delete files

68ae363

github-actions Bot added spark docs labels Aug 28, 2026

nssalian requested a review from RussellSpitzer August 28, 2026 20:26

anuragmantri mentioned this pull request Aug 28, 2026

SparkExecutorCache causes slowness of RewriteDataFilesSparkAction #11648

Open

3 tasks

RussellSpitzer reviewed Aug 28, 2026

View reviewed changes

Spark 4.1: Rename option to cache-delete-files and drop config-only t…

a6240b3

…ests

anuragmantri commented Aug 28, 2026

View reviewed changes

Uh oh!

Conversation

anuragmantri commented Aug 28, 2026

Uh oh!

anuragmantri commented Aug 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RussellSpitzer Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anuragmantri Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kinolaev commented Aug 28, 2026

Uh oh!

anuragmantri left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anuragmantri Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

RussellSpitzer Aug 28, 2026 •

edited

Loading

anuragmantri Aug 28, 2026 •

edited

Loading

anuragmantri Aug 28, 2026 •

edited

Loading