Skip to content

Conversation

@prangarang
Copy link

@prangarang prangarang commented Nov 22, 2019

Swaps graceful shutdown behavior of nginx in our buildpack to SIGTERM.
Graceful shutdown becomes SIGTERM
Fast shutdown becomes SIGCONT, as a bogus signal

Testing:
Not done yet, 2 options:

Create a test app on heroku

heroku create myapp
heroku add-buildpack -a myapp <blah>
heroku run bash -a myapp

Run the binary the buildpack build outputs.

The artifact is an ELF

➜  bin git:(master) ✗ file nginx-cedar-14
nginx-cedar-14: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/l, for GNU/Linux 2.6.24, BuildID[sha1]=c49cdcd6beb3ff726a87683ad749f91acecb1737, with debug_info, not stripped

But not compatible w/ BSD/OSX :(
I think we could start a docker container of cedar-14 and run the ELF from there, but probably easier to just make a dummy heroku app?

➜  bin git:(master) ✗ docker images | grep cedar
heroku/cedar                                   14                  ca0f5d00ade9        7 months ago        1.36GB

Notes:
I picked SIGCONT (continue stopped process) as the new signal for "fast shutdown", defined macros for signal resolution meant I couldn't just give a bogus/unused signal like 9000 without having a SIGNINETHOUSAND def.

I don't think this will cause issues, and felt better than having QUIT do something unexpected if Heroku ever starts sending QUIT.

I only did this for cedar-14, would want to make sure this is good for heroku 16/18 if we do this in the future

It took longer to write these notes than to make the actual POC, it's not great we'd have to potentially do something here as we upgrade stacks, but feel like right now a stack upgrade is probably non-trivial.

The output of the relevant part of the build is:

Graceful shutdown on term, terminate on bogus signal
Signals defined as
#define NGX_SHUTDOWN_SIGNAL      TERM
#define NGX_TERMINATE_SIGNAL     CONT
#define NGX_NOACCEPT_SIGNAL      WINCH
#define NGX_RECONFIGURE_SIGNAL   HUP
#define NGX_REOPEN_SIGNAL        INFO
#define NGX_CHANGEBIN_SIGNAL     XCPU
#define NGX_REOPEN_SIGNAL        USR1
#define NGX_CHANGEBIN_SIGNAL     USR2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants