Skip to content

feat: support systemd socket activation #10977

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

Conversation

elohmeier
Copy link

@elohmeier elohmeier commented Nov 4, 2023

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:

  • 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
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • 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:.

Copy link

changeset-bot bot commented Nov 4, 2023

🦋 Changeset detected

Latest commit: 7ad9733

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

@elohmeier elohmeier force-pushed the adapter-node-systemd-socket-activation branch from 02e6464 to 0154801 Compare November 4, 2023 14:32
@eltigerchino eltigerchino changed the title adapter-node: configure systemd socket activation feat: systemd socket activation support Nov 5, 2023
@eltigerchino eltigerchino changed the title feat: systemd socket activation support feat: support systemd socket activation Nov 5, 2023
@Rich-Harris
Copy link
Member

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?

@karimfromjordan
Copy link
Contributor

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.

@benmccann
Copy link
Member

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

@karimfromjordan
Copy link
Contributor

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.

Comment on lines +15 to +18
const sd_listen_fds_start = 3;

if (listen_pid && listen_fds && process.pid === parseInt(listen_pid)) {
const fd = sd_listen_fds_start + parseInt(listen_fds) - 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const sd_listen_fds_start = 3;
if (listen_pid && listen_fds && process.pid === parseInt(listen_pid)) {
const fd = sd_listen_fds_start + parseInt(listen_fds) - 1;
// systemd file descriptor start index - https://0pointer.de/blog/projects/socket-activation.html
const SD_LISTEN_FDS_START = 3;
if (listen_pid && listen_fds && process.pid === parseInt(listen_pid)) {
const fd = SD_LISTEN_FDS_START + parseInt(listen_fds) - 1;

@karimfromjordan
Copy link
Contributor

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.

@lucasew
Copy link

lucasew commented Jan 15, 2024

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
Copy link

lucasew commented Jan 16, 2024

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

@karimfromjordan
Copy link
Contributor

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.

@lucasew
Copy link

lucasew commented Jan 16, 2024

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

@karimfromjordan
Copy link
Contributor

I've just created an alternative PR #11653 with documentation and instructions on how to try it out yourself.

@elohmeier
Copy link
Author

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

@elohmeier elohmeier closed this Jan 17, 2024
@elohmeier elohmeier deleted the adapter-node-systemd-socket-activation branch January 17, 2024 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

adapter-node: systemd socket activation support
7 participants