-
-
Notifications
You must be signed in to change notification settings - Fork 676
Conversation
exec ./dendrite-monolith-server --really-enable-open-registration --tls-cert server.crt --tls-key server.key --config dendrite.yaml -api=${API:-0} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CC: @MadLittleMods I think you were complaining about longer build/execution times, iirc?
Yes! Especially with Synapse, matrix-org/synapse#13204
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like Synapse is already PID 1 inside the container and receives the SIGTERM
request, so this is not the issue. :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, although might be best to run it by @kegsay too.
…nto s7evink/complement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to see some actual timings before/after.
./generate-config -server $SERVER_NAME --ci > dendrite.yaml && \ | ||
cp /complement/ca/ca.crt /usr/local/share/ca-certificates/ && update-ca-certificates && \ | ||
./dendrite-monolith-server --really-enable-open-registration --tls-cert server.crt --tls-key server.key --config dendrite.yaml -api=${API:-0} | ||
exec ./dendrite-monolith-server --really-enable-open-registration --tls-cert server.crt --tls-key server.key --config dendrite.yaml -api=${API:-0} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What difference does this make in practice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seeming 5m 49s on this PR, vs 7m 5s on the last main
run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For Postgres it's almost 5m faster (8m 19s vs 12m 49s)
This PR does the following:
keysize
parameter togenerate-keys
, so we can use lower sized keys when running in CIexec
when executingdendrite-monotlith-server
, making it PID 1 inside docker, which results in Dendrite actually receiving theSIGTERM
signal send by Docker. (Making it faster when running tests with Complement, as we don't take 10 seconds to timeout)CC: @MadLittleMods I think you were complaining about longer build/execution times, iirc?