kit icon indicating copy to clipboard operation
kit copied to clipboard

feat: support systemd socket activation

Open elohmeier opened this issue 2 years ago • 2 comments

Adds support for systemd socket activation for the node adapter. Fixes #10134.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • [x] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • [x] This message body should clearly illustrate what problems it solves.
  • [ ] Ideally, include a test that fails without this PR but passes with it.

Tests

  • [x] Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • [x] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

elohmeier avatar Nov 04 '23 14:11 elohmeier

🦋 Changeset detected

Latest commit: 7ad973308402e8517796c7a3ecc23e8b9240d042

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/adapter-node Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Nov 04 '23 14:11 changeset-bot[bot]

Thank you. This PR is rather cryptic to me — things like const sd_listen_fds_start = 3 make me very nervous (what is 3?).

It really seems like this sort of thing belongs in a custom server rather than in the turnkey server you get OOTB. Is there any reason that wouldn't work?

Rich-Harris avatar Jan 09 '24 20:01 Rich-Harris

3 is what systemd says will be the file descriptor to listen on:

The file descriptors systemd passes to us are inherited one after the other beginning with fd #3. See the docs

I've been playing around with this myself and wanted to create a PR for this too. You could indeed do this with a custom server but the changes required to do this ootb are very minimal and other adapters try to take advantage of the platform being deployed to as much as possible, so why not adapter-node too? But I think the code should contain a few comments and links to the systemd documentation.

karimfromjordan avatar Jan 14 '24 01:01 karimfromjordan

I'm open to the idea that this PR is necessary given that systemd is used on most machines the adapter-node output will be deployed to, but can't really give an opinion on it yet because there are no docs explaining what this feature is or does. We'll need docs if we're going to merge this, so it would be really helpful if those can be added so that we can give this a real review

benmccann avatar Jan 14 '24 17:01 benmccann

I can help with that.

@elohmeier in your code it looks like you don't throw an error when more than one FD is passed to the app by systemd. Is there a reason for that? From what I understand you would only ever need one socket in a SvelteKit app. The official systemd examples handle this case and throw an error when parseInt(process.env.LISTEN_FDS) !== 1.

karimfromjordan avatar Jan 15 '24 00:01 karimfromjordan

I just realized that the code to actually shut down the server is not included so this PR as it is wouldn't fix the original issue. Maybe we could add a adapter({ timeout: 500 }) option to the adapter to handle this.

karimfromjordan avatar Jan 15 '24 04:01 karimfromjordan

Thank you. This PR is rather cryptic to me — things like const sd_listen_fds_start = 3 make me very nervous (what is 3?).

It really seems like this sort of thing belongs in a custom server rather than in the turnkey server you get OOTB. Is there any reason that wouldn't work?

It's 3 because each process starts with three FDs

0 = stdin 1 = stdout 2 = stderr

then it's 3 for the socket because it's the next available.

BTW the whole awesomeness around this socket activation system is that systemd is the one that binds the socket. While there is no access to it the application doesn't run. When something connects the socket systemd wakes up the server then handover the control of the socket. There are two ways of doing this. One way is systemd calls accept and the content of the connection is the stdin and stdout of the waken up service or (our case), systemd wakes up the service on demand, hands over the socket and for now on the control of the socket is done by the service process itself. I've done some testing in Python [1] actually to see how it's in practice.

BTW today I did a on demand system that wakes up a inference service to run a STT model. I don't want it everytime eating my VRAM so now I have this inference service running on demand and auto sleeping after one minute of inactivity and a Telegram bot that transcribes audio messages.

When binding the port, instead of creating a new socket the process basically only wraps the socket that is already opened on fd 3. NGL this stuff is kinda boring to test as one must create a service unit and a socket unit that links to the service unit.

[1] https://github.com/lucasew/nixcfg/blob/master/nix/nodes/riverwood/test_socket_activated/service.py

lucasew avatar Jan 15 '24 19:01 lucasew

I basically applied the changes of this PR using patch-package in one of my projects [1]. Stuff seems to be working fine. I consider my problem solved basically but the service will never finish on it's own to be retriggered on demand.

You guys are free to take this patch and apply on your own stuff. Just follow the setup instructions [2] and put this patch in the patches folder. The patch will be applied automatically when doing a npm install.

[1] https://github.com/lucasew/cf-torrent/blob/dc1482d04507bbe585a6b0a2d774adfc4e5ed68a/patches/%40sveltejs%2Badapter-node%2B1.2.3.patch [2] https://www.npmjs.com/package/patch-package#set-up

lucasew avatar Jan 16 '24 01:01 lucasew

You can install this (or any) branch into a SvelteKit project directly from GitHub using https://gitpkg.vercel.app/

But as you said, it doesn't handle shutting down the server so I wouldn't say it's solved just yet. I would also only accept one file descriptor like most apps unless anyone can think of a use case where someone would want to pass multiple .socket units. I have a version of adapter-node that accepts a TIMEOUT environment variable and handles all of this but I would need some time to prepare a proper PR that also includes some documentation.

karimfromjordan avatar Jan 16 '24 02:01 karimfromjordan

You can install this (or any) branch into a SvelteKit project directly from GitHub using https://gitpkg.vercel.app/

But as you said, it doesn't handle shutting down the server so I wouldn't say it's solved just yet. I would also only accept one file descriptor like most apps unless anyone can think of a use case where someone would want to pass multiple .socket units. I have a version of adapter-node that accepts a TIMEOUT environment variable and handles all of this but I would need some time to prepare a proper PR that also includes some documentation.

I found how to do this. I updated that patch to support it. That link will not show the latest version because it's a permalink.

Everything is opt-in. The socket activation logic will be activated only when it detects the environment variables that systemd sets and the idle shutdown will only happens if INACTIVE_TIMEOUT is set to the amount of allowed inactivity in miliseconds. When the server shuts down it shuts down cleanly and it's properly restarted if there is more acesses.

BTW the cf-torrent unit is taking about 17MB of RAM (from systemctl status). I am doing it as a systemd unit because NixOS makes it very convenient to use.

[1] https://github.com/lucasew/cf-torrent/blob/45bc52f8a593892874064e211a378ac811deba7b/patches/%40sveltejs%2Badapter-node%2B1.2.3.patch

lucasew avatar Jan 16 '24 16:01 lucasew

I've just created an alternative PR https://github.com/sveltejs/kit/pull/11653 with documentation and instructions on how to try it out yourself.

karimfromjordan avatar Jan 17 '24 04:01 karimfromjordan

@karimfromjordan cool, thanks for picking it up! Much appreciated 👍

elohmeier avatar Jan 17 '24 10:01 elohmeier