WIP: Implement systemd-notify protocol #21140
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
delvh
reviewed
Sep 11, 2022
| func (srv *Server) Notify(msg NotifyMsg) { | ||
| notifySocket := Os.Getenv("NOTIFY_SOCKET") | ||
| if notifySocket == "" { | ||
| log.Debug("no NOTIFY_SOCKET given") |
There was a problem hiding this comment.
Suggested change
| log.Debug("no NOTIFY_SOCKET given") | |
| log.Debug("no NOTIFY_SOCKET given, Gitea is probably not deployed using systemd") |
Comment on lines
+155
to
+169
| type NotifyMsg int8 | ||
|
|
||
| const { | ||
| ReadyMsg NotifyMsg = iota | ||
| StoppingMsg | ||
| } | ||
|
|
||
| func (msg *NotifyMsg) Bytes() []byte { | ||
| switch msg { | ||
| case ReadyMsg: | ||
| return []byte("READY=1") | ||
| case StoppingMsg: | ||
| return []byte("STOPPING=1") | ||
| } | ||
| } |
There was a problem hiding this comment.
As far as I can see, only the bytes are needed in your implementation.
So, why not skip the ints and make them strings/ byte arrays immediately?
There was a problem hiding this comment.
I was trying to pretend that golang has abstract data types but I probably should just give up and use byte strings.
| } | ||
| } | ||
|
|
||
| // systemd notify protocol |
There was a problem hiding this comment.
Suggested change
| // systemd notify protocol | |
| // Notify notifies systemd (if present) about the given startup status of Gitea |
Comment on lines
+186
to
+198
| log.Warn("failed to unsetenv NOTIFY_SOCKET: %v", err) | ||
| return | ||
| } | ||
|
|
||
| conn, err := net.DialUnix(socketAddr.Net, nil, socketAddr) | ||
| if err != nil { | ||
| log.Warn("failed to dial NOTIFY_SOCKET: %v", err) | ||
| return | ||
| } | ||
| defer conn.close() | ||
|
|
||
| if _, err = conn.Write(msg.Bytes()) { | ||
| log.Warn("failed to notify NOTIFY_SOCKET: %v", err) |
There was a problem hiding this comment.
Suggested change
| log.Warn("failed to unsetenv NOTIFY_SOCKET: %v", err) | |
| return | |
| } | |
| conn, err := net.DialUnix(socketAddr.Net, nil, socketAddr) | |
| if err != nil { | |
| log.Warn("failed to dial NOTIFY_SOCKET: %v", err) | |
| return | |
| } | |
| defer conn.close() | |
| if _, err = conn.Write(msg.Bytes()) { | |
| log.Warn("failed to notify NOTIFY_SOCKET: %v", err) | |
| SystemdNotificationImpossible("unsetenv", err) | |
| return | |
| } | |
| conn, err := net.DialUnix(socketAddr.Net, nil, socketAddr) | |
| if err != nil { | |
| SystemdNotificationImpossible("dial", err) | |
| return | |
| } | |
| defer conn.close() | |
| if _, err = conn.Write(msg.Bytes()) { | |
| SystemdNotificationImpossible("notify", err) |
| return []byte("STOPPING=1") | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Suggested change
| func SystemdNotificationImpossible(action string, err error) { | |
| log.Warn("Failed to %s NOTIFY_SOCKET (%v). Systemd might show some weird states: %v", action, os.Getenv("NOTIFY_SOCKET"), err) | |
| } | |
GiteaBot
added
the
lgtm/need 2
|
So I think this is in the wrong place. The Notify function will belong in manager_unix.go and manager.go not server.go. The functions and structs in server.go are called for multiple connections that gitea uses. The manager is the controller and it is that which should be notifying things. |
zeripath
pushed a commit
to zeripath/gitea
that referenced
this pull request
Sep 12, 2022
This PR adds support for the systemd notify protocol. Several status messagess are provided. We should likely add a common notify/status message for graceful. Replaces go-gitea#21140 Signed-off-by: Andrew Thornton <[email protected]>
|
Will close this in favour of @zeripath's PR since he's putting the work into this. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Right now, the systemd service will mark gitea as started immediately after the process is started, even though there are multiple time-consuming steps that might be run before the server is actually up. This implements systemd's notify protocol to indicate when the server is ready to receive requests, and when it's shutting down. This is particularly useful for socket activation, since systemd will hold the traffic in a queue until the process is ready, to avoid dropping requests.
Since notifications are done via an environment variable, this has no effect on systems where this environment variable is not present.
Marking as a draft since I'm currently developing from a pinebook and it's a bit too slow for me to test the changes for now, but I wanted to put up the code as a proof of concept and will mark it ready once I test it.