Skip to content

Commit

Permalink
Fix race condition making it possible for batches to be >batchSize
Browse files Browse the repository at this point in the history
  • Loading branch information
Philipp Heckel committed Nov 16, 2022
1 parent e147a41 commit db9ca80
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 6 deletions.
13 changes: 8 additions & 5 deletions util/batching_queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,13 @@ func NewBatchingQueue[T any](batchSize int, timeout time.Duration) *BatchingQueu
func (q *BatchingQueue[T]) Enqueue(element T) {
q.mu.Lock()
q.in = append(q.in, element)
limitReached := len(q.in) == q.batchSize
var elements []T
if len(q.in) == q.batchSize {
elements = q.dequeueAll()
}
q.mu.Unlock()
if limitReached {
q.out <- q.dequeueAll()
if len(elements) > 0 {
q.out <- elements
}
}

Expand All @@ -61,8 +64,6 @@ func (q *BatchingQueue[T]) Dequeue() <-chan []T {
}

func (q *BatchingQueue[T]) dequeueAll() []T {
q.mu.Lock()
defer q.mu.Unlock()
elements := make([]T, len(q.in))
copy(elements, q.in)
q.in = q.in[:0]
Expand All @@ -75,7 +76,9 @@ func (q *BatchingQueue[T]) timeoutTicker() {
}
ticker := time.NewTicker(q.timeout)
for range ticker.C {
q.mu.Lock()
elements := q.dequeueAll()
q.mu.Unlock()
if len(elements) > 0 {
q.out <- elements
}
Expand Down
2 changes: 1 addition & 1 deletion util/batching_queue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestBatchingQueue_InfTimeout(t *testing.T) {
for i := 0; i < 101; i++ {
go q.Enqueue(i)
}
time.Sleep(500 * time.Millisecond)
time.Sleep(time.Second)
mu.Lock()
require.Equal(t, 100, total) // One is missing, stuck in the last batch!
require.Equal(t, 4, len(batches))
Expand Down

0 comments on commit db9ca80

Please sign in to comment.