Implement graceful shutdown of HTTP server #15
Conversation
|
cc @Templum |
main.go
Outdated
| path, lockErr := lock() | ||
|
|
||
| if lockErr != nil { | ||
| log.Panicf("Cannot write %s. To disable lock-file set env suppress_lock=true.\n Error: %s.\n", path, lockErr.Error()) |
There was a problem hiding this comment.
Have you tested this @burtonr ? I can't see any code that handles a suppress_lock environmental variable.
There was a problem hiding this comment.
This was a relic of the copy from the original watchdog. I'll need to double check we can still disable the lock file here and test it. I'll need to update that panic message regardless
|
|
||
| acceptingConnections = false | ||
|
|
||
| if err := s.Shutdown(context.Background()); err != nil { |
There was a problem hiding this comment.
It might be a good idea to make use of a context with timeout. ctx, _ := context.WithTimeout(context.Background(), 5*time.Second). As this can be used in the Shutdown, without the need of an additional construct
|
I added the |
When the of-watchdog receives a SIGTERM, it should stop accepting new requests. The current processes in progress should continue up to the write_timeout property value. A new environment variable was also ported over from the original faas watchdog 'suppress_lock' that will not write a lock file to disk, and not provide Swarm with healthchecks. This change is largely copied from the original faas watchdog Tested with the 'hey' tool by invoking functions and forcing the scale to 1 while monitoring the logs to verify the in-flight requests were completed before shutting down the container Signed-off-by: Burton Rheutan <rheutan7@gmail.com>
|
I pushed one more commit up to update the README with the Tested locally the same as before, but this time with the Verified that Swarm healthchecks were not available and the |
|
Fixed the tests by adding an additional request to the mockHttpServer to account for the additional call for Still a WIP though as I need to add additional tests around that piece of functionality |
|
@burtonr thanks for porting my code across from the main watchdog. I can see a conflict now and I need to re-write this code a little bit in light of https://github.com/openfaas/faas/pull/873/files. |
When the of-watchdog reveives a SIGTERM, it should stop accepting
new requests. The current processes in progress should continue up to
the write_timeout property value.
This change is largely copied from the original faas watchdog
Tested with the 'hey' tool by invoking functions and forcing the scale
to 1 while monitoring the logs to verify the in-flight requests were
completed before shutting down the container
Signed-off-by: Burton Rheutan rheutan7@gmail.com
Description
Motivation and Context
Resolves Implement graceful shutdown from classic watchdog #10
How Has This Been Tested?
Tested using the
heytesting toolhey -n 500 -m POST -d "testing hey" -T text/plain http://localhost:8080/function/timer-testdocker service scale timer-test=1Observed the service logs of the
timer-testservice:Types of changes
Checklist:
git commit -s