[CELEBORN-2402] Log the hostname for celeborn shuffle fetch failures - #3780
Closed
r7raul1984 wants to merge 1 commit into
Closed
[CELEBORN-2402] Log the hostname for celeborn shuffle fetch failures#3780r7raul1984 wants to merge 1 commit into
r7raul1984 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves debuggability of Celeborn shuffle fetch failures by propagating the worker hostname into the FetchFailedException message, making it easier to correlate Spark task failures with specific Celeborn workers during incident investigation.
Changes:
- Extended
ExceptionMaker.makeFetchFailureExceptionto accept aworkerHostparameter. - Resolved the worker host in
CelebornInputStreamand passed it through when constructing fetch-failure exceptions. - Updated Spark-side exception maker and tests to validate the hostname is present in the exception message.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/spark-it/src/test/scala/org/apache/celeborn/tests/spark/CelebornFetchFailureSuite.scala | Updates integration test call site to provide the new workerHost argument. |
| common/src/main/java/org/apache/celeborn/common/util/ExceptionMaker.java | Extends the exception-maker interface to include workerHost. |
| client/src/main/java/org/apache/celeborn/client/read/CelebornInputStream.java | Extracts worker hostname from the current reader location and propagates it into fetch-failure exceptions. |
| client-spark/spark-3/src/test/scala/org/apache/spark/shuffle/celeborn/CelebornShuffleReaderSuite.scala | Adds/updates unit test ensuring the fetch-failure exception message includes the worker host. |
| client-spark/spark-3/src/main/scala/org/apache/spark/celeborn/ExceptionMakerHelper.scala | Embeds workerHost into the FetchFailedException message. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
r7raul1984
force-pushed
the
CELEBORN-2402
branch
from
August 4, 2026 12:25
f638d54 to
046bd1b
Compare
r7raul1984
force-pushed
the
CELEBORN-2402
branch
from
August 4, 2026 12:59
046bd1b to
a4967b2
Compare
Member
|
@r7raul1984, thanks for updates. Please use the template of pull request to update the description of this pull request. |
yew1eb
approved these changes
Aug 5, 2026
Member
|
Thanks. Merged to main(v1.0.0). |
turboFei
pushed a commit
that referenced
this pull request
Aug 6, 2026
### What changes were proposed in this pull request? Added `workerHost` parameter to the `ExceptionMaker.makeFetchFailureException()` interface and all its implementations (spark-3). `CelebornInputStream` now resolves the hostname from `currentReader.getLocation()` (falling back to `"unknown"` if unavailable) and embeds it in the `FetchFailedException` message. New exception message format: ``` Celeborn FetchFailure with appShuffleId/shuffleId: <id>/<id> from worker <host> ``` ### Why are the changes needed? Previously, the `FetchFailedException` message contained no information about which worker was involved in the failure. This made it difficult to identify the problematic worker during incident investigation. Embedding the hostname improves observability at no cost. ### Does this PR resolve a correctness bug? - [ ] Yes ### Does this PR introduce _any_ user-facing change? - [ ] Yes ### How was this patch tested? Added unit test in `CelebornShuffleReaderSuite` (spark-3) to verify that the worker hostname appears in the `FetchFailedException` message. Closes #3780 from r7raul1984/CELEBORN-2402. Authored-by: r7raul1984 <8199300+r7raul1984@users.noreply.github.com> Signed-off-by: Nicholas Jiang <programgeek@163.com> (cherry picked from commit e8464d6) Signed-off-by: Fei Wang <fwang12@ebay.com>
Member
|
I saw the jira ticket target version is 0.7.0. merged to branch-0.7 as well |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Added
workerHostparameter to theExceptionMaker.makeFetchFailureException()interface and all its implementations (spark-3).CelebornInputStreamnow resolves the hostname fromcurrentReader.getLocation()(falling back to"unknown"if unavailable) and embeds it in theFetchFailedExceptionmessage.New exception message format:
Why are the changes needed?
Previously, the
FetchFailedExceptionmessage contained no information about which worker was involved in the failure. This made it difficult to identify the problematic worker during incident investigation. Embedding the hostname improves observability at no cost.Does this PR resolve a correctness bug?
Does this PR introduce any user-facing change?
How was this patch tested?
Added unit test in
CelebornShuffleReaderSuite(spark-3) to verify that the worker hostname appears in theFetchFailedExceptionmessage.