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

    https://github.com/apache/spark/pull/4157#discussion_r23365112
  
    --- Diff: 
core/src/main/scala/org/apache/spark/network/nio/ConnectionManager.scala ---
    @@ -375,16 +375,22 @@ private[nio] class ConnectionManager(
                     }
                   }
                 } else {
    -              logInfo("Key not valid ? " + key)
    +              logInfo("Key not valid ? " + key + " remote address: " + 
    +                  key.channel().asInstanceOf[SocketChannel].socket
    --- End diff --
    
    My first reaction was that this risks an exception just for a log message. 
Since this work is repeated several times, how about creating a method that 
will return the remote address in case of a `SocketChannel` and the default 
`toString()` otherwise? although given the current code, it will always be a 
`SocketChannel`.
    
    Can you use string interpolation here?
    
    Finally, the second cast isn't needed is it? it does not change the 
`toString()` that is called.


---
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 [email protected] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to