[CELEBORN-2310] Reject RESERVE_SLOTS when disks are full

[saurabhd336]

What changes were proposed in this pull request?

Disk full only lead to HARD_SPLITs as a response to writes. However, doesn't lead to reserve slot rejections. This means too many write retries (due to HARD_SPLITs on each write attempt) leads to wasted network I/O. We can reject RESERVE_SLOT during disk full to avoid the wasted data write network IO.

Why are the changes needed?

Reject reserve slots during disk full, avoid unnecessary network IO.

Does this PR resolve a correctness bug?

No.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Added UTs, CI.

[Copilot]

Pull request overview

This PR updates the worker’s local-disk “availability” checks to treat disks with actualUsableSpace <= 0 as unusable, so RESERVE_SLOTS can be rejected earlier during disk-full scenarios and avoid wasted push/write network I/O.

Changes:

• Refine healthyWorkingDirs() to exclude disks that are HEALTHY but have actualUsableSpace <= 0.
• Refine createDiskFile() directory selection to avoid using a suggested mount point when its disk has no usable space.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[SteNicholas]

Review: [CELEBORN-2310] Reject RESERVE_SLOTS when disks are full

PR: #3666 | Author: saurabhd336 | Change: +6 / -2 in StorageManager.scala

• What it does

Adds an actualUsableSpace > 0 check in two places so that workers reject RESERVE_SLOTS requests upfront when local disks are full, instead of accepting the reservation and only triggering a HARD_SPLIT after wasted network I/O on write.

• Looks good

  • Correct and low-risk — if a disk has zero usable space, rejecting early is the right call. The staleness window of actualUsableSpace matches the existing DiskStatus.HEALTHY refresh cycle, so no new race conditions are introduced.
  • Narrow scope — surgical change in the right places (healthyWorkingDirs() and createDiskFile()).

• Issues / suggestions

  1. Duplicated predicate — (status == HEALTHY) && (actualUsableSpace > 0) now appears in two separate locations. Extract a helper like isDiskWritable(diskInfo): Boolean to centralize this and prevent future divergence.
  2. No unit tests — A test for healthyWorkingDirs() excluding a HEALTHY disk with actualUsableSpace = 0 would be straightforward and would guard against regressions.
  3. Formatting nit — The multi-line if in createDiskFile splits awkwardly mid-.equals(). Prefer aligning each condition on its own line:
     if (diskInfo != null &&
     diskInfo.status.equals(DiskStatus.HEALTHY) &&
     diskInfo.actualUsableSpace > 0) {
  4. Threshold consideration — > 0 is a fine start, but a disk with 1 byte free will pass the check and immediately fail on write. A configurable minimum space threshold could be a good follow-up.

• Verdict
  Approve with minor comments. The logic is sound and the change is valuable. Adding a unit test and extracting the duplicated predicate would make it merge-ready.

[SteNicholas]

@saurabhd336, thanks for contribution. Please address above comments of Copilot and Claude Code.

[saurabhd336]

@SteNicholas Addressed comments. PTAL!

[github-actions]

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.

[SteNicholas]

@saurabhd336, could you please rebase the latest main branch to resolve the conflicts?

[saurabhd336]

@SteNicholas Thanks for helping review. Rebased