Skip to content

[CELEBORN-2423] Fix rack-aware replica storage selection - #3803

Open
Kalvin2077 wants to merge 1 commit into
apache:mainfrom
Kalvin2077:fix/CELEBORN-2423
Open

[CELEBORN-2423] Fix rack-aware replica storage selection#3803
Kalvin2077 wants to merge 1 commit into
apache:mainfrom
Kalvin2077:fix/CELEBORN-2423

Conversation

@Kalvin2077

@Kalvin2077 Kalvin2077 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This follow-up to PR #3781:

  • Validates that rack-aware replica candidates support the requested storage type.
  • Builds replica StorageInfo from the selected replica worker.
  • Adds regression tests for diskless replicas and different mount points.

Why are the changes needed?

During best-effort fallback, the allocator could select a diskless worker for a local-disk replica or incorrectly reuse the primary worker's mount point.

Does this PR resolve a correctness bug?

  • Yes

Does this PR introduce any user-facing change?

  • Yes

How was this patch tested?

build/mvn -pl master -am -Dtest=SlotsAllocatorRackAwareSuiteJ test

@Kalvin2077 Kalvin2077 changed the title [CELEBORN-2423] Refactor slot assignment strategy and fix storage selection [CELEBORN-2423] Fix rack-aware replica storage selection Aug 13, 2026
@Kalvin2077
Kalvin2077 marked this pull request as ready for review August 21, 2026 08:10
@github-actions github-actions Bot added the correctness Correctness bugfix label Aug 21, 2026
@Kalvin2077

Copy link
Copy Markdown
Contributor Author

@zaynt4606 @SteNicholas
PTAL, thx.

@yew1eb

yew1eb commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

@Kalvin2077 LGTM overall. One nit on test coverage: offerSlotsRackAwareFallbackRequiresReplicaDisk only covers the case where no eligible replica exists. Could we add a case where the replica rack has both a diskless and a local-disk worker, and assert the allocator skips the diskless on and still allocates? That's the path the new canAssign predicate actually guards — currently it would regress unnoticed.

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.

2 participants