Add systemd setup that supports workers #4662
Conversation
|
I have this setup in use as you see it here right now. However if others could test this as well and report success/failure back here I'd sleep a little more easily. |
Codecov Report
@@ Coverage Diff @@
## develop #4662 +/- ##
===========================================
- Coverage 75.34% 75.28% -0.07%
===========================================
Files 340 340
Lines 34939 34950 +11
Branches 5722 5723 +1
===========================================
- Hits 26324 26311 -13
- Misses 7002 7024 +22
- Partials 1613 1615 +2 |
contrib/systemd-with-workers/system/matrix-synapse-homeserver.service
Outdated
Show resolved
Hide resolved
|
All requested changes implemented. |
|
Maybe wait a bit before merging still. Using a target you can't easily restart all services. I found these possible solutions though which are using either I'll experiment with this soon and update the pull request with whichever solution I found easier to use/cleaner. |
|
There we go. Now you can restart synapse and all its workers in one go. Even more, if the homeserver app crashes for any reason all workers are automatically stopped by systemd. See the README for more information on what you can do with this setup. From my point of view this is ready to merge. I don't know about the buildkite fails. If you want me to rebase my branch against your current master just let me know. |
targodan
force-pushed
the
systemd-workers
branch
from
0875dc0 to
76bf6f1
Compare
|
I rebased against master in hopes to solve the checks. Don't know what's up with that. |
|
sorry, this fell off my radar a bit. Can you rebase against current develop instead? |
This setup is a way to manage workers with systemd. It does however not require workers. You can use this setup without workers. You just have to make sure that the homeserver is forking and writes its PID file to the location the service is looking in. The currently distributed setup in the debian package does not work in conjunction with workers.
Sets all services to `type=simple` and disables daemonizing on the synapse side.
targodan
force-pushed
the
systemd-workers
branch
from
76bf6f1 to
f767446
Compare
|
Yes of course, sorry. Don't know why the checks still fail though. |
|
Actually found the problem. It's my changelog.d file. Fixing now. |
| [Unit] | ||
| Description=Contains matrix services like synapse, bridges and bots | ||
| After=network.target | ||
| AllowIsolate=no |
There was a problem hiding this comment.
isn't this the default? Is there any reason to add it here?
There was a problem hiding this comment.
Assuming you are refering to the AllowIsolate=no. Yes, it's the default value according to the systemd docs. However I personally am a friend of doing important things explicitly. The matrix.target is one you would never want to isolate as all non matrix services would be stopped, effectively killing the system.
Thus I deem this important. Take the (admittedly unlikely) case that systemd decides to change the default value. We would still want this to be false/no as this target is designed as a grouping of services rather than an isolatable target.
To be fair even with this set to true an admin would have to do systemctl isolate matrix.target in order to cause mayhem, but I'd like to prevent this under any circumstances.
TL; DR: I deem it important that isolated is false and I personally prefer to do important things explicitly rather than implicitly by default values. But yes, it is the default value and I'll leave the final decision up to you. I won't be offended if you tell me to remove it. 🙂
There was a problem hiding this comment.
If you feel like this warrants a deeper/longer discussion feel free to pm me in the matrix @luca:targodan.de
There was a problem hiding this comment.
However I personally am a friend of doing important things explicitly
I think you could argue that there are a bunch of other default settings which are just as important, and pretty soon you end up with a unit file a mile long.
However, I don't have strong feelings here.
Co-Authored-By: targodan <[email protected]>
Co-Authored-By: targodan <[email protected]>
Co-Authored-By: targodan <[email protected]>
richvdh
changed the title
| [Unit] | ||
| Description=Synapse Matrix Worker | ||
| After=matrix-synapse.service | ||
| BindsTo=matrix-synapse.service |
There was a problem hiding this comment.
Does the BindsTo enforce the matrix-synapse.service gets started, too? This seems to be bad when a worker is running on a host different from the Synapse service. Or do I have to start the Synapse process on each host running any worker?
Pull Request Checklist
org/synapse/blob/master/CONTRIBUTING.rst#sign-off)
Content
The currently distributed setup in the debian package does not work in
conjunction with workers. This setup provides a way to manage workers
with systemd. It does however not require workers. You can use this
setup without workers, you just haveto make sure that the homeserver
is forking and writes its PID file to the location the service is looking in.
It also supplies a nice way to autostart services like bridges alongside synapse
utilizing its own
matrix.target.Should the package manager for the debian package be interested in this
and/or has any questions about this, feel free to comment here or contact me
via matrix @luca:targodan.de
Signed-off-by: Luca Corbatto [email protected]