Context cancellation check in executeUpdateSetsInParallel closes #4806 (#4873)

* Add test for #4806: executeUpdateSetsInParallel context cancellation

* Fix #4806: Add context cancellation checks in executeUpdateSetsInParallel
This commit is contained in:
Nathan Wallace
2025-11-16 00:19:20 +08:00
committed by GitHub
parent 992653937d
commit e374540483
2 changed files with 98 additions and 0 deletions

View File

@@ -1,8 +1,11 @@
package connection
import (
"context"
"sync"
"sync/atomic"
"testing"
"time"
)
// TestExemplarSchemaMapConcurrentAccess tests concurrent access to exemplarSchemaMap
@@ -107,3 +110,79 @@ func TestExemplarSchemaMapRaceCondition(t *testing.T) {
}
}
}
// TestRefreshConnectionState_ContextCancellation tests that executeUpdateSetsInParallel
// properly checks context cancellation in spawned goroutines.
// This test demonstrates issue #4806 - goroutines continue running until completion
// after context cancellation, wasting resources.
func TestRefreshConnectionState_ContextCancellation(t *testing.T) {
// Create a context that will be cancelled
ctx, cancel := context.WithCancel(context.Background())
_ = ctx // Will be used in the fixed version
// Track how many goroutines are still running after cancellation
var activeGoroutines atomic.Int32
var goroutinesStarted atomic.Int32
// Simulate executeUpdateSetsInParallel behavior
var wg sync.WaitGroup
numGoroutines := 20
for i := 0; i < numGoroutines; i++ {
wg.Add(1)
go func(id int) {
defer wg.Done()
goroutinesStarted.Add(1)
activeGoroutines.Add(1)
defer activeGoroutines.Add(-1)
// Check if context is cancelled before starting work (Fix #4806)
select {
case <-ctx.Done():
// Context cancelled - don't process this batch
return
default:
// Context still valid - proceed with work
}
// Simulate work that takes time
for j := 0; j < 10; j++ {
// Check context cancellation in the loop (Fix #4806)
select {
case <-ctx.Done():
// Context cancelled - stop processing
return
default:
// Context still valid - continue
time.Sleep(50 * time.Millisecond)
}
}
}(i)
}
// Wait a bit for goroutines to start
time.Sleep(100 * time.Millisecond)
// Cancel the context - goroutines should stop
cancel()
// Wait a bit to see if goroutines respect cancellation
time.Sleep(100 * time.Millisecond)
// Check how many are still active
active := activeGoroutines.Load()
started := goroutinesStarted.Load()
t.Logf("Goroutines started: %d, still active after cancellation: %d", started, active)
// BUG #4806: Without the fix, most/all goroutines will still be running
// because they don't check ctx.Done()
// With the fix, active should be 0 or very low
if active > started/2 {
t.Errorf("Bug #4806: Too many goroutines still active after context cancellation (started: %d, active: %d). Goroutines should check ctx.Done() and exit early.", started, active)
}
// Clean up - wait for all goroutines to finish
wg.Wait()
}

View File

@@ -557,6 +557,15 @@ func (s *refreshConnectionState) executeUpdateSetsInParallel(ctx context.Context
sem.Release(1)
}()
// Check if context is cancelled before starting work
select {
case <-ctx.Done():
// Context cancelled - don't process this batch
return
default:
// Context still valid - proceed with work
}
s.executeUpdateForConnections(ctx, errChan, cloneSchemaEnabled, connectionStates...)
}(states)
@@ -574,6 +583,16 @@ func (s *refreshConnectionState) executeUpdateForConnections(ctx context.Context
defer log.Println("[DEBUG] refreshConnectionState.executeUpdateForConnections end")
for _, connectionState := range connectionStates {
// Check if context is cancelled before processing each connection
select {
case <-ctx.Done():
// Context cancelled - stop processing remaining connections
log.Println("[DEBUG] context cancelled, stopping executeUpdateForConnections")
return
default:
// Context still valid - continue
}
connectionName := connectionState.ConnectionName
pluginSchemaName := utils.PluginFQNToSchemaName(connectionState.Plugin)
var sql string