Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CELEBORN-1335] RpcTimeoutException should inherit from CelebornIOException #2395

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

AngersZhuuuu
Copy link
Contributor

What changes were proposed in this pull request?

We meet a case Spark task throw RpcTimeoutException, then causing all executor excluded when enable
spark.excludeOnFailure.application.fetchFailure.enabled.
Even we have ignore CelebornIOException in spark side.
RpcTimeoutException should also inherit from CelebornIOException

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Copy link

codecov bot commented Mar 18, 2024

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 48.97%. Comparing base (96df0d6) to head (0edada3).
Report is 17 commits behind head on main.

Files Patch % Lines
.../org/apache/celeborn/common/util/ThreadUtils.scala 0.00% 3 Missing and 1 partial ⚠️
...la/org/apache/celeborn/common/rpc/RpcTimeout.scala 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2395      +/-   ##
==========================================
+ Coverage   48.74%   48.97%   +0.24%     
==========================================
  Files         209      209              
  Lines       13021    13113      +92     
  Branches     1121     1136      +15     
==========================================
+ Hits         6346     6421      +75     
- Misses       6264     6273       +9     
- Partials      411      419       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@RexXiong
Copy link
Contributor

Celeborn check TimeoutException occurs in many places, if we change RpcTimeoutException to CelebornIOException, it may lead to unexpected behavior.

@RexXiong RexXiong marked this pull request as draft March 28, 2024 03:25
Copy link

This PR is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the stale label Apr 17, 2024
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 48.97%. Comparing base (96df0d6) to head (0edada3).
Report is 69 commits behind head on main.

Files Patch % Lines
.../org/apache/celeborn/common/util/ThreadUtils.scala 0.00% 3 Missing and 1 partial ⚠️
...la/org/apache/celeborn/common/rpc/RpcTimeout.scala 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2395      +/-   ##
==========================================
+ Coverage   48.74%   48.97%   +0.24%     
==========================================
  Files         209      209              
  Lines       13021    13113      +92     
  Branches     1121     1136      +15     
==========================================
+ Hits         6346     6421      +75     
- Misses       6264     6273       +9     
- Partials      411      419       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants