cloudflare / tableflip

Graceful process restarts in Go
BSD 3-Clause "New" or "Revised" License
2.88k stars 147 forks source link

Keep foreground control with signal propagation after successful upgrade #59

Closed jschaf closed 3 years ago

jschaf commented 3 years ago

First off: I'm loving tableflip, I use it to livereload a dev server for my homegrown static site generator server https://github.com/jschaf/b2/blob/master/cmd/server/server.go#L121.

This is more of question than an issue. If this isn't a good spot for it, I'm good with closing it.

After a successful tableflip.Upgrade, I'd like the new process to be the foreground process in a terminal or shell. The reason I want this is so I can forward SIGINT via ctrl-c in the terminal. Using systemd is a bit heavyweight for my simple local development use-case.

What happens now

  1. Start server in terminal with go run ./cmd/server.
  2. Server runs in foreground and output goes to stdout and stderr.
  3. Upgrade server which starts a new server process with a different PID in the background.
  4. Terminal shows prompt since foreground process exited.
  5. New server still writes output to terminal.
  6. Old server exits, leaving the new process parentless, so new process reparents to the PID 1 (systemd --user in my case).

What I'd like to happen

In step 3, the new server should keep running in the foreground and continue writing stdout and stderr of the new process.

I'm not quite sure how to go about this. Would something like the following work?

Alternately, maybe get the new PID from the PID file and do some exec magic?

jschaf commented 3 years ago

I have the option of keeping the old process alive and forwarding signals mostly working. I've hit one snag, tableflip.Upgrade returns the error parent hasn't exited from the new process.

I've verified the the old process calls tableflip.Stop(). Is there additional state that's only cleaned up if the parent process exits?

Edit Looks like neverCloseThisFile might prevent the parent upgrader from exiting.

// Save file in exitFd, so that it's only closed when the process
// exits. This signals to the new process that the old process
// has exited.
u.exitFd <- neverCloseThisFile{file}
lmb commented 3 years ago

Sorry for the late reply, I'm glad you find tableflip useful!

You're pretty much spot on, this is a feature I'd like as well. The problem with keeping the old parent around is that it violates one of the invariants that I set for this package: after a successful upgrade, no old code is left running.

Think of it another way, what would happen if you implement your idea and then do multiple upgrades? Would you keep growing the number of processes? I've not found good answers to this, so I just resorted to using a systemd-run shim for testing upgrades.

jschaf commented 3 years ago

Think of it another way, what would happen if you implement your idea and then do multiple upgrades?

I think you can do it if you add another opt-in invariant: after receiving a single upgrade the old code must exit. That's a bad invariant for production. If the new server fails to start, now you have no servers, so maybe not worth adding.

For a simpler approach, maybe I could orchestrate with a wrapper binary, similar to the systemd shim? And even simpler still, I could skip tableflip and kill the server then restart it without the graceful handoff.

lmb commented 3 years ago

I think you can do it if you add another opt-in invariant: after receiving a single upgrade the old code must exit.

So you'd be able to trigger one upgrade, but triggering number 2 would have the same behaviour as currently? I think that's pretty difficult to explain to users, so I'd rather not add it.

For a simpler approach, maybe I could orchestrate with a wrapper binary, similar to the systemd shim?

Yes, if you want to avoid systemd that's doable.

jschaf commented 3 years ago

Thanks, I don't see a good way to add this functionality to the library so I'll close this.