Ngone51 commented on a change in pull request #31102:
URL: https://github.com/apache/spark/pull/31102#discussion_r555866261



##########
File path: 
core/src/main/scala/org/apache/spark/storage/BlockManagerDecommissioner.scala
##########
@@ -154,7 +158,7 @@ private[storage] class BlockManagerDecommissioner(
   // Shuffles which are queued for migration & number of retries so far.
   // Visible in storage for testing.
   private[storage] val shufflesToMigrate =
-    new java.util.concurrent.ConcurrentLinkedQueue[(ShuffleBlockInfo, Int)]()
+    new java.util.concurrent.LinkedBlockingQueue[(ShuffleBlockInfo, Int)]()

Review comment:
       Thanks for pointing this out.
   
   AFAIK, both `ConcurrentLinkedQueue` and `LinkedBlockingQueue` are guaranteed 
to ensure thread-safe for their operations. And `ConcurrentLinkedQueue` is 
based on CAS while `LinkedBlockingQueue` is based on `ReentrantLock`. So, 
`ConcurrentLinkedQueue` is generally faster for `LinkedBlockingQueue`.
   
   And in this specific case, using `ConcurrentLinkedQueue` requires us to 
manually maintain a periodic(per second) checking for the queue, which may 
occupy unnecessary CPU resources for a while when the queue is empty (e.g., 
after the first round migration)...but, it should not be big deal since the 
node is going to shut down...
   
   And for `LinkedBlockingQueue`, because of the blocking functionality, we 1) 
don't need the manual checking, which simplifies the logic 2) won't occupy CPU 
resources when the queue is empty
   
   And if we care more about the reading performance of the queue, I'm ok to 
revert this to use the `ConcurrentLinkedQueue`. WDYT?




----------------------------------------------------------------
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.

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