-
Notifications
You must be signed in to change notification settings - Fork 4.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Call RemoveServer for reap events #5317
Conversation
In both the leaving and reaping case the server is no longer available and therefore we must remove it from our list of servers.
Still need to do some more digging here. There might be a LAN router problem too. |
This ensures we add/remove updated/failed servers properly.
There definitely was a LAN router problem. In fact all 3 places that handled the serf events were wrong. The resulting behavior is that servers stuck around in our RPC routers even after they had been reaped by serf. This would persist until the node rejoined or the node was restarted. |
I dont think this will cause a problem to ignore after further investigation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
In both the leaving and reaping case the server is no longer available and therefore we must remove it from our list of servers.
If we do not remove servers for a reap event they may stay around forever until the node gets restarted.
Update: Also ensure that
EventMemberUpdate
events are handled just like joins to updated the server info.AddServers
internally handles update vs new addition.