Skip to content
This repository has been archived by the owner on Apr 11, 2024. It is now read-only.

Support graceful shutdown and hot-deploying #57

Merged
merged 6 commits into from
Mar 1, 2017

Conversation

tcnksm
Copy link
Contributor

@tcnksm tcnksm commented Jan 24, 2017

For hot deploy, support lestrrat/go-server-starter. Graceful shutdown is done by Go1.8 new feature.

Graceful shutdown is kicked by server-starter by sending SIGTERM. If it receives SIGTERM, first it shutdown net/http server. After shutdown, then it will wait until queue will be empty.

Tests are done by same way as this but using samples/client.go.

Not yet done queue flushing test ... (any suggestion how to do this ?)

For hot deploy, support `lestrrat/go-server-starter`.
Graceful shutdown is done by Go1.8 new feature.
@@ -25,6 +25,7 @@ type SectionCore struct {
QueueNum int64 `toml:"queues"`
NotificationMax int64 `toml:"notification_max"`
PusherMax int64 `toml:"pusher_max"`
ShutdownTimeout int64 `toml:"shutdown_timeout"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Document this in CONFIGURATION.md.

Copy link
Contributor

@cubicdaiya cubicdaiya Feb 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And add example config to conf/gaurun.toml.

gaurun.LogError.Info("wait until queue is empty")
flushWaitInterval := 1 * time.Second
for len(gaurun.QueueNotification) > 0 {
time.Sleep(flushWaitInterval)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you improve this mechanism by channel-blocking? This implementation intend to wait 1sec in most cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can not get comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add global wait group !

gaurun/worker.go Outdated
@@ -51,6 +60,9 @@ Retry:

func pushAsync(pusher func(req RequestGaurunNotification) error, req RequestGaurunNotification, retryMax int, pusherCount *int64) {
Retry:
PusherWg.Add(1)
defer PusherWg.Done()

Copy link
Contributor

@cubicdaiya cubicdaiya Feb 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is wrong. As pushAsync() is called asynchronously, These functions should be called before calling pushAsync().

FYI

The comment around below will help you.

https://github.com/mercari/gaurun/blob/feature/hot-deploying/gaurun/worker.go#L108

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

}

gaurun.LogError.Info(fmt.Sprintf("wait until queue is empty. Current queue len: %d", queue))
time.Sleep(3 * time.Second)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be 1 * time.Second?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

}

// Start a goroutine to log number of job queue.
go func() {
Copy link
Contributor

@cubicdaiya cubicdaiya Feb 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This annonymous function has a little deep nest. How about making named function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ummm, I think it's enough (not so deep nest).

@cubicdaiya cubicdaiya added the LGTM label Mar 1, 2017
@cubicdaiya cubicdaiya merged commit 60fc1da into master Mar 1, 2017
@catatsuy catatsuy deleted the feature/hot-deploying branch October 10, 2019 08:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants