You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2019/10/08 19:14:33 UTC

[GitHub] [spark] vanzin commented on a change in pull request #26058: [SPARK-10614][core] Add monotonic time to Clock interface.

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


With regards,
Apache Git Services

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