Skip to content

Commit

Permalink
chore: refactor shutdown config, to prevent setting zero values
Browse files Browse the repository at this point in the history
  • Loading branch information
kevwan committed Jan 1, 2025
1 parent 22a41ca commit 3859089
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 23 deletions.
36 changes: 23 additions & 13 deletions core/proc/shutdown.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,25 @@ import (
"github.com/zeromicro/go-zero/core/threading"
)

type ProcConf struct {
WrapUpTime time.Duration `json:",default=1s"`
WaitTime time.Duration `json:",default=5.5s"`
}
const (
defaultWrapUpTime = time.Second
// why we use 5500 milliseconds is because most of our queue are blocking mode with 5 seconds
defaultWaitTime = 5500 * time.Millisecond
)

var (
wrapUpListeners = new(listenerManager)
shutdownListeners = new(listenerManager)
wrapUpTime = time.Second
// why we use 5500 milliseconds is because most of our queue are blocking mode with 5 seconds
delayTimeBeforeForceQuit = 5500 * time.Millisecond
wrapUpTime = defaultWrapUpTime
waitTime = defaultWaitTime
)

// ShutdownConf defines the shutdown configuration for the process.
type ShutdownConf struct {
WrapUpTime time.Duration `json:",default=1s"`
WaitTime time.Duration `json:",default=5.5s"`
}

// AddShutdownListener adds fn as a shutdown listener.
// The returned func can be used to wait for fn getting called.
func AddShutdownListener(fn func()) (waitForCalled func()) {
Expand All @@ -40,12 +46,16 @@ func AddWrapUpListener(fn func()) (waitForCalled func()) {

// SetTimeToForceQuit sets the waiting time before force quitting.
func SetTimeToForceQuit(duration time.Duration) {
delayTimeBeforeForceQuit = duration
waitTime = duration
}

func Setup(conf ProcConf) {
wrapUpTime = conf.WrapUpTime
delayTimeBeforeForceQuit = conf.WaitTime
func Setup(conf ShutdownConf) {
if conf.WrapUpTime > 0 {
wrapUpTime = conf.WrapUpTime
}
if conf.WaitTime > 0 {
waitTime = conf.WaitTime
}
}

// Shutdown calls the registered shutdown listeners, only for test purpose.
Expand All @@ -67,8 +77,8 @@ func gracefulStop(signals chan os.Signal, sig syscall.Signal) {
time.Sleep(wrapUpTime)
go shutdownListeners.notifyListeners()

time.Sleep(delayTimeBeforeForceQuit - wrapUpTime)
logx.Infof("Still alive after %v, going to force kill the process...", delayTimeBeforeForceQuit)
time.Sleep(waitTime - wrapUpTime)
logx.Infof("Still alive after %v, going to force kill the process...", waitTime)

Check warning on line 81 in core/proc/shutdown.go

View check run for this annotation

Codecov / codecov/patch

core/proc/shutdown.go#L80-L81

Added lines #L80 - L81 were not covered by tests
_ = syscall.Kill(syscall.Getpid(), sig)
}

Expand Down
53 changes: 45 additions & 8 deletions core/proc/shutdown_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,13 @@ import (
)

func TestShutdown(t *testing.T) {
t.Cleanup(func() {
wrapUpTime = defaultWrapUpTime
waitTime = defaultWaitTime
})

SetTimeToForceQuit(time.Hour)
assert.Equal(t, time.Hour, delayTimeBeforeForceQuit)
assert.Equal(t, time.Hour, waitTime)

var val int
called := AddWrapUpListener(func() {
Expand All @@ -31,8 +36,13 @@ func TestShutdown(t *testing.T) {
}

func TestShutdownWithMultipleServices(t *testing.T) {
t.Cleanup(func() {
wrapUpTime = defaultWrapUpTime
waitTime = defaultWaitTime
})

SetTimeToForceQuit(time.Hour)
assert.Equal(t, time.Hour, delayTimeBeforeForceQuit)
assert.Equal(t, time.Hour, waitTime)

var val int32
called1 := AddShutdownListener(func() {
Expand All @@ -49,8 +59,13 @@ func TestShutdownWithMultipleServices(t *testing.T) {
}

func TestWrapUpWithMultipleServices(t *testing.T) {
t.Cleanup(func() {
wrapUpTime = defaultWrapUpTime
waitTime = defaultWaitTime
})

SetTimeToForceQuit(time.Hour)
assert.Equal(t, time.Hour, delayTimeBeforeForceQuit)
assert.Equal(t, time.Hour, waitTime)

var val int32
called1 := AddWrapUpListener(func() {
Expand All @@ -67,6 +82,11 @@ func TestWrapUpWithMultipleServices(t *testing.T) {
}

func TestNotifyMoreThanOnce(t *testing.T) {
t.Cleanup(func() {
wrapUpTime = defaultWrapUpTime
waitTime = defaultWaitTime
})

ch := make(chan struct{}, 1)

go func() {
Expand Down Expand Up @@ -97,10 +117,27 @@ func TestNotifyMoreThanOnce(t *testing.T) {
}

func TestSetup(t *testing.T) {
Setup(ProcConf{
WrapUpTime: time.Second * 2,
WaitTime: time.Second * 30,
cleanup := func() {
wrapUpTime = defaultWrapUpTime
waitTime = defaultWaitTime
}

t.Run("valid time", func(t *testing.T) {
defer cleanup()

Setup(ShutdownConf{
WrapUpTime: time.Second * 2,
WaitTime: time.Second * 30,
})
assert.Equal(t, time.Second*2, wrapUpTime)
assert.Equal(t, time.Second*30, waitTime)
})

t.Run("valid time", func(t *testing.T) {
defer cleanup()

Setup(ShutdownConf{})
assert.Equal(t, defaultWrapUpTime, wrapUpTime)
assert.Equal(t, defaultWaitTime, waitTime)
})
assert.Equal(t, time.Second*2, wrapUpTime)
assert.Equal(t, time.Second*30, delayTimeBeforeForceQuit)
}
4 changes: 2 additions & 2 deletions core/service/serviceconf.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type (
Prometheus prometheus.Config `json:",optional"`
Telemetry trace.Config `json:",optional"`
DevServer DevServerConfig `json:",optional"`
Proc proc.ProcConf `json:",optional"`
Shutdown proc.ShutdownConf `json:",optional"`

Check failure on line 40 in core/service/serviceconf.go

View workflow job for this annotation

GitHub Actions / Windows

undefined: proc.ShutdownConf
}
)

Expand All @@ -62,7 +62,7 @@ func (sc ServiceConf) SetUp() error {
sc.Telemetry.Name = sc.Name
}
trace.StartAgent(sc.Telemetry)
proc.Setup(sc.Proc)
proc.Setup(sc.Shutdown)

Check failure on line 65 in core/service/serviceconf.go

View workflow job for this annotation

GitHub Actions / Windows

undefined: proc.Setup
proc.AddShutdownListener(func() {
trace.StopAgent()
})
Expand Down

0 comments on commit 3859089

Please sign in to comment.