Support zero downtime restarts #690

Merged
merged 1 commit into from Sep 11, 2014

Projects

None yet

3 participants

@pushrax
Contributor
pushrax commented Aug 16, 2014

Getting the ball rolling for #261. Seems to work well for me, but I haven't tested it in production yet. As with unix sockets, SSL is not (yet) supported.

@xthexder

@brendensoares brendensoares added this to the v0.11 milestone Aug 19, 2014
@brendensoares
Member

How are you going about testing this?

@pushrax
Contributor
pushrax commented Aug 20, 2014

My local testing indicates it's working, and I'll try it in production on mods.io soon. We should probably set up some automated tests as well, though it's a bit of an annoying case to test.

Additionally, we can release it as a beta feature since it's behind the config flag, and invite people to test it out.

@pushrax
Contributor
pushrax commented Aug 21, 2014

btw, no need to assign the PR author to the PR :P

@brendensoares
Member

@pushrax don't mess with my conventions however redundant they may be! :) I guess in my defense, an assignee shows who has taken responsibility for an issue in the case that someone has submitted a PR needing team review. When a team member submits a PR, it gets a little weird lol.

Also, I like the beta feature idea 👍 How should we track such features for testing and finalizing in a future release?

@brendensoares brendensoares merged commit f157814 into develop Sep 11, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@brendensoares
Member

Let's see how it goes :)

@pushrax pushrax deleted the graceful-restarts branch Sep 13, 2014
@pushrax
Contributor
pushrax commented Sep 13, 2014

Cool, will try this out in production ASAP as well.

@ghost
ghost commented Oct 2, 2014

@pushrax, I think it should be included on *nix based systems only. Looks like it breaks the build process on Windows systems. The error is in rcrowley/goagain: undefined: syscall SIGUSR1/2 because these are UNIX commands (Related).

@brendensoares
Member

Would be an interesting future issue to investigate a Windows capable solution. I'm sure there are Windows API syscalls that could accomplish the same thing. Just needs more expertise/experimenting.

Though, Windows has always been low priority in my mind for Revel.

@pushrax
Contributor
pushrax commented Oct 3, 2014

We can remove this and leave it up to the app once main() is exposed.

@brendensoares
Member

@pushrax that seems like a heavy responsibility for an average developer to care for. I'd much rather come up with a fix for Windows.

@pushrax
Contributor
pushrax commented Oct 8, 2014

The average developer won't make use of this anyway – it's more of a feature for production deployments. In addition, I highly doubt anyone runs Revel in production on a Windows box, so if we really want this we can just use conditional compilation to exclude Windows.

@brendensoares
Member

@pushrax the way this was written cannot be conditionally compiled (currently).

@pushrax
Contributor
pushrax commented Oct 8, 2014

Yes, we would need to make a new file.

@brendensoares
Member

Curious, is this PR going to jive with #593?

@brendensoares
Member

TODO

  • use go build tags to offer a windows and non-windows solution

This has been pushed back to v0.12 and will be removed from the v0.11 release. Thanks.

@brendensoares brendensoares restored the graceful-restarts branch Oct 20, 2014
@brendensoares brendensoares modified the milestone: v0.13, v0.11 Jan 2, 2015
@paritosh90

@brendensoares Have we come up with any solution on this.
@pushrax The main file is generated dynamically at compile time in tmp. Can we change it without breaking process?

@pushrax pushrax was unassigned by paritosh90 Aug 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment