mridulm commented on a change in pull request #27583: [SPARK-29149][YARN]  
Update YARN cluster manager For Stage Level Scheduling
URL: https://github.com/apache/spark/pull/27583#discussion_r380132387
 
 

 ##########
 File path: 
resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala
 ##########
 @@ -142,22 +150,31 @@ private[yarn] class YarnAllocator(
   } else {
     0
   }
-  // Number of cores per executor.
-  protected val executorCores = sparkConf.get(EXECUTOR_CORES)
+  // Number of cores per executor for the default profile
+  protected val defaultExecutorCores = sparkConf.get(EXECUTOR_CORES)
 
 
 Review comment:
   I am wondering if we want to make the locking semantics more formal in this 
class.
   Earlier, it was volatiles and concurrent hashmap (or sets back by concurrent 
hashmap) to eliminate need for locking - but lot of state changes were in 
context of 'this' being synchronized.
   
   Do we want to make sure all changes are guarded by a lock now ? Either use 
'this' everywhere or some explicit private lock object and mark it via 
"\@GuardedBy"
   It is becoming slightly difficult too reason about the MT-safety of this 
class.
   Do you have any thoughts Tom ?

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to