Skip to content

Commit 3ba4e25

Browse files
fix: only drain workers on graceful shutdown (#1405)
* Only drains workers on shutdown. * trigger build * Marks func as experimental. --------- Co-authored-by: Alliballibaba <alliballibaba@gmail.com>
1 parent 619c903 commit 3ba4e25

File tree

4 files changed

+31
-19
lines changed

4 files changed

+31
-19
lines changed

caddy/caddy.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func (f *FrankenPHPApp) Stop() error {
113113
// note: Exiting() is currently marked as 'experimental'
114114
// https://github.com/caddyserver/caddy/blob/e76405d55058b0a3e5ba222b44b5ef00516116aa/caddy.go#L810
115115
if caddy.Exiting() {
116-
frankenphp.Shutdown()
116+
frankenphp.DrainWorkers()
117117
}
118118

119119
// reset configuration so it doesn't bleed into later tests

frankenphp.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ func Shutdown() {
374374
return
375375
}
376376

377-
drainWorkers()
377+
drainWatcher()
378378
drainAutoScaling()
379379
drainPHPThreads()
380380

metrics.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -380,9 +380,9 @@ func (m *PrometheusMetrics) Shutdown() {
380380
}
381381

382382
if err := m.registry.Register(m.queueDepth); err != nil &&
383-
!errors.As(err, &prometheus.AlreadyRegisteredError{}) {
384-
panic(err)
385-
}
383+
!errors.As(err, &prometheus.AlreadyRegisteredError{}) {
384+
panic(err)
385+
}
386386
}
387387

388388
func getWorkerNameForMetrics(name string) string {
@@ -432,9 +432,9 @@ func NewPrometheusMetrics(registry prometheus.Registerer) *PrometheusMetrics {
432432
}
433433

434434
if err := m.registry.Register(m.queueDepth); err != nil &&
435-
!errors.As(err, &prometheus.AlreadyRegisteredError{}) {
436-
panic(err)
437-
}
435+
!errors.As(err, &prometheus.AlreadyRegisteredError{}) {
436+
panic(err)
437+
}
438438

439439
return m
440440
}

worker.go

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -86,18 +86,14 @@ func newWorker(o workerOpt) (*worker, error) {
8686
return w, nil
8787
}
8888

89-
func drainWorkers() {
90-
watcher.DrainWatcher()
89+
// EXPERIMENTAL: DrainWorkers finishes all worker scripts before a graceful shutdown
90+
func DrainWorkers() {
91+
_ = drainWorkerThreads()
9192
}
9293

93-
// RestartWorkers attempts to restart all workers gracefully
94-
func RestartWorkers() {
95-
// disallow scaling threads while restarting workers
96-
scalingMu.Lock()
97-
defer scalingMu.Unlock()
98-
94+
func drainWorkerThreads() []*phpThread {
9995
ready := sync.WaitGroup{}
100-
threadsToRestart := make([]*phpThread, 0)
96+
drainedThreads := make([]*phpThread, 0)
10197
for _, worker := range workers {
10298
worker.threadMutex.RLock()
10399
ready.Add(len(worker.threads))
@@ -108,17 +104,33 @@ func RestartWorkers() {
108104
continue
109105
}
110106
close(thread.drainChan)
111-
threadsToRestart = append(threadsToRestart, thread)
107+
drainedThreads = append(drainedThreads, thread)
112108
go func(thread *phpThread) {
113109
thread.state.waitFor(stateYielding)
114110
ready.Done()
115111
}(thread)
116112
}
117113
worker.threadMutex.RUnlock()
118114
}
119-
120115
ready.Wait()
121116

117+
return drainedThreads
118+
}
119+
120+
func drainWatcher() {
121+
if watcherIsEnabled {
122+
watcher.DrainWatcher()
123+
}
124+
}
125+
126+
// RestartWorkers attempts to restart all workers gracefully
127+
func RestartWorkers() {
128+
// disallow scaling threads while restarting workers
129+
scalingMu.Lock()
130+
defer scalingMu.Unlock()
131+
132+
threadsToRestart := drainWorkerThreads()
133+
122134
for _, thread := range threadsToRestart {
123135
thread.drainChan = make(chan struct{})
124136
thread.state.set(stateReady)

0 commit comments

Comments
 (0)