Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix graceful shutdown #1102

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix graceful shutdown #1102

wants to merge 1 commit into from

Conversation

timja
Copy link
Contributor

@timja timja commented Sep 10, 2024

Jira link

Follow up to a bug introduced in https://tools.hmcts.net/jira/browse/DTSPO-17277

Change description

Previously this was completely broken, it only worked on the https server which is only used locally.
It also only stopped new requests after 4 seconds and then began closing them.

Shuts the server down so that new connections are no longer accepted by the server.
A hard shutdown of the server happens at 10 seconds now.

Testing done

I implemented a route with a delay of 10 seconds and then 20 seconds before it responds.

+  app.get('/random10', (req, res) => {
+    setTimeout(() => {
+    res.render('home');
+  }, 10000)
+  });
+  app.get('/random20', (req, res) => {
+    setTimeout(() => {
+    res.render('home');
+  }, 20000)
+  });

The 10 second route shutdown cleanly with a 10 second delay roughly:

^C2024-09-10T14:53:35+01:00 - info: [server] ⚠️ Caught SIGINT, gracefully shutting down
2024-09-10T14:53:44+01:00 - info: [server] Connections closed, exiting

The 20 second route was forcefully shutdown:

^C2024-09-10T14:53:58+01:00 - info: [server] ⚠️ Caught SIGINT, gracefully shutting down
2024-09-10T14:54:08+01:00 - info: [server] Forcesfully shutting down application

With no open connections it shuts down immediately:

^C2024-09-10T15:03:52+01:00 - info: [server] ⚠️ Caught SIGINT, gracefully shutting down
2024-09-10T15:03:52+01:00 - info: [server] Connections closed, exiting

I reverted the healthcheck changes as when no more connections are allowed it never responded anyway:

curl localhost:3100/health/readiness
curl: (7) Failed to connect to localhost port 3100 after 0 ms: Couldn't connect to server

Checklist

  • commit messages are meaningful and follow good commit message guidelines
  • README and other documentation has been updated / added (if needed)
  • tests have been updated / new tests has been added (if needed)
  • Does this PR introduce a breaking change

@timja timja requested review from a team as code owners September 10, 2024 14:06
@timja timja requested review from DanielKChristou, NickAzureDevops and Tyler-35 and removed request for a team September 10, 2024 14:06
@timja timja force-pushed the fix-graceful-shutdown branch from 8d9768f to ba0e525 Compare September 10, 2024 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants