venkata91 opened a new pull request, #3856: URL: https://github.com/apache/celeborn/pull/3856
## Symptom A Spark job pushing to a 498-worker Celeborn cluster emitted **20,231,990 hard-split INFO events in 7 minutes** (~48,000/s fleet-wide) from a handful of partitions, alongside a synchronised storm of push retries. Shortly after, DNS resolution for the worker headless service began failing, worker-to-worker replication broke, and the job died with `SLOT_NOT_AVAILABLE` while the cluster had ample free disk and every worker still registered. ## Cause Three independent things compound. **1. The SOFT split branch can be silently unreachable.** In `PushDataHandler.checkDiskFullAndSplit`, a split is evaluated when `fileLength > splitThreshold`, and is SOFT only if `fileLength < partitionSplitMaximumSize`. When a deployment sets `celeborn.worker.shuffle.partitionSplit.max` equal to (or below) `celeborn.client.shuffle.partitionSplit.threshold`, those two conditions are mutually exclusive: every split is HARD, and `partitionSplit.mode: SOFT` has no effect. Nothing warns about this, and the two configs live on opposite sides of the client/worker split so nothing validates them against each other. **2. A wedged split file re-logs and re-fails on every push.** A hard split does not shrink the file, so until a new epoch propagates to every pusher, each concurrent push to that file fails the same threshold check and logs the full multi-line `CheckDiskFullAndSplit hardSplit` INFO again. That is the 20M events. The sibling warning in `handlePushMergedData` fires once per hard-split partition per merged push, and attributes every hard split to `disk full` — `checkDiskFullAndSplit` returns `HARD_SPLIT` for three distinct reasons and only one is disk pressure, so that line actively misdirects triage. **3. Retries have no backoff, and the HARD_SPLIT path has no bound.** Retries are resubmitted to `pushDataRetryPool` immediately. The `dueTime` computed at each site is *not* a retry delay — it is the deadline for the revive busy-wait inside the retry task — so the effective cadence is bounded only by `celeborn.client.push.revive.interval` (100ms). Worse, the two HARD_SPLIT sites pass `remainReviveTimes` through **without decrementing it**, so a partition wedged in HARD_SPLIT never exhausts its revive budget and the loop is unbounded. ## How this became a DNS failure Every retry ends at `dataClientFactory.createClient(...)`, so each one is a fresh connection setup, and each connection setup is a DNS resolution of `celeborn-worker-N.celeborn-worker-headless...`. Thousands of map tasks blocked on the same wedged partitions, retrying in lockstep every ~100ms with no bound, turned into a sustained resolution load against cluster DNS. Resolution latency then climbed to ~20s and began returning `UnknownHostException` for worker hostnames. Because push replication was enabled, a single unresolvable peer failed every push routed through it, workers were marked unresponsive, and revive had nowhere to land. We can't prove from our data that the retry storm alone exhausted DNS, and there may have been an independent cluster-side trigger. What is clear is that an unbounded, unjittered, connection-establishing retry loop is a large amplifier pointed directly at name resolution. ## Changes **Commit 1 — log the hard split cause once per split file.** Latches on the `PartitionDataWriter`, which lives and dies with the file, so no cleanup hook is needed. Follows the shape of `1e39758cb`. `WorkerSource.WRITE_DATA_HARD_SPLIT_COUNT` already carries the rate. The misleading merged-push warning moves to `debug` with corrected wording. Behavior is unchanged, this is log noise only. **Commit 2 — exponential backoff with jitter on push data retries.** `pushDataRetryPool` becomes a `ScheduledExecutorService` so the delay costs no pool thread; sleeping in the task would be worse than the problem, since the pool defaults to 8 threads that already block on the revive wait. Applied to all five retry sites, not only HARD_SPLIT: the failure sites fire exactly when a worker is already sick, and that is what turns one bad worker into a connection storm. | Config | Default | Rationale | |---|---|---| | `celeborn.client.push.retry.backoff.initial` | `500ms` | ~3-5x the current effective cadence; `0` disables | | `celeborn.client.push.retry.backoff.max` | `10s` | reached in ~5 retries; far below the 120s push timeout | | `celeborn.client.push.retry.backoff.jitter` | `0.5` | equal jitter, so delay never collapses to 0 | ## Open questions for reviewers 1. **Thread-pool metrics regression.** `newDaemonCachedThreadPool` calls `ThreadPoolSource.registerSource`; `newDaemonThreadPoolScheduledExecutor` does not, so `celeborn-retry-sender` loses its pool metrics. Options: accept, add `registerSource` to the scheduled factory (affects other callers), or keep the cached pool and add a separate single-thread scheduler as in `CommitManager`. Happy to take direction. 2. **Config naming.** `celeborn.client.push.retry.backoff.*` groups with `push.retry.threads`; `push.revive.backoff.*` would group with `revive.interval`/`maxRetries`. Either is defensible. 3. **Merged-data HARD_SPLIT site** takes the initial delay with jitter rather than exponential growth, because `remainReviveTimes` is a parameter there with no per-batch counter. Threading one through `doPushMergedData` is left as follow-up. 4. **Deliberately not included:** making the HARD_SPLIT sites decrement `remainReviveTimes`. That would bound those retries and start failing tasks — a real behaviour change that deserves its own ticket. Worth doing, separately. 5. **Should the worker warn at startup when `partitionSplit.max <= partitionSplit.threshold`?** That combination silently disables a documented feature and is what set this incident up. Happy to add it here or file it separately. ## Testing status **Not yet tested — this is why the PR is a draft.** The author's machine has only JDK 24 and the build requires Java 8/11/17 (`common` fails on `sun.misc.Signal`), so nothing could be compiled or run locally. The edited Java parses cleanly under `javac -proc:none`, which is the only verification performed. Specifically still owed, and I'll add them before marking ready: - Unit test for `pushRetryBackoffMs`: monotonic growth, cap, jitter bounds, `0` disables, no overflow at large retry counts. - Once-per-file test in `PushDataHandlerSuite` alongside the existing `checkDiskFullAndSplit` cases. - Regenerate `docs/configuration/client.md` via `ConfigurationSuite` with `UPDATE=1` — **the rows in this PR are hand-written and may not match the generator byte-for-byte.** - `dev/reformat` for spotless/scalafmt. - Run `RetryReviveTest` as regression. JIRA ticket to be filed; `CELEBORN-XXXX` placeholders will be updated. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
