Copilot commented on code in PR #3720:
URL: https://github.com/apache/celeborn/pull/3720#discussion_r3371125402
##########
worker/src/main/java/org/apache/celeborn/service/deploy/worker/memory/MemoryManager.java:
##########
@@ -435,7 +435,8 @@ public void reserveSortMemory(long fileLen) {
public boolean sortMemoryReady() {
return maxSortMemory == 0
- || (servingState == ServingState.NONE_PAUSED &&
sortMemoryCounter.get() < maxSortMemory);
+ || (servingState != ServingState.PUSH_AND_REPLICATE_PAUSED
+ && sortMemoryCounter.get() < maxSortMemory);
}
Review Comment:
The new behavior in sortMemoryReady() is user-facing (sorting is now allowed
while PUSH_PAUSED) but there isn’t any unit test coverage verifying the
state-dependent gating. Consider extending the existing MemoryManagerSuite to
assert: (1) sorting is allowed when servingState=PUSH_PAUSED and
sortMemoryCounter < maxSortMemory, and (2) sorting is blocked when
servingState=PUSH_AND_REPLICATE_PAUSED (when maxSortMemory > 0).
--
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]
