add: zero-downtime restarts with SIGHUP

[mkleczek]

Provides a way to implement zero-downtime upgrades by letting PostgREST start a replacement process and hand traffic over before the old process exits.

The idea is to Install a SIGHUP handler that requests a restart. It starts the current executable again, waits until the replacement reaches the application ready point, commits the handover, and then stops the old server.

Enable the SIGHUP restart handler only when server-reuseport is enabled and both the main and admin servers use TCP sockets. This keeps restart enabled only for configurations where the replacement can bind its listening sockets before the parent shuts down.

Restart process is integrated with systemd notify by reporting RELOADING=1 during restart and then updating MAINPID together with READY=1 once the replacement process is ready.

---

Implemented control flow:

Standalone Startup

1. runRestartable starts.
2. It checks PGRST_HANDOVER_READ_FD and PGRST_HANDOVER_WRITE_FD.
3. No handover fds are present, so mode is standalone.
4. App.run starts the admin server immediately.
5. App.run installs normal signal handlers.
6. App.run starts the PostgreSQL listener.
7. App.run loads the schema cache.
8. App.run binds the main API socket.
9. Warp enters beforeMainLoop.
   1. PostgREST records the main socket as live.
   2. ready closeSockets ... is called.
   3. If restart is enabled, the SIGHUP handler is installed.
   4. Process is now serving API and admin traffic.

Restart Request

1. The old process receives SIGHUP.
2. The SIGHUP handler calls requestReplacement.
3. The old process sends RELOADING=1 to systemd, if NOTIFY_SOCKET is present.
4. The old process creates two pipes for the private handover channel.
5. The old process forks.
6. The child execs replacementExecutable with the same arguments and environment plus the handover fd env vars.
7. The old process waits for READY from the child.

Replacement Startup

1. The new process starts.
2. runRestartable sees the handover fd env vars.
3. Mode is replacement.
4. App.run does not start the admin server immediately.
5. App.run installs normal signal handlers.
6. App.run starts the PostgreSQL listener.
7. App.run loads the schema cache.
   • If add: load schema cache dump from URI at startup #5031 is merged it can be loaded from parent admin server - even with SO_REUSEPORT there is no risk because replacement admin server is not started yet.
8. App.run binds the main API socket.
9. Warp enters beforeMainLoop.
   1. PostgREST records the main socket as live.
   2. Because mode is replacement, the admin server is started now.
   3. ready closeSockets ... is called.
   4. The new process writes READY to the old process.
   5. The new process blocks waiting for COMMIT.

Commit

1. The old process receives READY.
2. The old process sends MAINPID= and READY=1 to systemd.
3. The old process writes COMMIT to the new process.
4. The old process runs stopAction, which closes old API/admin sockets.
5. Old Warp stops accepting new connections and drains in-progress work.
6. The new process receives COMMIT.
7. The new process installs its own SIGHUP handler if restart is enabled.
8. The new process closes the handover channel.
9. The new process is now the active process.

Failure Before Commit

1. If the new process exits or closes the handover channel before READY, the old process reports HandoverFailed.
2. The old process keeps serving.
3. If needed, the old process terminates the replacement child.
4. No COMMIT is sent.
5. Old sockets remain open, so handover does not create downtime.

[steve-chavez]

@mkleczek Can you explain what's the difference between this and #5036?

[mkleczek]

@mkleczek Can you explain what's the difference between this and #5036?

This one is based on #5036 and automates the whole process inside PostgREST itself (making #5036 obsolete).

With this PR, user is able to restart (without downtime!) PostgREST. So upgrade would be:

1. copy the new binary overwriting the old one
2. issue kill -HUP <pid>

It also cooperates with systemd so you can add the following to the service file:

...
NotifyAccess=main
ExecReload=/bin/kill -HUP $MAINPID
...

to have zero-downtime restarts (so you can upgrade but also change any configuration value).

[steve-chavez]

It also cooperates with systemd

We also have #1517 asking for systemd integration.

Looks very interesting but there's a non-trivial amount of code added and to be reviewed.

Perhaps we can reuse this package https://hackage.haskell.org/package/systemd-2.3.0 somehow? Or is there a way to introduce systemd integration in a more gradual way?

[steve-chavez]

We should also update this doc https://docs.postgrest.org/en/v14/integrations/systemd.html with this feature.

[mkleczek]

It also cooperates with systemd

We also have #1517 asking for systemd integration.

Looks very interesting but there's a non-trivial amount of code added and to be reviewed.

