advancedxy commented on code in PR #510:
URL: https://github.com/apache/incubator-uniffle/pull/510#discussion_r1089869885
##########
server/src/main/java/org/apache/uniffle/server/ShuffleFlushManager.java:
##########
@@ -95,6 +96,10 @@ public ShuffleFlushManager(ShuffleServerConf
shuffleServerConf, String shuffleSe
while (true) {
try {
ShuffleDataFlushEvent event = flushQueue.take();
+ // Only for test
+ while (isSuspend) {
Review Comment:
I did a quick overview of related code, and there might be two proposals:
1. modify unit test to use Hamcrest match API to match multiple possible
values, since the first block could already be flushed, we can consider that in
the unit test
2. refactor `ShuffleFlushManager` processEventRunnable construct, extract
related logic into one method, for example `processEvent`.
In `ShuffleBufferManagerTest#shuffleFlushThreshold`, a new
TestShuffleFlushManager should be created and overrides the `processEvent` to
inject test related logic.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]