Skip to content

[SPARK-58750][CORE] Tolerate FileAlreadyExistsException from checkpoint part file rename - #57976

Open
james-willis wants to merge 2 commits into
apache:masterfrom
james-willis:SPARK-58750
Open

[SPARK-58750][CORE] Tolerate FileAlreadyExistsException from checkpoint part file rename#57976
james-willis wants to merge 2 commits into
apache:masterfrom
james-willis:SPARK-58750

Conversation

@james-willis

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

ReliableCheckpointRDD.writePartitionToCheckpointFile renames the attempt-temp file onto the final part file and only handles a rename that reports failure by returning false (HDFS semantics). This PR additionally catches FileAlreadyExistsException from that rename and routes it into the same existing handling: if the final part file exists, another attempt of this task already committed it, so the temp file is deleted and the write is treated as successful. If the destination does not exist, the existing checkpointFailedToSaveError is still thrown.

Why are the changes needed?

Since HADOOP-16721 (Hadoop 3.3.1), S3A deliberately raises FileAlreadyExistsException when the rename destination is an existing file, instead of returning false as HDFS does. ABFS behaves the same way. The Hadoop FileSystem specification does not guarantee HDFS-style false reporting.

Under speculative execution (or a zombie attempt racing a retry), two attempts of the same checkpoint task race to rename onto the same final part file. On HDFS the loser sees rename() == false and Spark correctly treats it as "some other copy of this task must've finished before us". On S3A/ABFS the loser gets an unhandled FileAlreadyExistsException, which fails the task — and because the destination now permanently exists, every retry of that task fails on the same rename, so spark.task.maxFailures is always exhausted and the job aborts, even though the checkpoint data was written successfully by the winning attempt.

Observed in production (Spark 4.0.1, Hadoop 3.4.1, spark.speculation=true):

org.apache.hadoop.fs.FileAlreadyExistsException: Failed to rename
s3://<bucket>/<prefix>/spark-checkpoints/<uuid>/rdd-207/.part-00379-attempt-25364
to s3://<bucket>/<prefix>/spark-checkpoints/<uuid>/rdd-207/part-00379; destination file exists
    at org.apache.hadoop.fs.s3a.S3AFileSystem.initiateRename(S3AFileSystem.java:2468)
    at org.apache.hadoop.fs.s3a.S3AFileSystem.rename(S3AFileSystem.java:2392)
    at org.apache.spark.rdd.ReliableCheckpointRDD$.writePartitionToCheckpointFile(ReliableCheckpointRDD.scala:229)
    ...
ERROR TaskSetManager: Task 379 in stage 105.0 failed 4 times; aborting job

See SPARK-58750 for full details. Structured Streaming's CheckpointFileManager was already hardened for divergent rename semantics; the RDD checkpoint writer is the remaining caller assuming HDFS semantics.

Does this PR introduce any user-facing change?

No. RDD checkpointing to S3A/ABFS under speculative execution (or task retry after a committed rename) now succeeds instead of unrecoverably failing the job, which is the bug fix itself.

How was this patch tested?

Added a regression test to CheckpointStorageSuite that writes the same checkpoint partition from two task attempts against a FileSystem mimicking S3A's rename semantics (raises FileAlreadyExistsException when the destination file exists). Without the fix, the second attempt throws and would fail the task; with the fix, it succeeds and exactly one committed part file remains, with the attempt-temp file cleaned up.

Ran build/sbt "core/testOnly org.apache.spark.CheckpointStorageSuite" locally.

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

Generated-by: Claude Code (model claude-fable-5)

@james-willis
james-willis marked this pull request as ready for review August 12, 2026 20:09
@james-willis
james-willis marked this pull request as draft August 12, 2026 20:30
@james-willis
james-willis marked this pull request as ready for review August 12, 2026 22:58
@james-willis

Copy link
Copy Markdown
Contributor Author

maybe @viirya or @dongjoon-hyun would be good reviewers on this.

@uros-b

uros-b commented Aug 13, 2026

Copy link
Copy Markdown
Member

Thank you @james-willis!

@uros-b
uros-b requested review from dongjoon-hyun and viirya August 13, 2026 07:25

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

Verified the fix against the surrounding commit logic and it's correct. Mapping FileAlreadyExistsException to false funnels the S3A/ABFS loser attempt into the exact same if (!renamed) path that HDFS's false already exercises, and it keeps the fs.exists(finalOutputPath) recheck: the exception is demoted to "rename didn't succeed", not assumed to be success. If the destination genuinely doesn't exist, checkpointFailedToSaveError still fires. The try wraps only the rename call and catches just that one exception type.

The direction matches existing practice: Structured Streaming's CheckpointFileManager already catches FileAlreadyExistsException around renameTempFile and treats an existing destination as benign (CheckpointFileManager.scala:162). This brings the RDD checkpoint writer — the remaining caller assuming HDFS semantics — in line.

The test is a real regression test: FileAlreadyExistsRenameFileSystem overrides only rename to raise on an existing destination, so the first attempt commits via super.rename and the second hits the injected exception, reproducing the race; it asserts exactly one committed part-00000 with the temp file cleaned up.

One nit, non-blocking: the new logDebug(s"...", e) uses string interpolation while the surrounding logs in this method use structured logging (log"... ${MDC(...)}"). Since it's a debug-level line, it's fine either way.

LGTM.

@james-willis

Copy link
Copy Markdown
Contributor Author

Thanks for the reviews @viirya and @uros-b

I updated the logDebug to use structured logging to match the surrounding logs.

I'm not a committer so cannot merge myself.

Also lmk what, if any, versions I should backport this to. Once this merges I'll raise the ports to the appropriate branches

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants