mjwesterhof
commented
7 years ago The pulse node server does nothing that would help for this. It's an interesting idea, though. Here's some thoughts on this...
I can see one huge argument against this, first off -- that's the "Purist's Argument": this is a feature that would only benefit "buggy code" and the correct solution is for each node server to fix its own code. The counter argument to that is that in the real world, stuff happens. And more importantly, stuff can happen in dependencies that the node server author has no ability to fix or control.
Secondly, one might argue that there's already such a feature: the heartbeat (ping/pong messages). I'd counter that by noting that the heartbeat as implemented is insufficient in many cases.
Assuming that one agrees we should add code to do this, and that the existing heartbeat mechanism is inadequate, the next issue is what we need to do instead of or in addition to the heartbeat? Clearly we need to handle the exiting of the node server (the heartbeat mechanism handles only a hanging node server). That would imply we need to add another mechanism, though, to signal a legitimate exit -- so another message to the node server manager to say "I'm exiting for all the right reasons, please don't restart me", and another that says "I'm exiting to restart myself, please do so but don't log this as a failure". And if we don't get one of the aforementioned messages before the node server process exits, then polyglot would log the message and restart it.
So, now we'd need limits on the restart. It seems reasonable to only restart a certain number of times (call it 3 for argument's sake). However, since a node server is an "always running" server, and the node server manager may run for months (as long as the host OS isn't Windows, of course :-) ), we probably need a time element on that. So perhaps we say 3 times in the past hour.
The above might be sufficient - but we also might find it useful to add logic to handle badly-behaved node servers that are struggling but actually doing something... this is complex. And I'm not sure I support this, because of that. But conceptually, the node server manager would keep track of the inbound and outbound messages to the ISY (which is all it can see), and using some sort of heuristic based on that, it might keep restarting a node server that fails more often than the aforementioned timeout mechanism allows. The problem is selecting that heuristic.
Footnote: I fear this. Since I've started working on and with Polyglot, I've added more and more features and code and special cases. All of it, in isolation, seemed like a good idea at the time. As I reflect, I feel that Polyglot has become too big and too unwieldy, and I fear I've just added bloat to it with some of the code. And an auto-restart seems like a good idea, but again, it just makes it more complicated and bigger... I'm feeling that we're heading in the wrong direction. (This is not to say that I've a better idea - just to share that I'm not sure going deeper down this path is the right way!)
mkohanim
Einstein42
jimboca
rmkraus
I've seen a few random crashes in various nodeservers that should be automatically restarted. Now the problem is we don't want to restart it so many times it floods everything on a legitimate error, so we need some sort of mechanism to do so. @mjwesterhof does the Pulse server already do this? I need to get some kind of information from you on how that mechanism works so I can document it.