Zero Downtime reload config capability #55
Conversation
vk4u
commented
Oct 6, 2016
vk4u
commented
- Introducing ability to reload microgateway for new config without dropping messages.
- Reloading is done using IPC through sockets.
- After this change, microgateway will always be starting in cluster mode.
- Owing to this there are 3 new commands added. Status, Reload, Stop.
…dropping messages. 2. Reloading is done using IPC through sockets. 3. After this change, microgateway will always be starting in cluster mode. 4. Owing to this there are 3 new commands added. Status, Reload, Stop.
| .option('-e, --env <env>', 'the environment') | ||
| .option('-k, --key <key>', 'key for authenticating with Edge') | ||
| .option('-s, --secret <secret>', 'secret for authenticating with Edge') | ||
| .description('reoad the edgemicro cluster by pulling new configuration') |
|
Aside from the typo this looks ok to me. 👍 |
| } | ||
| } | ||
| }); | ||
| run.start(options); |
There was a problem hiding this comment.
Are we removing the callback because the success/error messages are logged in the start-agent?
There was a problem hiding this comment.
Yes. That is true. Moreover, the callback isn't available anymore. Because the we are passing the start file to cluster.setupMaster now.
| opt.args = [JSON.stringify(args)]; | ||
| opt.timeout = 10; | ||
|
|
||
| var cluster = reloadCluster(path.join(__dirname, 'start-agent.js'), opt); |
There was a problem hiding this comment.
I think this should be declared above line 66 so that the cluster reference is obvious. Also, please rename cluster to something else (perhaps mgCluster) so that it is not confused with the Node cluster module. When I see a variable named cluster in Node I think var cluster = require('cluster');
There was a problem hiding this comment.
Sure. Made the changes.
| edgeconfig.save(config, cache); | ||
| } | ||
|
|
||
| gateway(config); |
There was a problem hiding this comment.
I believe this does nothing, please delete line 59
There was a problem hiding this comment.
Interesting. Ill remove it. But just curious to know what it was doing earlier ?
There was a problem hiding this comment.
great question, no idea, it's been there since this odd commit
d065991
| for (var i = 0; i < numWorkers; i++) { | ||
| cluster.fork(); | ||
| } | ||
| gateway(config); |
There was a problem hiding this comment.
same as above, please remove line 136 (even though it was already present, may as well take the chance to clean it up)
2. Credits for code inspiration added
…eload incorporated.
| @@ -107,6 +106,40 @@ Gateway.prototype.start = (options) => { | |||
| process.exit(0); | |||
| }); | |||
|
|
|||
| var pollInterval = config.edgemicro.config_change_poll_interval ? config.edgemicro.config_change_poll_interval : defaultPollInterval; | |||
There was a problem hiding this comment.
var pollInterval = config.edgemicro.config_change_poll_interval || defaultPollInterval
| function hasConfigChanged(oldConfig, newConfig) { | ||
| // This may not be the best way to do the check. But it works for now. | ||
| if (JSON.stringify(oldConfig) == JSON.stringify(newConfig)) { |
There was a problem hiding this comment.
return JSON.stringify(oldConfig) != JSON.stringify(newConfig)
…as used for testing purposes)
