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]

Reply via email to