deepstreamIO / deepstream.io

deepstream.io server
https://deepstreamio.github.io
MIT License
7.14k stars 381 forks source link

Stopping of the server hangs #1037

Closed whitelizard closed 4 years ago

whitelizard commented 4 years ago

When trying to stop a Deepstream server it does not release/finish the node process, but hangs instead. The following example never exits.

const { Deepstream } = require('@deepstream/server');

const run = async () => {
  const server = new Deepstream('./emptyConf.yml');
  server.start();
  await new Promise(r => setTimeout(r, 5000));
  server.stop();
};

run();
yasserf commented 4 years ago

Not sure what is hanging since the state correctly goes to closed

INFO | State transition (stop): RUNNING -> CONNECTION_ENDPOINT_SHUTDOWN
INFO | State transition (connection-endpoints-closed): CONNECTION_ENDPOINT_SHUTDOWN -> PLUGIN_SHUTDOWN
INFO | State transition (plugins-closed): PLUGIN_SHUTDOWN -> HANDLER_SHUTDOWN
INFO | State transition (handlers-closed): HANDLER_SHUTDOWN -> SERVICE_SHUTDOWN
CLUSTER_LEAVE | k6cxesf1-qz7wohlkq7k0
INFO | State transition (services-closed): SERVICE_SHUTDOWN -> LOGGER_SHUTDOWN
INFO | State transition (logger-closed): LOGGER_SHUTDOWN -> STOPPED
whitelizard commented 4 years ago

So, to clarify with an explicit example:

Save this as test.js:

const { Deepstream } = require('@deepstream/server');

const run = async () => {
  const server = new Deepstream({});
  server.start();
  await new Promise(r => setTimeout(r, 2000));
  server.stop();
};

run();

Then run this command: node test.js && echo 'previous process exited'

Expected output is to see "[...] previous process exited'", and then get back the terminal prompt. Actual output is "[...] CLUSTER_LEAVE | k6hrhfi2-st7ebknc0o00" and no terminal prompt.

whitelizard commented 4 years ago

Perhaps it is also related, that it is impossible to start a stopped server.

An example would be the following:

const { Deepstream } = require('@deepstream/server');

const run = async () => {
  const server = new Deepstream({});
  server.start();
  await new Promise(r => setTimeout(r, 2000));
  server.stop();
  await new Promise(r => setTimeout(r, 2000));
  server.start();
  await new Promise(r => setTimeout(r, 2000));
  server.stop();
};

run();

Which only starts once, errors on second stop ("The server is already stopped."), and never exits the process.

yasserf commented 4 years ago

Yeah we don’t support cleaning up the server resources directly. This decision was because it got a bit buggy reusing the same state. You need to instead just create a new one each time.

We usually exit by doing a

Server.on(‘stop’, process.exit) but i can see how that’s not always wanted. Need to find out what is hanging though which is a little time consuming to do

On Wed, 12 Feb 2020 at 01:56, Esbjörn Blomquist notifications@github.com wrote:

Perhaps it is also related, that it is impossible to start a stopped server.

An example would be the following:

const { Deepstream } = require('@deepstream/server');

const run = async () => { const server = new Deepstream({}); server.start(); await new Promise(r => setTimeout(r, 2000)); server.stop(); await new Promise(r => setTimeout(r, 2000)); server.start(); await new Promise(r => setTimeout(r, 2000)); server.stop(); };

run();

Which only starts once, errors on second stop ("The server is already stopped."), and never exits the process.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/deepstreamIO/deepstream.io/issues/1037?email_source=notifications&email_token=AAU3WS7W7JU7TLQ6EYMJ6DLRCKN7BA5CNFSM4KELD4LKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELMJ3CY#issuecomment-584621451, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAU3WS4WEJIDRKMHB7FA6OTRCKN7BANCNFSM4KELD4LA .

jaime-ez commented 4 years ago

Maybe it's more of a documentation problem perhaps?

On the client repo we discussed a related issue, once a client is closed it can not be reconnected, and yasser suggested using the singleton pattern for handling reconnection scenarios after a client is closed.

In the documentation it should be clarified that if the server is closed it can not be re-started, and stopping the server does not imply exiting the node process. Which I think are the correct behaviours.

I'll try to make a pull request to the docs, for both client and server, explicitely stating this behaviour and suggest using the singleton pattern if required and the Server.on(‘stop’, process.exit) solution. Let me know what you think.

yasserf commented 4 years ago

Closing since your comment makes perfect sense.