[GitHub] LucaCanali commented on a change in pull request #23727: [SPARK-26817][CORE] Use System.nanoTime to measure time intervals

[GitBox <gi...@apache.org>]

LucaCanali commented on a change in pull request #23727: [SPARK-26817][CORE] Use System.nanoTime to measure time intervals
URL: https://github.com/apache/spark/pull/23727#discussion_r253375376
 
 

 ##########
 File path: core/src/main/scala/org/apache/spark/MapOutputTracker.scala
 ##########
 @@ -744,7 +744,7 @@ private[spark] class MapOutputTrackerWorker(conf: SparkConf) extends MapOutputTr
         }
       }
       logDebug(s"Fetching map output statuses for shuffle $shuffleId took " +
-        s"${System.currentTimeMillis - startTime} ms")
+        s"${System.nanoTime() - startTime} ns")
 
 Review comment:
   Nice change + this could be a good occasion to standardize the units used to report executor task metrics (currently some of them are reported in ms and some in ns), see SPARK-25170. 
   On the downside, this change will force updates on monitoring tools feeding on executor metrics values, but I guess it makes sense for a major release like 3.0. 
   For the case of executor task metrics units, using milleseconds seems too coarse to me, so I'd rather stick to nanoseconds (or microseconds would also do).
   I would also suggest to keep the doc (monitoring.md) in sync with the metrics units updates in this PR.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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