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



##########
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:
       Now I am starting to get this part. Can we try to simplify this? :)
   
   On a decommissioned executor there will be no new tasks started, right? 
(Theoretically one task could be started as the scheduler does not processed 
the `DecommissionExecutor` message but let's take this corner case out as we 
can have this first 1 sec or a bit more sleep which more or less handles this).
   
   So when `executor.numRunningTasks` will be 0 it stays 0. As I remember 
caching is part of task running so no new cached blocks will be created when 
numRunningTasks=0.
   
   So would it work if we first wait to reach numRunningTasks==0 in a sleep 
check loop and then we would check for the migration finished flag without 
using the time part of `lastMigrationInfo`?
   
   




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