SteNicholas commented on PR #3720:
URL: https://github.com/apache/celeborn/pull/3720#issuecomment-4670257526
Heads-up @s0nskar: `b7e086b` ("Apply suggestions from code review")
accidentally broke the new test — applying the comment reword also deleted the
adjacent stub line:
```scala
Mockito.when(memoryManager.getMemoryUsage).thenReturn(pushThreshold + 1)
```
Diff of `MemoryManagerSuite.scala` between `20b5659` and `b7e086b`:
```diff
- // back-pressured. Before Fix 1 this incorrectly returned false.
- Mockito.when(memoryManager.getMemoryUsage).thenReturn(pushThreshold + 1)
+ // back-pressured (previously sorting was also blocked in this state).
```
That line is what drives the transition into `PUSH_PAUSED`. With
`getMemoryUsage` still stubbed to `0L`, `currentServingState()` can only return
`NONE_PAUSED` (usage exceeds neither pause threshold and is below the resume
ratio), so the next assertion `assert(memoryManager.servingState ==
ServingState.PUSH_PAUSED)` in "sortMemoryReady allows sorting in PUSH_PAUSED
but blocks in PUSH_AND_REPLICATE_PAUSED" cannot pass — the currently-pending CI
run will fail on it. `pushThreshold` is also left as an unused val.
Restoring the stub line under the new comment fixes it. The production
change itself is unaffected — my approval from the earlier review stands.
--
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]