Perhaps we can reuse this package https://hackage.haskell.org/package/systemd-2.3.0 somehow? Or is there a way to introduce systemd integration in a more gradual way?

I wasn't aware of systemd package - looks like it can indeed be re-used.

Having said that, the goal of this PR is not systemd integration by itself, it is here only because restart without notifying systemd about new main process will interfere with systemd managing it. So I implemented basic systemd notification support.

[mkleczek]

Perhaps we can reuse this package https://hackage.haskell.org/package/systemd-2.3.0 somehow? Or is there a way to introduce systemd integration in a more gradual way?

I wasn't aware of systemd package - looks like it can indeed be re-used.

Having said that, the goal of this PR is not systemd integration by itself, it is here only because restart without notifying systemd about new main process will interfere with systemd managing it. So I implemented basic systemd notification support.

[mkleczek]

Looks very interesting but there's a non-trivial amount of code added and to be reviewed.

The most difficulty was not systemd integration as such but with coming up with the right startup sequence and coordination between the parent and child process so that this feature, SO_REUSEPORT and (once merged) #5031 can seamlessly work together.

[steve-chavez]

I wasn't aware of systemd package - looks like it can indeed be re-used.

Let's try to do that and see how much code reduction can we get.

The most difficulty was not systemd integration as such but with coming up with the right startup sequence and coordination between the parent and child process so that this feature SO_REUSEPORT and (once merged) #5031 can seamlessly work together.

It'd be easier to read some docs about the above behavior before trying to review code here, something like a sequence diagram could help. Otherwise it's not easy to understand the logic.

[steve-chavez]

Also check if https://github.com/hercules-ci/warp-systemd helps. TBH I never understood why is systemd socket activation not enough for zero-downtime upgrades for our case. Seems this doesn't need SO_REUSEPORT interaction too.

[mkleczek]

Also check if https://github.com/hercules-ci/warp-systemd helps. TBH I never understood why is systemd socket activation not enough for zero-downtime upgrades for our case. Seems this doesn't need SO_REUSEPORT interaction too.

Systemd socket activation is not zero-downtime - there is downtime period between old instance stopping listening and new instance starting accepting connections. New requests are queued during this period and the clients either:

• see a spike in request latency when all goes well
• if startup time is longer for some reason (or the new instance cannot start) clients will experience timeouts

To achieve real zero-downtime we need multiple instances running at the same time, so that the new instance is handling traffic before the old one stops listening and gracefully shuts down.
That can be achieved only with some kind of a load balancing proxy in front of PostgREST: SO_REUSEPORT is such a (kernel level) proxy.

Secondly, systemd socket activation is Linux only. This PR implements it on all Posix compliant systems (systemd notifications are optional - lack of systemd environment does not prevent the feature to work).

And last but not least: from the point of view of operations or devops teams, having it implemented OOTB in PostgREST simplifies their lives a lot: no need to implement/maintain additional, custom, environment/OS specific scripts/configurations.

[mkleczek]

I wasn't aware of systemd package - looks like it can indeed be re-used.

Let's try to do that and see how much code reduction can we get.

On the second thought - I am not convinced it is worth it. We are talking only a single function, 30 lines of code:

withSystemdNotifier :: (SystemdNotifier -> IO a) -> IO a
withSystemdNotifier action =
  lookupEnv "NOTIFY_SOCKET" >>= maybe (action mempty) withNotifySocket
  where
    withNotifySocket notifySocket =
      bracket
        (openNotifySocket notifySocket)
        NS.close
        $ \sock ->
          action $ NSB.sendAll sock . renderSystemdNotifications

    openNotifySocket notifySocket =
      bracketOnError
        (NS.socket NS.AF_UNIX NS.Datagram NS.defaultProtocol)
        NS.close
        $ \sock ->
          NS.connect sock (NS.SockAddrUnix $ notifySocketAddress notifySocket) $> sock

    notifySocketAddress ('@':xs) = '\0' : xs
    notifySocketAddress xs       = xs

    renderSystemdNotifications =
      ensureTrailingNewline . BS.intercalate "\n" . toList . fmap render
      where
        render = \case
          NotifyReady        -> "READY=1"
          NotifyReloading    -> "RELOADING=1"
          NotifyMainPid pid  -> "MAINPID=" <> showByteString (fromIntegral pid :: Int)

    ensureTrailingNewline txt
      | "\n" `BS.isSuffixOf` txt = txt
      | otherwise                = txt <> "\n"

