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:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
