Kalvin2077 commented on PR #3803: URL: https://github.com/apache/celeborn/pull/3803#issuecomment-5390304298
> @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. @yew1eb Thx. I've added offerSlotsRackAwareFallbackSkipsDisklessReplica to cover this path. It uses two partitions so the diskless candidate is scanned regardless of the randomized initial index, and verifies both replicas are allocated to the eligible disk-backed worker. Removing the canAssign guard causes the test to fail. PTAL. -- 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]
