Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19583#discussion_r148917664
  
    --- Diff: core/src/main/scala/org/apache/spark/HeartbeatReceiver.scala ---
    @@ -51,7 +51,26 @@ private case class ExecutorRegistered(executorId: String)
     
     private case class ExecutorRemoved(executorId: String)
     
    -private[spark] case class HeartbeatResponse(reregisterBlockManager: 
Boolean)
    +private[spark] case class UpdatedEpoch(epoch: Long)
    +
    +private[spark] object HeartbeatResponse {
    +  def apply(reregisterBlockManager: Boolean,
    +            updatedEpoch: Option[Long] = None): HeartbeatResponse =
    +    
updatedEpoch.fold[HeartbeatResponse](BasicHeartbeatResponse(reregisterBlockManager))
 {
    +      epoch => HeartbeatResponseWithEpoch(reregisterBlockManager, 
Some(epoch))
    +    }
    +}
    +
    +private[spark] sealed trait HeartbeatResponse {
    +  def reregisterBlockManager: Boolean
    +  def updatedEpoch: Option[Long] = None
    +}
    +
    +private[spark] case class BasicHeartbeatResponse(reregisterBlockManager: 
Boolean)
    +  extends HeartbeatResponse
    +private[spark] case class 
HeartbeatResponseWithEpoch(reregisterBlockManager: Boolean,
    +                                                     override val 
updatedEpoch: Option[Long])
    +  extends HeartbeatResponse
    --- End diff --
    
    seems not a big saving, I think we can even always include the epoch in the 
heartbeat response.


---

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

Reply via email to