You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "wang-haihua (via GitHub)" <gi...@apache.org> on 2026/08/03 09:25:36 UTC
[PR] [CELEBORN-2401] Change default of celeborn.client.spark.fetch.cleanFailedShuffle to true [celeborn]
wang-haihua opened a new pull request, #3779:
URL: https://github.com/apache/celeborn/pull/3779
### What changes were proposed in this pull request?
Flip the default of `celeborn.client.spark.fetch.cleanFailedShuffle` from `false` to `true`.
Only two lines change:
- `common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala` — `createWithDefault(false)` → `createWithDefault(true)` on `CLIENT_FETCH_CLEAN_FAILED_SHUFFLE`.
- `docs/configuration/client.md` — the golden config table is updated in the same commit so `ConfigurationSuite` stays green.
### Why are the changes needed?
`celeborn.client.spark.fetch.cleanFailedShuffle` was introduced in 0.6.0 by [CELEBORN-1896](https://issues.apache.org/jira/browse/CELEBORN-1896) (PR #3109) and hardened in 0.7.0 by [CELEBORN-2088](https://issues.apache.org/jira/browse/CELEBORN-2088) (PR #3401). When enabled:
1. The driver records shuffleIds that hit `FetchFailed`.
2. When the retried stage registers a new shuffleId, the previously failed shuffleId is enqueued for cleanup by `FailedShuffleCleaner`.
3. `FailedShuffleCleaner` reuses the existing `UnregisterShuffle` path to reclaim the stale shuffle data on workers.
Since 0.4.0 `celeborn.client.spark.stageRerun.enabled` has defaulted to `true`, so `FetchFailed` stage rerun is already the out-of-the-box behavior. However `cleanFailedShuffle` still defaults to `false`, so every retried stage silently leaves its old shuffle data on the workers until the app exits or the periodic expired-shuffle cleanup catches it. The two defaults contradict each other.
A recent production incident on our cluster:
- A single stage retried three times.
- Retry #1 wrote ~143 TiB of shuffle data before failing.
- Retry #2 wrote another ~143 TiB before failing.
- Retry #3 was still running, ~61 TiB written so far.
- **300+ TiB of stale shuffle bytes were sitting on the workers simultaneously**, none of which the job would ever fetch again.
On a shared cluster this drives workers into disk-full / high-usage exclusion, cascading into more `FetchFailed` events for unrelated applications and destabilizing the cluster until the offending app exits. Flipping the default removes this failure mode without adding any new deletion path.
The proposal was raised on dev@ and supported there; see the [discussion thread](https://lists.apache.org/list.html?dev@celeborn.apache.org) for context.
### Does this PR introduce _any_ user-facing change?
Yes — behavior change with an explicit escape hatch.
- **Before**: after a `FetchFailed`-driven stage rerun, the failed shuffleId's data lingered on the workers until the application exited (or `celeborn.worker.shuffle.expired.timeout` caught it).
- **After**: the failed shuffleId's data is reclaimed as soon as the retried stage registers, via the existing `UnregisterShuffle` path.
- **Rollback**: users who need the previous behavior can set `celeborn.client.spark.fetch.cleanFailedShuffle=false` in their Spark configuration.
The release notes should call out both the new default and the rollback flag.
### How was this patch tested?
- `ConfigurationSuite` — golden `docs/configuration/client.md` regenerated to keep the doc test green.
- `CelebornFetchFailureDiskCleanSuite` — the end-to-end integration test for this path already sets `spark.celeborn.client.spark.fetch.cleanFailedShuffle=true` explicitly and continues to pass; it exercises the exact code path affected by this default flip.
- No new deletion route or code path is introduced; the mechanism is the existing `FailedShuffleCleaner` → `UnregisterShuffle` flow that has shipped since 0.6.0.
Related JIRA: [CELEBORN-2401](https://issues.apache.org/jira/browse/CELEBORN-2401).
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2401] Change default of celeborn.client.spark.fetch.cleanFailedShuffle to true [celeborn]
Posted by "zaynt4606 (via GitHub)" <gi...@apache.org>.
zaynt4606 commented on PR #3779:
URL: https://github.com/apache/celeborn/pull/3779#issuecomment-5238333887
Thanks for the contribution! Merged to `main` (00113daf) and `branch-0.7` (190c5132).
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2401] Change default of celeborn.client.spark.fetch.cleanFailedShuffle to true [celeborn]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #3779:
URL: https://github.com/apache/celeborn/pull/3779#issuecomment-5201238123
## [Codecov](https://app.codecov.io/gh/apache/celeborn/pull/3779?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 58.56%. Comparing base ([`07dde50`](https://app.codecov.io/gh/apache/celeborn/commit/07dde50e4043e096e385197c53208e21e8d316ce?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`1bdb2ce`](https://app.codecov.io/gh/apache/celeborn/commit/1bdb2cedb91c9abb083e1c6685a7b78008b599a3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 4 commits behind head on main.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #3779 +/- ##
============================================
+ Coverage 58.41% 58.56% +0.16%
Complexity 229 229
============================================
Files 398 399 +1
Lines 27993 28047 +54
Branches 2734 2738 +4
============================================
+ Hits 16349 16423 +74
+ Misses 10446 10425 -21
- Partials 1198 1199 +1
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/celeborn/pull/3779?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2401] Change default of celeborn.client.spark.fetch.cleanFailedShuffle to true [celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #3779:
URL: https://github.com/apache/celeborn/pull/3779#discussion_r3703722146
##########
common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala:
##########
@@ -5258,7 +5258,7 @@ object CelebornConf extends Logging {
.version("0.6.0")
.doc("whether to clean those disk space occupied by shuffles which cannot be fetched")
.booleanConf
- .createWithDefault(false)
+ .createWithDefault(true)
Review Comment:
Please add some explanation in `migration.md`.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2401] Change default of celeborn.client.spark.fetch.cleanFailedShuffle to true [celeborn]
Posted by "wang-haihua (via GitHub)" <gi...@apache.org>.
wang-haihua commented on PR #3779:
URL: https://github.com/apache/celeborn/pull/3779#issuecomment-5200214922
@yew1eb Hi, CI on the latest commit 1bdb2cedb is queued in action_required, would you mind approving?
The two failures on the previous run (dd4142024) look unrelated to this PR: QuotaManagerSuite: test tenant level conf and CelebornFetchFailureDiskCleanSuite: the failed shuffle file is cleaned up correctly.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2401] Change default of celeborn.client.spark.fetch.cleanFailedShuffle to true [celeborn]
Posted by "zaynt4606 (via GitHub)" <gi...@apache.org>.
zaynt4606 closed pull request #3779: [CELEBORN-2401] Change default of celeborn.client.spark.fetch.cleanFailedShuffle to true
URL: https://github.com/apache/celeborn/pull/3779
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2401] Change default of celeborn.client.spark.fetch.cleanFailedShuffle to true [celeborn]
Posted by "wang-haihua (via GitHub)" <gi...@apache.org>.
wang-haihua commented on code in PR #3779:
URL: https://github.com/apache/celeborn/pull/3779#discussion_r3703868345
##########
common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala:
##########
@@ -5258,7 +5258,7 @@ object CelebornConf extends Logging {
.version("0.6.0")
.doc("whether to clean those disk space occupied by shuffles which cannot be fetched")
.booleanConf
- .createWithDefault(false)
+ .createWithDefault(true)
Review Comment:
Thanks, added a one-line migration note in the latest commit d689a1037, under # Upgrading from 0.6 to 0.7.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org