vanzin commented on a change in pull request #26058: [SPARK-10614][core] Add 
monotonic time to Clock interface.
URL: https://github.com/apache/spark/pull/26058#discussion_r332686468
 
 

 ##########
 File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
 ##########
 @@ -288,7 +288,7 @@ private[spark] class ExecutorAllocationManager(
     }
 
     // Update executor target number only after initializing flag is unset
-    updateAndSyncNumExecutorsTarget(clock.getTimeMillis())
+    updateAndSyncNumExecutorsTarget(clock.nanoTime())
 
 Review comment:
   Are you sure about that?
   
   This is what the javadocs say:
   
       The same origin is used by all invocations of this method in an instance 
of a Java virtual
       machine; other virtual machine instances are likely to use a different 
origin. 
   
   (Comment about "other virtual machines" aside.)
   
   That seems to be corroborated by other sources, too (e.g. 
https://stackoverflow.com/questions/510462/is-system-nanotime-completely-useless).
 Seems in a long gone past there might have been issues, but not today.

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to