attilapiros commented on a change in pull request #29211:
URL: https://github.com/apache/spark/pull/29211#discussion_r460263043



##########
File path: 
core/src/main/scala/org/apache/spark/storage/BlockManagerDecommissioner.scala
##########
@@ -327,4 +354,28 @@ private[storage] class BlockManagerDecommissioner(
     }
     logInfo("Stopped storage decommissioner")
   }
+
+  /*
+   *  Returns the last migration time and a boolean for if all blocks have 
been migrated.
+   *  If there are any tasks running since that time the boolean may be 
incorrect.
+   */
+  private[storage] def lastMigrationInfo(): (Long, Boolean) = {
+    if (stopped || (stoppedRDD && stoppedShuffle)) {
+      (System.nanoTime(), true)
+    } else {
+      // Chose the min of the running times.
+      val lastMigrationTime = if (
+        conf.get(config.STORAGE_DECOMMISSION_SHUFFLE_BLOCKS_ENABLED) &&
+        conf.get(config.STORAGE_DECOMMISSION_RDD_BLOCKS_ENABLED)) {
+        Math.min(lastRDDMigrationTime, lastShuffleMigrationTime)

Review comment:
       This must be `Math.max`.
   
   As I understand in `CoarseGrainedExecutorBackend` the `lastTaskRunningTime` 
increases with about 1000ms in every iteration, let's assume exactly 1000ms. So 
if the RDD migration finished at 500ms (let's count in the example from 0 here) 
but shuffle files to be migrated are still left and they will be finished only 
in the next round (let's assume in 1500ms) then we we never shutdown the 
executor: as in the current round `blocksMigrated` is false and in all the 
following ones `migrationTime` will be less than `lastTaskRunningTime`, so this 
condition will be never satisfied:
   
   
https://github.com/apache/spark/blob/484f8e216d5727e516488aedbdb41b1f63569701/core/src/main/scala/org/apache/spark/executor/CoarseGrainedExecutorBackend.scala#L305
   
   




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