The most difficulty was not systemd integration as such but with coming up with the right startup sequence and coordination between the parent and child process so that this feature SO_REUSEPORT and (once merged) #5031 can seamlessly work together.

It'd be easier to read some docs about the above behavior before trying to review code here, something like a sequence diagram could help. Otherwise it's not easy to understand the logic.

See updated PR description.

[steve-chavez]

This PR implements it on all Posix compliant systems (systemd notifications are optional - lack of systemd environment does not prevent the feature to work).

@mkleczek I wasn't aware of that at all, could you add some user-facing docs to better understand?

[steve-chavez]

This separation of src/library-windows/PostgREST/Process/Restart/Impl.hs and src/library-posix/.. does help a lot in reviewing.

I'm not sure about the Impl.hs naming though. @taimoorzaeem Perhaps you have other suggestions?

[steve-chavez]

On the second thought - I am not convinced it is worth it. We are talking only a single function, 30 lines of code

Now that the code is better organized into modules it looks easier to review, so not adamant on reusing a library anymore.

[steve-chavez]

Why is systemd inside the library-posix, shouldn't that be in another module?

[mkleczek]

Why is systemd inside the library-posix, shouldn't that be in another module?

It was just too small of a function (and used in only one place) to extract it to a separate module. I'm open to do that if you find it necessary/useful.

[taimoorzaeem]

Hmm, I don't understand why we need the whole library-posix/ and library-windows separation when all we need to do is just add one preprocessor directive. And then move this module to library/PostgREST.

I haven't yet seen any haskell library do this kind of separation so it seems odd to me. Using the CPP directives is the idiomatic haskell way, unless implementation differences are vast, which doesn't seem to be the case here.

Suggested change

	runRestartable ::
	ReplacementConfig ->
	AppRun a ->
	IO a
	runRestartable replacementCfg runApp = do
	bracketOnError
	getChildControl
	(traverse_ closeDuplexChannel) $
	\childControl -> do
	handoverLock <- newMVar ()
	runApp
	(HandoverMode $ isJust childControl)
	(ready replacementCfg childControl handoverLock)
	runRestartable ::
	ReplacementConfig ->
	AppRun a ->
	IO a
	#ifndef mingw32_HOST_OS
	runRestartable replacementCfg runApp = do
	bracketOnError
	getChildControl
	(traverse_ closeDuplexChannel) $
	\childControl -> do
	handoverLock <- newMVar ()
	runApp
	(HandoverMode $ isJust childControl)
	(ready replacementCfg childControl handoverLock)
	#else
	runRestartable _ runApp =
	runApp (HandoverMode False) readyUnsupported
	where
	readyUnsupported _ withRequestReplacement =
	withRequestReplacement $ throwIO $ HandoverFailed "Restart handover is not supported on this platform."
	#endif

[mkleczek]

Hmm, I don't understand why we need the whole library-posix/ and library-windows separation when all we need to do is just add one preprocessor directive.

Preprocessor directives and conditional compilation are evil :)

The differences are more than one function - there are differences in imports as well (unix library is only available on non-windows).

I haven't yet seen any haskell library do this kind of separation so it seems odd to me. Using the CPP directives is the idiomatic haskell way, unless implementation differences are vast, which doesn't seem to be the case here.

I find this way of separating platform specific code much more readable and principled - instead of ad-hoc text inclusion/exclusion that quickly becomes unreadable mess, you have clear separation and visibility into what's platform neutral, what's platform specific and what the platform interface is.

But I am open to changing it to conditional compilation if that's preferred way.

@steve-chavez @wolfgangwalther WDYT?

[wolfgangwalther]

Conditional compilation has other problems, I believe. Some tooling doesn't work with it nicely. I remember doctests were a problem, but maybe not anymore since they are now changed to run compiled code anyway. I think there was something else as well, but I can't remember what it was.

I like the approach introduced here - but I think we should follow through on it. How about using the same pattern for the existing PostgREST.Unix module - which is a historic relict anyway... now that Windows supports Unix Sockets...

We should probably introduce this pattern for existing code in a separate PR and have the discussion there.

[steve-chavez]

I remember the sample bash script that was added on #5036 before and it was a few lines.

Frankly, the cost of of maintaining that bash script is looking much smaller than maintaining this amount of code in core.

And last but not least: from the point of view of operations or devops teams, having it implemented OOTB in PostgREST simplifies their lives a lot: no need to implement/maintain additional, custom, environment/OS specific scripts/configurations.

