You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "cravani (via GitHub)" <gi...@apache.org> on 2026/10/14 17:22:28 UTC

[PR] [SPARK-60246][CORE] Close the download channel when fetchFile fails [spark]

cravani opened a new pull request, #59472:
URL: https://github.com/apache/spark/pull/59472

   ### What changes were proposed in this pull request?
   - `Utils.downloadFile` now closes its input stream in the `finally` block, so the stream is closed on every path, including when creating the temp file fails.
   - `FileDownloadCallback.onData` stops writing once the reader has closed its end of the pipe and discards the rest of that stream, with a debug log. Before, the write failed with `IOException: Broken pipe`, which closed the shared connection and failed every other download on it.
   
   ### Why are the changes needed?
   When `Utils.fetchFile` fails before it reads a `spark://` download, for example because the target directory was deleted, the download channel is never closed. The driver keeps sending the file. The executor's file-download Netty thread (`spark.files.io.threads` defaults to 1) then blocks forever in `FileDownloadCallback.onData` writing to a full pipe. After that, every jar, file, and REPL class download through that `RpcEnv` hangs, and so does every task that needs one.
   
   ### Does this PR introduce _any_ user-facing change?
   Yes. A failed `spark://` file fetch no longer blocks later file and class downloads on the executor.
   
   ### How was this patch tested?
   New tests in `NettyRpcEnvSuite`. `SslNettyRpcEnvSuite` and `AutoNettyRpcEnvSuite` run them too.
   
   - `a failed file fetch must not leave its download channel open`: fetches a 16 MiB jar into a missing directory, then fetches a small jar through the same `RpcEnv`. Without the fix, the second fetch hangs and the test fails after 60 seconds.
   - `a download reader closing early must not fail other downloads on the connection`: opens two downloads on one connection and closes the first without reading it. Without the `onData` change, the second download fails with `Broken pipe`.
   - `a file fetch failing after the download keeps its error and the connection usable`: the original `SparkException` still surfaces, and later fetches still work.
   
   The existing `file server` test still passes.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   Yes, Claude Opus 5.5
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-60246][CORE] Close the download channel when fetchFile fails [spark]

Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang closed pull request #59472: [SPARK-60246][CORE] Close the download channel when fetchFile fails
URL: https://github.com/apache/spark/pull/59472


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-60246][CORE] Close the download channel when fetchFile fails [spark]

Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #59472:
URL: https://github.com/apache/spark/pull/59472#issuecomment-6179457912

   **Merge Summary:**
   - merged into master https://github.com/apache/spark/commit/de8bfbe658d6288d48d6b235fc0646d05f4658b8
   - merged into branch-4.x https://github.com/apache/spark/commit/87e4c8ad40d8ca3686cef7920903d04ca8c8ad29
   
   *Posted by `merge_spark_pr.py`*


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org