aboutsummaryrefslogtreecommitdiffstats
path: root/modules/queue/workergroup.go
diff options
context:
space:
mode:
authorwxiaoguang <wxiaoguang@gmail.com>2024-03-03 00:07:54 +0800
committerGitHub <noreply@github.com>2024-03-02 16:07:54 +0000
commit6465f94a2d26cdacc232fddc20f98d98df61ddac (patch)
tree13ef5246ba51d6906cf9d902c22b05b8cd1e4a9b /modules/queue/workergroup.go
parenta3f05d0d98408bb47333b19f505b21afcefa9e7c (diff)
downloadgitea-6465f94a2d26cdacc232fddc20f98d98df61ddac.tar.gz
gitea-6465f94a2d26cdacc232fddc20f98d98df61ddac.zip
Fix queue worker incorrectly stopped when there are still more items in the queue (#29532)
Without `case <-t.C`, the workers would stop incorrectly, the test won't pass. For the worse case, there might be only one running worker processing the queue items for long time because other workers are stopped. The root cause is related to the logic of doDispatchBatchToWorker. It isn't a serious problem at the moment, so keep it as-is.
Diffstat (limited to 'modules/queue/workergroup.go')
-rw-r--r--modules/queue/workergroup.go20
1 files changed, 16 insertions, 4 deletions
diff --git a/modules/queue/workergroup.go b/modules/queue/workergroup.go
index 147a4f335e..e3801ef2b2 100644
--- a/modules/queue/workergroup.go
+++ b/modules/queue/workergroup.go
@@ -60,6 +60,9 @@ func (q *WorkerPoolQueue[T]) doDispatchBatchToWorker(wg *workerGroup[T], flushCh
full = true
}
+ // TODO: the logic could be improved in the future, to avoid a data-race between "doStartNewWorker" and "workerNum"
+ // The root problem is that if we skip "doStartNewWorker" here, the "workerNum" might be decreased by other workers later
+ // So ideally, it should check whether there are enough workers by some approaches, and start new workers if necessary.
q.workerNumMu.Lock()
noWorker := q.workerNum == 0
if full || noWorker {
@@ -143,7 +146,11 @@ func (q *WorkerPoolQueue[T]) doStartNewWorker(wp *workerGroup[T]) {
log.Debug("Queue %q starts new worker", q.GetName())
defer log.Debug("Queue %q stops idle worker", q.GetName())
+ atomic.AddInt32(&q.workerStartedCounter, 1) // Only increase counter, used for debugging
+
t := time.NewTicker(workerIdleDuration)
+ defer t.Stop()
+
keepWorking := true
stopWorking := func() {
q.workerNumMu.Lock()
@@ -158,13 +165,18 @@ func (q *WorkerPoolQueue[T]) doStartNewWorker(wp *workerGroup[T]) {
case batch, ok := <-q.batchChan:
if !ok {
stopWorking()
- } else {
- q.doWorkerHandle(batch)
- t.Reset(workerIdleDuration)
+ continue
+ }
+ q.doWorkerHandle(batch)
+ // reset the idle ticker, and drain the tick after reset in case a tick is already triggered
+ t.Reset(workerIdleDuration)
+ select {
+ case <-t.C:
+ default:
}
case <-t.C:
q.workerNumMu.Lock()
- keepWorking = q.workerNum <= 1
+ keepWorking = q.workerNum <= 1 // keep the last worker running
if !keepWorking {
q.workerNum--
}