Fix network bridge local-side close - #2475
Conversation
cshannon
left a comment
There was a problem hiding this comment.
@mattrpav - Ok, I played around with this a bunch and I think it will work after all because of the disposed flag. That flag gets checked before anything happens in both serviceLocalCommand and serviceRemoteCommand. The disposed flag gets set to true when a bridge is stopped on purpose.
My main concern here was making sure that if the ShutDownInfo command is received that we only process and trigger the restart if there was an unexpected close. On a normal bridge shtudown the command won't get processed because disposed is set to true so all of that is skipped.
Also, you pointed out that the serviceRemoteCommand does the same thing on ShutdownInfo by calling the exception handler which triggers the bridge failed and reconnect as well so that further verifies this should be ok.
I made one comment inline that I think the test should simulate how the connection is stopped by the slow consumer strategy with an exception here
Also it would be good to expand on the tests:
- I would make sure duplex bridges are also handled correctly with reconnect
- You could try and test that the bridge doesn't reconnect if you stopped and and connection.serviceException() is called (this might already be tested by other bridge tests not sure)
| var bridgeLocalConnection = findVmConnection(localBroker); | ||
| assertNotNull("expected the bridge's local vm:// connection on the local broker", bridgeLocalConnection); | ||
| LOG.info("stopping the bridge's local connection server-side: {}", bridgeLocalConnection); | ||
| bridgeLocalConnection.stop(); |
There was a problem hiding this comment.
This should probably simulate passing an IOException just like the slow consumer strategy does
Currently, a local-side connection close is treated as the same as a broker shutdown. The bridge is stopped, but not removed from activeBridges or restarted.
This fix differentiates between an administrative close (inactivity monitor, connection.close(), etc.) to ensure the bridge is closed, cleared and recreated.