Github user sarutak commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2019#discussion_r16631095
  
    --- Diff: core/src/main/scala/org/apache/spark/network/Connection.scala ---
    @@ -263,14 +282,20 @@ class SendingConnection(val address: 
InetSocketAddress, selector_ : Selector,
     
       val DEFAULT_INTEREST = SelectionKey.OP_READ
     
    +  var alreadyReading = false
    +
       override def registerInterest() {
         // Registering read too - does not really help in most cases, but for 
some
         // it does - so let us keep it for now.
    -    changeConnectionKeyInterest(SelectionKey.OP_WRITE | DEFAULT_INTEREST)
    +    changeConnectionKeyInterest(
    +      SelectionKey.OP_WRITE | (if (!alreadyReading) {
    +        alreadyReading = true
    +        DEFAULT_INTEREST
    +      } else { 0 }))
    --- End diff --
    
    I understand registering DEFAULT_INTEREST (OP_READ) is to detect closing 
connection by remote host.
    But, once blocked by channel.read() in SendingConnection#read, 
DEFAULT_INTEREST is not needed.
    
    In addition, because SendingConnection is never unregistered OP_READ, 2 
threads on the same SendingConnection should be active and during one of the 
thread cancels its key, another thread can evaluate key.isValid in 
ConnectionManager#run.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

Reply via email to