[GitHub] [spark] zsxwing commented on a change in pull request #24140: [SPARK-27198][core] Heartbeat interval mismatch in driver and executor

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

zsxwing commented on a change in pull request #24140: [SPARK-27198][core] Heartbeat interval mismatch in driver and executor
URL: https://github.com/apache/spark/pull/24140#discussion_r271055624
 
 

 ##########
 File path: core/src/main/scala/org/apache/spark/executor/Executor.scala
 ##########
 @@ -831,9 +832,11 @@ private[spark] class Executor(
     }
 
     val message = Heartbeat(executorId, accumUpdates.toArray, env.blockManager.blockManagerId)
+    val heartbeatIntervalInSec =
+      conf.getTimeAsMs("spark.executor.heartbeatInterval", "10s").millis.toSeconds.seconds
     try {
       val response = heartbeatReceiverRef.askSync[HeartbeatResponse](
-          message, RpcTimeout(conf, "spark.executor.heartbeatInterval", "10s"))
+          message, new RpcTimeout(heartbeatIntervalInSec, "spark.executor.heartbeatInterval"))
 
 Review comment:
   Right now the default time unit in master is `ms`, which is the same after this fix.
   
   Before this fix, when a time unit is not provided, for example, using 1000, the behavior is sending the heartbeat every 1000ms and the timeout of sending the heartbeat message is 1000s (which I think is a bug introduced in #10365).
   
   I'm +1 for this fix since it has the same behavior as the master branch.
   
   However, I suggest to apply the same changes related to `spark.executor.heartbeatInterval` from https://github.com/apache/spark/commit/9362c5cc273fdd09f9b3b512e2f6b64bcefc25ab#diff-6bdad48cfc34314e89599655442ff210 rather than this patch to make all places consistent. @ajithme could you submit a follow PR to make the change?

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