Node Weight not honoured on new node

[solgarius]

I also just noticed that the node weight is not being honoured when a new node is being added. It appears to always set the new node to master ahead of existing masters regardless of the weight of the node.

I am attempting to setup a system so that the older node will always be master e.g.:

var n1 = new Discover({
            weight: Date.now() * -1 
        });

I suggest either checking the weight when a new node is added, or perhaps there should be some sort of "shouldBeMaster()" function that can be used on both new node & in the checkId interval?

I only note this because even though I haven't tried it, it looks like if there should be a master count of n > 1, then it will only promote a node if it has the highest weight as opposed to being in the top n weights. I have not tested to see if this is the case though, just looks like it might be a bug.

[wankdanker]

Hey @solgarius,

I am not seeing the behavior you describe. Are you running each instance in a separate process?

I updated example/master.js to reflect what you are saying:

var Discover = require("../");

var d = new Discover({ key : process.argv[2], weight : Date.now() * -1, mastersRequired : 2 });

When I run it in four different consoles, the process which was started first stays as the master, even as other processes are started. There was an issue with processing masters if mastersRequired was greater than one. But that had to do with a node thinking that an active master was a valid higher weight, which it shouldn't. I fixed that in https://github.com/wankdanker/node-discover/commit/7d5b389dffd7ce44e3c774b16d10f1380b68db4f, and released v0.0.11.

Maybe I'm misunderstanding what you are saying in your initial question though...

I suggest either checking the weight when a new node is added, or perhaps there should be some sort of "shouldBeMaster()" function that can be used on both new node & in the checkId interval?

This sounds like you think that if a higher weighted node is added to the network then it should automatically become the master. If that is the case, then I disagree. I think that once a master is established, either automatically or by manual promotion, that it should stay the master until it is manually demoted or the process is ended.

If you want to make one process that has been added to the network most recently a master, then call promote() on it. Any node can become a master regardless of its weight manually by calling promote(). Every other node will see this and demote() themselves if they are currently a master. Then, if more than one mastersRequired, the proper node will promote itself based on weight.

Let me know if I'm misunderstanding anything.

Thanks,

Dan

[wankdanker]

By the way, I like your idea of { weight : -Date.now() }, I might make that the default, rather than Math.random(). Actually, I would probably do { weight : -(Date.now()/Math.pow(10,String(Date.now()).length)) } as the default. I made a conscious decision to try to keep the automatically assigned weights < 0. That way, if you manually set a weight >=1, then it would always be master.

[solgarius]

My intent all along was to have the process that became master first to be the master first (same as your reasoning above). In my work with the app that I am building (which is very big), I am experiencing the following scenario:

1. Start Process A
2. Process A becomes master
3. Start Process B (can be soon after process A or a while after process A)
4. Process B is master (possibly because it sets itself as master before discovering process A??)
5. Process A receives an added message from Process B and so it demotes itself becoming the slave.

I would expect that Process A remained master and Process B was not the master. The issue seems to be that step 4 occurs.

When I simplify the test down (after your results) to just using node-discover and nothing else, the behaviour is as you described, with process A never becoming master.

I cannot explain why I am experiencing different behaviour, just that perhaps there is some race condition somewhere as theorised above.

I assume you mean in your second comment about Date.now() that you wanted weights -1<x<1, as -Date.now() should be negative?

[wankdanker]

If you think that this may be happening because process B doesn't discover A before promoting itself, then you may want to increase the checkInterval to something greater than the default (which is 2000 ms).

var d = new Discover({ checkInterval : 10000 });

Thinking about your large application... I'm wondering how long the startup time of the application is. If you initialize Discover at the beginning of your application and then do some Sync config loads or heavy processing that blocks the event loop for more than 2000ms, then it could definitely be the case that it doesn't discover the master in time.

You are correct, I want the default automatically assigned weights to be -1 < x < 1. That way, users who use positive integers as their manually assigned weight will always be masters over any node that did not have a manual weight assigned.

[solgarius]

Thanks, the check interval setting resolves the issue. As you note, due to the large application and the synchronous startup process, it takes a few seconds to get itself stabilised.

As it requires a largish check interval, and we want to avoid downtime, I might instead load up the discover module, immediately stop it and then start it again once the system is through the initial startup phase.

I'll close the issue as no further changes required.