search

wankdanker / node-discover

Automatic and decentralized discovery and monitoring of nodejs instances with built in support for a variable number of master processes, service advertising and channel messaging.
229 stars 59 forks source link

New Leadership module introduces undesirable master flopping #36

Closed wankdanker closed 4 years ago

wankdanker commented 4 years ago

If you run node examples/master.js in three separate terminals, you'll see that the last node added becomes a master and then gets demoted shortly after.

This happens because the BasicLeadershipElection.prototype.check is being called on each BasicLeadershipElection.prototype.helloReceived. So, if the first packet the newest node sees is from not a master it will promote itself. There should be a timeout so that the new node can have some time to learn about all the nodes in the network before determining if it should be promoted.

related: #34, #35

aikar commented 4 years ago

I can see the race condition here, but I'm questioning why wouldn't that have still been a problem with previous code. I thought I kept the overall flow the same?

I guess the leadership election shouldn't start until checkInterval + 1000(leadershipDelay) has passed.

wankdanker commented 4 years ago

I can see the race condition here, but I'm questioning why wouldn't that have still been a problem with previous code. I thought I kept the overall flow the same?

All of the logic for checking leadership was in the check() function. That function was only called by setInterval()every checkInterval ms. That changes that were introduced called the check function on other occasions besides the interval (like helloReceived).

Anyway, node-discover@1.1.2 is on npm which fixes this.

Thanks for your help!