spark-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From viirya <...@git.apache.org>
Subject [GitHub] spark pull request #20179: [SPARK-22982] Remove unsafe asynchronous close() ...
Date Tue, 09 Jan 2018 08:58:13 GMT
Github user viirya commented on a diff in the pull request:

    https://github.com/apache/spark/pull/20179#discussion_r160347954
  
    --- Diff: core/src/main/scala/org/apache/spark/rpc/netty/NettyRpcEnv.scala ---
    @@ -376,18 +374,13 @@ private[netty] class NettyRpcEnv(
     
         def setError(e: Throwable): Unit = {
           error = e
    -      source.close()
         }
     
         override def read(dst: ByteBuffer): Int = {
           Try(source.read(dst)) match {
    +        case _ if error != null => throw error
    --- End diff --
    
    I think it is better to also add a short comment here. This bug is subtle and no test
against it now. Just from this code, it is hard to know why we check error even success.


---

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


Mime
View raw message