[CELEBORN-2060] Refactor master slot allocation with strategy abstraction

[Kalvin2077]

What changes were proposed in this pull request?

This PR refactors master-side slot allocation so that strategy-specific slot-budget computation is
separated from the shared allocation workflow. The existing round-robin and load-aware algorithms
are moved behind SlotsAssignStrategy, while SlotsAllocator retains the common progressive
allocation and best-effort fallback behavior.

The change is intentionally structural. Existing names and allocation behavior are preserved where
possible so that the old implementation can be compared directly with the refactored code.

The refactor also exposed several potential correctness issues that are intentionally left for
separate follow-up changes:

• With replication enabled, remote-only storage and rack awareness disabled, primary and replica
  locations may be assigned to the same worker.
• During budget-free rack-aware fallback, a replica may reuse the primary worker's StorageInfo,
  which can produce an incorrect mount-point hint and slot accounting when workers have different
  mount points.
• During budget-free rack-aware fallback with local storage enabled, replica selection does not
  verify that the selected worker has a local disk, so allocation may be deferred to a worker that
  cannot satisfy the requested local placement.

Why are the changes needed?

SlotsAllocator previously mixed strategy-specific budget calculation with worker selection,
storage selection, rack awareness, interruption awareness, and fallback handling. Separating the
strategy boundary makes the allocation stages easier to follow and allows strategies to evolve
without duplicating the common allocation workflow.

Does this PR resolve a correctness bug?

• Yes

Does this PR introduce any user-facing change?

• Yes

How was this patch tested?

• build/sbt "celeborn-master/testOnly org.apache.celeborn.service.deploy.master.slotsalloc.*"

[zaynt4606]

Minor: The parameter name skipLocationsOnSameWorkerCheck is misleading. When true, the expression !(skipLocationsOnSameWorkerCheck && index == selectedPrimaryIndex) evaluates to index != selectedPrimaryIndex, which prevents same-worker placement. When false, the condition is always true, which allows it. So true actually enforces the check, not skips it — the opposite of what the name implies. Consider renaming to enforceDifferentWorker for clarity.

[zaynt4606]

LGTM

[Copilot]

Pull request overview

Refactors master-side slot allocation by introducing a SlotsAssignStrategy abstraction that isolates strategy-specific slot-budget computation (round-robin vs load-aware) from the shared allocation workflow (progressive allocation + best-effort fallbacks) now owned by slotsalloc.SlotsAllocator.

Changes:

• Replaces offerSlotsRoundRobin / offerSlotsLoadAware with a unified SlotsAllocator.offerSlots(..., SlotsAssignStrategy) entry point.
• Introduces SlotsAssignStrategy implementations (RoundRobinSlotsAssignStrategy, LoadAwareSlotsAssignStrategy) plus SlotsAssignStrategyFactory for config-based selection.
• Updates master/tests/bench/docs to the new org.apache.celeborn.service.deploy.master.slotsalloc package and API.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
master/src/main/scala/org/apache/celeborn/service/deploy/master/Master.scala	Switches master to use the new slotsalloc.SlotsAllocator with a strategy chosen by SlotsAssignStrategyFactory.
master/src/main/java/org/apache/celeborn/service/deploy/master/SlotsAllocator.java	Removes the old monolithic allocator implementation from the master package.
master/src/main/java/org/apache/celeborn/service/deploy/master/slotsalloc/SlotsAllocator.java	New shared allocation workflow with progressive fallbacks and strategy-provided budgets.
master/src/main/java/org/apache/celeborn/service/deploy/master/slotsalloc/SlotsAssignStrategy.java	Defines the strategy interface for computing per-disk budgets.
master/src/main/java/org/apache/celeborn/service/deploy/master/slotsalloc/SlotsAssignStrategyFactory.java	Creates the appropriate strategy from CelebornConf (load-aware vs round-robin).
master/src/main/java/org/apache/celeborn/service/deploy/master/slotsalloc/RoundRobinSlotsAssignStrategy.java	Moves round-robin budget computation behind the strategy interface.
master/src/main/java/org/apache/celeborn/service/deploy/master/slotsalloc/LoadAwareSlotsAssignStrategy.java	Moves load-aware budget computation behind the strategy interface.
master/src/main/java/org/apache/celeborn/service/deploy/master/slotsalloc/UsableDiskInfo.java	Extracts UsableDiskInfo as a top-level helper in the new package.
master/src/test/java/org/apache/celeborn/service/deploy/master/slotsalloc/SlotsAllocatorSuiteJ.java	Updates tests to use offerSlots(..., strategy) and the new package.
master/src/test/java/org/apache/celeborn/service/deploy/master/slotsalloc/SlotsAllocatorRackAwareSuiteJ.java	Updates rack-aware tests to use the new API and package.
master/src/test/java/org/apache/celeborn/service/deploy/master/slotsalloc/BuildStorageInfoSuiteJ.java	Updates build-storage-info tests for the new buildStorageInfo signature and types.
master/src/test/java/org/apache/celeborn/service/deploy/master/slotsalloc/SlotsAllocatorJmhBenchmark.java	Updates benchmark to use the new API/package and a shared strategy instance.
docs/developers/sbt.md	Updates testOnly examples to the new test package path.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.