Yeah but then that's shifting a higher maintenance cost on us, I see some mention of windows being unimplemented in the code; that opens the door for users demanding we implement that later too and deal with edge cases.

Secondly, systemd socket activation is Linux only. This PR implements it on all Posix compliant systems (systemd notifications are optional - lack of systemd environment does not prevent the feature to work).

I don't think we need that kind of flexibilty, we only need systemd.

I'll let other chime in but as it is now this is looking like too much to review and not the right design.

[mkleczek]

I remember the sample bash script that was added on #5036 before and it was a few lines.

Yeah, this PR root is actually your suggestion here: #4703 (comment)

I think SIGHUP handling is a very useful feature in servers (such as haproxy or Nginx).

Frankly, the cost of of maintaining that bash script is looking much smaller than maintaining this amount of code in core.

We already pay the cost of maintaining in-process configuration reloading even though it is imperfect and does not provide full reloading (eg. it is not possible to change log level without restart). It will also never provide a way to turn on GHC metrics in runtime as it requires restart.

So one might think of this PR as the ultimate solution for configuration (and schema cache) reloading. As a bonus it allows upgrades.
The enabler was #4703.

Yeah but then that's shifting a higher maintenance cost on us, I see some mention of windows being unimplemented in the code; that opens the door for users demanding we implement that later too and deal with edge cases.

True.

Secondly, systemd socket activation is Linux only. This PR implements it on all Posix compliant systems (systemd notifications are optional - lack of systemd environment does not prevent the feature to work).

I don't think we need that kind of flexibilty, we only need systemd.

I don't understand this. This PR is more complex because it implements integration with systemd - without that it would be much simpler (but would not work properly under systemd).

I'll let other chime in but as it is now this is looking like too much to review and not the right design.

I hear you - let's think about the idea some more and get back to it in the future.

[wolfgangwalther]

First of all, the idea and feature is fantastic. I believe we should most certainly have this, if we can do it in a maintainable and understandable way.

I'm not sure whether that works, but one thing that should be pretty maintainable, I believe, would be to separate this "restart yourself + handover to new process" into a generic library, one that is not PostgREST-specific. One way to do this would be to create an entirely separate Haskell/hackage project, but I believe this would not make it very maintainable in other aspects for us. However, if we can create this generic library as a sub-library in the postgrest.cabal file, have the entire generic code for it isolated and have some rather simple calls from PostgREST into this library to define the various PostgREST specific things at each step, that could work.

It would allow us to review, understand and maintain the two different complexities involved here separately: the restart/replacement/handover process itself vs. the handling of (admin) sockets, live/ready state etc.

---

I took the description of the implementation of the PR body and stripped it of everything specific to PostgREST - so the generic algorithm would be:

Standalone Startup

Standalone Startup

1. runRestartable starts.
2. It checks ***_HANDOVER_READ_FD and ***_HANDOVER_WRITE_FD.
3. No handover fds are present, so mode is standalone.
4. [App starts...]
5. If restart is enabled, the SIGHUP handler is installed.

Restart Request

1. The old process receives SIGHUP.
2. The SIGHUP handler calls requestReplacement.
3. The old process sends RELOADING=1 to systemd, if NOTIFY_SOCKET is present.
4. The old process creates two pipes for the private handover channel.
5. The old process forks.
6. The child execs replacementExecutable with the same arguments and environment plus the handover fd env vars.
7. The old process waits for READY from the child.

Replacement Startup

1. The new process starts.
2. runRestartable sees the handover fd env vars.
3. Mode is replacement.
4. [App starts in a different way than in standalone mode...]
5. The new process writes READY to the old process.
6. The new process blocks waiting for COMMIT.

Commit

1. The old process receives READY.
2. The old process sends MAINPID= and READY=1 to systemd.
3. The old process writes COMMIT to the new process.
4. The old process runs stopAction [...].
5. The new process receives COMMIT.
6. The new process installs its own SIGHUP handler if restart is enabled.
7. The new process closes the handover channel.
8. The new process is now the active process.

Failure Before Commit

1. If the new process exits or closes the handover channel before READY, the old process reports HandoverFailed.
2. The old process keeps [running].
3. If needed, the old process terminates the replacement child.
4. No COMMIT is sent.

---

A lot of this seems very generic to me. I have not looked at the code at all, but I would expect the generic interface to be something roughly like:

