spark-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From squito <...@git.apache.org>
Subject [GitHub] spark pull request #19583: [WIP][SPARK-22339] [CORE] [NETWORK-SHUFFLE] Push ...
Date Fri, 27 Oct 2017 22:18:07 GMT
Github user squito commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19583#discussion_r147526776
  
    --- 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))
{
    --- End diff --
    
    nit: a while back I think the community decided that `Option.fold` is pretty confusing
for folks that aren't used to scala, with little gain, so I'd get rid of it
    (though given my other comment about the unnecessary class, you can probably just remove
this whole thing.)


---

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


Mime
View raw message