[GitHub] [geode] mcmellawatt commented on pull request #2484: GEODE-5747: Handling SocketException in InternalDataSerializer

["mcmellawatt (GitHub)" <gi...@apache.org>]

We did consider a variation of what you are suggesting, but fundamentally we saw a SocketException as an indication that the connection to the client was in a bad state and figured we should treat it the same as an EOFException.

In the context of a gateway sender, the SerializationException that was wrapping the SocketException would cause all event processing to stop, which is undesirable.  There is handling for all IOExceptions (including SocketExceptions) in the gateway sender, so that it attempts to connect to a different locator instead of shutting down the event processor.  As you suggested, we could add handling at that part of the code to catch SerializationExceptions, check if the inner exception is a SocketException, and retry.  Do you prefer that options?  I assume you may prefer it because the scope of the change is minimized.  

Again, our thought for the change in this PR is that a SocketException is similar to the EOFException in that it indicates a bad connection, and callers have their own handling for these exception types (i.e. retrying in the gateway sender case).  We spent some time looking at various callers and didn't see any red flags for this change.  I could be convinced to instead make the change at the gateway sender logic if there is fear of undesired side effects though.  Let me know.

[ Full content available at: https://github.com/apache/geode/pull/2484 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org