1. Call runRestartable standaloneCallback restartCallback stopCallback, providing three functions which contain all the application specific flow of standalone and restart modes as well as when stopping.
2. The standaloneCallback and restartCallback functions take (at least) a single argument in which the restarter passes a ready or done function or so. For the standalone case, calling this function would register the SIGHUP handler. For the restart case, it would send READY and block for COMMIT.

Having a very simple executable compiled as a test-case for this generic library and run some simple tests with it, would be great - we could even test systemd integration via NixOS tests.

On the PostgREST-side we should be able to see the difference between the standalone and restart callbacks easily, to check on the PostgREST-specific logic here.

[mkleczek]

First of all, the idea and feature is fantastic. I believe we should most certainly have this, if we can do it in a maintainable and understandable way.

I'm not sure whether that works, but one thing that should be pretty maintainable, I believe, would be to separate this "restart yourself + handover to new process" into a generic library,

It is structured that way - there is a generic Restart module implementing the whole workflow. PostgREST App module just calls runRestartable function providing a callback.

There should be no problem with moving the generic module to a separate library.

[steve-chavez]

I fully agree. If we can have the generic parts into a separate library (in tree), review/test that independently and then just plug it into PostgREST as a final step that'd be much better from a maintenance perspective.

[wolfgangwalther]

It is structured that way - there is a generic Restart module implementing the whole workflow.

Yeah, I looked a bit a the code now - looking pretty much like what I had in mind :)

It's mostly separated, but installing the SIGHUP handler is (naturally) not. This would be needed in the generic library, too, to be able to test this with a very simple binary as-is. It's a bit odd to do in the library, though, because it's not extensible - the handler is replaced (and could be replaced from the application code again, in theory).

I think we can deal with this small inconsistency, our "generic" library does not need to be truly generic in the sense of being able to publish this on hackage. But it should be mostly "standalone" in the sense that we can easily test the generic feature entirely in that separate binary.

Separating this into a separate library would certainly increase the "let's treat this part as a blackbox" factor for me and make dealing with this easier mentally.

[steve-chavez]

However, if we can create this generic library as a sub-library in the postgrest.cabal file

It's quite small but we can also do the same for

postgrest/src/library/PostgREST/Debounce.hs

Lines 1 to 19 in 178b1d3

	module PostgREST.Debounce
	( makeDebouncer) where
	import Protolude
	-- | Make a new debouncer action. An internal "worker" thread runs forever
	-- ensuring "action" runs when the "trigger" is called. The "action" is only
	-- executed once over a burst of calls.
	makeDebouncer :: IO () -> IO (IO ())
	makeDebouncer action = do
	flag <- newEmptyMVar
	let worker = forever $ do
	takeMVar flag
	action
	trigger = void $ tryPutMVar flag ()
	void $ forkIO worker
	pure trigger

[taimoorzaeem]

It's quite small but we can also do the same for

postgrest/src/library/PostgREST/Debounce.hs

Lines 1 to 19 in 178b1d3

	module PostgREST.Debounce
	( makeDebouncer) where
	import Protolude
	-- | Make a new debouncer action. An internal "worker" thread runs forever
	-- ensuring "action" runs when the "trigger" is called. The "action" is only
	-- executed once over a burst of calls.
	makeDebouncer :: IO () -> IO (IO ())
	makeDebouncer action = do
	flag <- newEmptyMVar
	let worker = forever $ do
	takeMVar flag
	action
	trigger = void $ tryPutMVar flag ()
	void $ forkIO worker
	pure trigger

Yeah, I remember we discussed to move this to our own "prelude" once we merge protolude into PostgREST. :)

[mkleczek]

It is structured that way - there is a generic Restart module implementing the whole workflow.

Yeah, I looked a bit a the code now - looking pretty much like what I had in mind :)

It's mostly separated, but installing the SIGHUP handler is (naturally) not. This would be needed in the generic library, too, to be able to test this with a very simple binary as-is. It's a bit odd to do in the library, though, because it's not extensible - the handler is replaced (and could be replaced from the application code again, in theory).

If going this direction, it would be natural to make the library more of a "generic main implementation" that also abstracts SIGINT and SIGTERM handling and implements process lifecycle management.

[wolfgangwalther]

If going this direction, it would be natural to make the library more of a "generic main implementation" that also abstracts SIGINT and SIGTERM handling and implements process lifecycle management.

Not opposed to that. We'll still need to install signal handlers from two places, because of SIGUSR1 and SIGUSR2, though.