Open shangd opened 7 years ago
In general this looks good, I'm curious though what the use case for this is? Is there a reason you need the port to be sticky? I worry that strengthening the assumption that a port may not change (although not specifically set as so) could cause friction/confusion for new users.
@steveniemitz Thanks for the review, the reason we implemented this was that we wanted to minimize the need for clients to re-lookup the brokers from our service discovery + re-init the client after the brokers have been restarted, which may take a while in some cases. With sticky address the client can usually reconnect as soon as the broker is back up.
I think it's quite reasonable to the user that they can either force the port they want to use in the config or they can let the framework pick an arbitrary port which may or may not be the same. I know it is a bit inconsistent when the hostname stickiness is enforced and the port one is not, but we thought it was practically more convenient. I'll update the doc to explain the differences so people don't get confused.
I'll make the Option changes you suggested when I got some time (quite busy atm) and come back in a few days.
This change will make broker remember previous port and prefer to restart on same port.
It is slightly different to the current hostname stickiness in that it is soft-stickiness, if the port is not available then it simply chooses the first port offered (same as current behavior) rather than rejecting the offer. It does not use the stickiness period to avoid overloading the config with unnecessary complexity.
Basically the existing hostname stickiness and config will behave exactly the same, but it will always pick previous port if available before choosing other ports.