Terminate the workers more gracefully

[Pchelolo]

Previously our shutdown process only closed the http server, and we never actually close anything else that might need to be closed within the server if the num_workers was > 0. The procedure described here would basically do the same thing for the HTTP servers as done right now by disconnect, but additionally it would provide us a way to hook into the disconnection process and close whatever the services requested.

We could've done that by adding a listener in the worker for the disconnect event, but, that's problematic because the service-provided closing handler should explicitly close the HTTP server for the num_workers === 0 case and then with num_workers > 0 case calling the service-provided close method fails because the server was already killed by the cluster module.

Also, this is more in line with what the official docs say: https://nodejs.org/dist/latest-v6.x/docs/api/cluster.html#cluster_worker_disconnect

Bug: https://phabricator.wikimedia.org/T158265
cc @wikimedia/services

[gwicke]

If we expect each service to implement a graceful shutdown independently, then we should probably start all those in parallel.

[Pchelolo]

Ye, that's a good idea, why not..

[Pchelolo]

@gwicke I'd like to put the PR on hold now. You're making a good point about shutting things down under the feet of running requests, lemme try to see if we can avoid that problem and do better.

[gwicke]

Looking at https://nodejs.org/api/http.html#http_server_close_callback, we might actually be fine if each service is careful to implement its own graceful shutdown, starting with a close() and followed by a timeout. That's not the case right now though, so we'd need to first implement that carefully in all services.

[Pchelolo]

@gwicke Another thing here is that we don't really call this all when the service is stopped in production, right now this is only called in tests. So it would be nice to make prod do this too.