Github user mridulm commented on the pull request:
https://github.com/apache/spark/pull/2078#issuecomment-53155129
Looking at the code :
'''
val remoteAddress =
key.channel.asInstanceOf[SocketChannel].socket.getRemoteSocketAddress
'''
I agree, key.channel is document to always return non null value;
SocketChannel.socket is implemented to create new socket and return that in
case socket happens to be null.
And so looks like NPE is not possible.
Returned socket address can be null and making log message useless; but
atleast thankfully we wont barf out with an exception !
Given this is for logging, make it a addressInfo String (in a method
possibly since this is used in multiple places) and populate it with:
a) getRemoteSocketAddress (null should be fine I guess).
b) key.toString (would include key identifier to help debug lifecycle of a
selection key).
Possibly other info which might be relevant ... and use this instead of
remoteAddress in the code.
Given that spark logs sometimes go upto 60+ GB for some of our jobs, I can
see the value in having the socket addresses along with other state transition
information while debugging a problem (instead of having to grep to find this
info !)
---
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
