search

hashicorp / memberlist

Golang package for gossip based membership and failure detection
Mozilla Public License 2.0
3.62k stars 435 forks source link

stateLeft #213

Closed hanshasselberg closed 4 years ago

hanshasselberg commented 4 years ago

This PR introduces another state: stateLeft. This is an attempt to help fix https://github.com/hashicorp/consul/issues/6897.

When memberlist processes a dead message, it will now check where it is coming from and if the reporting node is equal to the reported node, we know this node wants to leave. In this case we don't have to prevent new nodes from using the same ip.

If a node reports another node, the behavior stays the same.

Note: this PR adds a new state which makes this update backwards incompatible because consumer of this library need to change their code to handle stateLeft. While I think a new state is the most elegant way to solve this problem, we could also add a flag to nodeState and not change the states.

hanshasselberg commented 4 years ago

Thank you for the review @mkcp!

I don't have a very strong opinion on the naming. There is one reason, why I like stateLeft better: it makes it clear that the node left already, while stateLeave could also mean that the node is still leaving. I might totally be wrong though because english is not my native language. I know we have leaveIntent in serf eg and I didn't want to be ambiguous here.

hanshasselberg commented 4 years ago

Reportedly fixed the problem: https://github.com/hashicorp/consul/pull/7184#issuecomment-580488095.

hanshasselberg commented 4 years ago

Thanks for the review! I missed the private bit and thought that the notification would include the new state as well. If thats not the case, we should be good!