fix: graceful shutdown - Githubissues

LePremierHomme commented 3 years ago

An attempt to solve https://github.com/ExchangeUnion/xud/issues/1668.

Various fixes were made to prevent delays in shutdown:

Revoke connection retries of closed peers.
When using Tor, first create a plain socket connection with the local service, before passing it to the SOCKS client. This enables us to keep track of the raw socket while SOCKS connection to destination is pending, and to close it on shutdown.
Keep track of the reachability verification dummy peer, so it can be closed on shutdown.
Keep track of ConnextClient's pending HTTP requests, and abort them on shutdown (besides the critical ones). A jest test was added, but it's using an HTTP client mock (which works according to nodejs documentation). This is not ideal, but I thought that using a real HTTP client/server for this test would be an overkill. This behaviour still need to be manually tested.

Pending swaps are still expected to (intentionally) cause delay in shutdown. There might be other issues which cause a delay which I haven't encountered. So any help in testing would be appriciated. Please paste your logs here (even if it works).

To launch via xud-docker:

$ bash ~/xud.sh -b fix_shutdown

kilrau commented 3 years ago

Fix is in, could you give this another look? @sangaman

rsercano commented 3 years ago

Another case (don't know if we want to stay it like this) is that somebody calling streamorders or new stream grpc call subscribe alerts (https://github.com/ExchangeUnion/xud/pull/1984) then unless stream is closed xud is not being closed.

Ignore if this's intentional.

kilrau commented 3 years ago

I think we should to close streams as part of the shutdown procedure. If that's not the case, want to take care of it as a separate PR? @rsercano

rsercano commented 3 years ago

I think we should to close streams as part of the shutdown procedure. If that's not the case, want to take care of it as a separate PR? @rsercano

Sure as long as @LePremierHomme doesn't want to address it, I can check too.

kilrau commented 3 years ago

Definitely not as part of this PR, so go ahead :+1:

rsercano commented 3 years ago

Definitely not as part of this PR, so go ahead 👍

After digging into this, I realized this has already been addressed for streamorders and my bad not to handle on subscribealerts by adding stream to current list, Im now adding it and should be handled on close.

ExchangeUnion / xud

fix: graceful shutdown #1994