[SPARK-58750][CORE] Tolerate FileAlreadyExistsException from checkpoint part file rename - #57976
[SPARK-58750][CORE] Tolerate FileAlreadyExistsException from checkpoint part file rename#57976james-willis wants to merge 2 commits into
Conversation
…nt part file rename
|
maybe @viirya or @dongjoon-hyun would be good reviewers on this. |
|
Thank you @james-willis! |
viirya
left a comment
There was a problem hiding this comment.
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.
What changes were proposed in this pull request?
ReliableCheckpointRDD.writePartitionToCheckpointFilerenames the attempt-temp file onto the final part file and only handles a rename that reports failure by returningfalse(HDFS semantics). This PR additionally catchesFileAlreadyExistsExceptionfrom 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 existingcheckpointFailedToSaveErroris still thrown.Why are the changes needed?
Since HADOOP-16721 (Hadoop 3.3.1), S3A deliberately raises
FileAlreadyExistsExceptionwhen the rename destination is an existing file, instead of returningfalseas HDFS does. ABFS behaves the same way. The Hadoop FileSystem specification does not guarantee HDFS-stylefalsereporting.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() == falseand Spark correctly treats it as "some other copy of this task must've finished before us". On S3A/ABFS the loser gets an unhandledFileAlreadyExistsException, which fails the task — and because the destination now permanently exists, every retry of that task fails on the same rename, sospark.task.maxFailuresis 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):See SPARK-58750 for full details. Structured Streaming's
CheckpointFileManagerwas 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
CheckpointStorageSuitethat writes the same checkpoint partition from two task attempts against aFileSystemmimicking S3A's rename semantics (raisesFileAlreadyExistsExceptionwhen 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)