mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: limit concurrent database connections in prebuild reconciliation (#20908)
## Description This PR addresses database connection pool exhaustion during prebuilds reconciliation by introducing two changes: * `CanSkipReconciliation`: Filters out presets that don't need reconciliation before spawning goroutines. This ensures we only create goroutines for presets that will (_most likely_) perform database operations, avoiding unnecessary connection pool usage. * Dynamic `eg.SetLimit`: Limits concurrent goroutines based on the configured database connection pool size (`CODER_PG_CONN_MAX_OPEN / 2`). This replaces the previous hardcoded limit of 5, ensuring the reconciliation loop scales appropriately with the configured pool size while leaving capacity for other database operations. ## Changes * Add `CanSkipReconciliation()` method to `PresetSnapshot` that returns true for inactive presets with no running workspaces, no pending jobs, or expired prebuilds. * Add `maxDBConnections` parameter to `NewStoreReconciler` and compute `reconciliationConcurrency` as half the pool size (minimum 1). * Add `ReconciliationConcurrency()` getter method to `StoreReconciler`. * Add `eg.SetLimit(c.reconciliationConcurrency)` to bound concurrent reconciliation goroutines. * Add `PresetsTotal` and `PresetsReconciled` to `ReconcileStats` for observability. * Add `TestCanSkipReconciliation` unit tests. * Add `TestReconciliationConcurrency` unit tests. * Add benchmark tests for reconciliation performance. ## Benchmarks * `BenchmarkReconcileAll_NoOps`: Tests presets with no reconciliation actions. All presets are filtered by `CanSkipReconciliation`, resulting in no goroutines spawned and no database connections used. * `BenchmarkReconcileAll_ConnectionContention`: Tests presets where all require reconciliation actions. All presets spawn goroutines, but concurrency is limited by `eg.SetLimit(reconciliationConcurrency)`. * `BenchmarkReconcileAll_Mix`: Simulates a realistic scenario with a large subset of inactive presets (filtered by `CanSkipReconciliation`) and a smaller subset requiring reconciliation (limited by `eg.SetLimit`). Closes: https://github.com/coder/coder/issues/20606
This commit is contained in:
Susana Ferreira
@@ -39,7 +39,9 @@ type ReconciliationOrchestrator interface {
|
||||
|
||||
// ReconcileStats contains statistics about a reconciliation cycle.
|
||||
type ReconcileStats struct {
|
||||
Elapsed time.Duration
|
||||
Elapsed time.Duration
|
||||
PresetsTotal int
|
||||
PresetsReconciled int
|
||||
}
|
||||
|
||||
type Reconciler interface {
|
||||
|
||||
@@ -82,6 +82,49 @@ func NewPresetSnapshot(
|
||||
}
|
||||
}
|
||||
|
||||
// CanSkipReconciliation returns true if this preset can safely be skipped during
|
||||
// the reconciliation loop.
|
||||
//
|
||||
// This is a performance optimization to avoid spawning goroutines for presets
|
||||
// that have no work to do. It only returns true for presets from inactive
|
||||
// template versions that have no running workspaces, no pending jobs, and no
|
||||
// in-progress builds.
|
||||
func (p PresetSnapshot) CanSkipReconciliation() bool {
|
||||
// Active presets are never skipped. Presets from active template versions always
|
||||
// go through the reconciliation loop to ensure desired_instances is maintained correctly.
|
||||
if p.isActive() {
|
||||
return false
|
||||
}
|
||||
|
||||
// Inactive presets with running prebuilds means there are prebuilds to delete.
|
||||
if len(p.Running) > 0 {
|
||||
return false
|
||||
}
|
||||
|
||||
// Inactive presets with expired prebuilds means there are expired prebuilds to delete.
|
||||
if len(p.Expired) > 0 {
|
||||
return false
|
||||
}
|
||||
|
||||
// Inactive presets with pending jobs means there are pending jobs to cancel.
|
||||
if p.PendingCount > 0 {
|
||||
return false
|
||||
}
|
||||
|
||||
// Backoff is only populated for active presets, but check defensively.
|
||||
if p.Backoff != nil {
|
||||
return false
|
||||
}
|
||||
|
||||
// Fields not checked (only relevant for active presets):
|
||||
// - PrebuildSchedules: Only affects desired instance calculation.
|
||||
// - InProgress: Only populated for active template versions.
|
||||
// - IsHardLimited: Only populated for active template versions.
|
||||
|
||||
// Inactive preset with nothing to clean up: safe to skip.
|
||||
return true
|
||||
}
|
||||
|
||||
// ReconciliationState represents the processed state of a preset's prebuilds,
|
||||
// calculated from a PresetSnapshot. While PresetSnapshot contains raw data,
|
||||
// ReconciliationState contains derived metrics that are directly used to
|
||||
|
||||
@@ -1527,6 +1527,262 @@ func TestCalculateDesiredInstances(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestCanSkipReconciliation ensures that CanSkipReconciliation only returns true
|
||||
// when CalculateActions would return no actions.
|
||||
func TestCanSkipReconciliation(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
clock := quartz.NewMock(t)
|
||||
logger := testutil.Logger(t)
|
||||
backoffInterval := 5 * time.Minute
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
preset database.GetTemplatePresetsWithPrebuildsRow
|
||||
running []database.GetRunningPrebuiltWorkspacesRow
|
||||
expired []database.GetRunningPrebuiltWorkspacesRow
|
||||
inProgress []database.CountInProgressPrebuildsRow
|
||||
pendingCount int
|
||||
backoff *database.GetPresetsBackoffRow
|
||||
isHardLimited bool
|
||||
expectedCanSkip bool
|
||||
expectedActionNoOp bool
|
||||
}{
|
||||
{
|
||||
name: "inactive_with_nothing_to_cleanup",
|
||||
preset: database.GetTemplatePresetsWithPrebuildsRow{
|
||||
UsingActiveVersion: false,
|
||||
Deleted: false,
|
||||
Deprecated: false,
|
||||
DesiredInstances: sql.NullInt32{Int32: 5, Valid: true},
|
||||
},
|
||||
running: []database.GetRunningPrebuiltWorkspacesRow{},
|
||||
expired: []database.GetRunningPrebuiltWorkspacesRow{},
|
||||
inProgress: []database.CountInProgressPrebuildsRow{},
|
||||
pendingCount: 0,
|
||||
backoff: nil,
|
||||
isHardLimited: false,
|
||||
expectedCanSkip: true, // Inactive with nothing to clean up
|
||||
expectedActionNoOp: true, // No actions needed
|
||||
},
|
||||
{
|
||||
name: "inactive_with_running_workspaces",
|
||||
preset: database.GetTemplatePresetsWithPrebuildsRow{
|
||||
UsingActiveVersion: false,
|
||||
Deleted: false,
|
||||
Deprecated: false,
|
||||
},
|
||||
running: []database.GetRunningPrebuiltWorkspacesRow{
|
||||
{ID: uuid.New()},
|
||||
},
|
||||
expired: []database.GetRunningPrebuiltWorkspacesRow{},
|
||||
inProgress: []database.CountInProgressPrebuildsRow{},
|
||||
pendingCount: 0,
|
||||
backoff: nil,
|
||||
isHardLimited: false,
|
||||
expectedCanSkip: false, // Has running prebuilds to delete
|
||||
expectedActionNoOp: false, // Returns ActionTypeDelete
|
||||
},
|
||||
{
|
||||
name: "inactive_with_pending_jobs",
|
||||
preset: database.GetTemplatePresetsWithPrebuildsRow{
|
||||
UsingActiveVersion: false,
|
||||
Deleted: false,
|
||||
Deprecated: false,
|
||||
},
|
||||
running: []database.GetRunningPrebuiltWorkspacesRow{},
|
||||
expired: []database.GetRunningPrebuiltWorkspacesRow{},
|
||||
inProgress: []database.CountInProgressPrebuildsRow{},
|
||||
pendingCount: 3,
|
||||
backoff: nil,
|
||||
isHardLimited: false,
|
||||
expectedCanSkip: false, // Has pending jobs to cancel
|
||||
expectedActionNoOp: false, // Returns ActionTypeCancelPending
|
||||
},
|
||||
{
|
||||
name: "inactive_with_backoff",
|
||||
preset: database.GetTemplatePresetsWithPrebuildsRow{
|
||||
UsingActiveVersion: false,
|
||||
Deleted: false,
|
||||
Deprecated: false,
|
||||
},
|
||||
running: []database.GetRunningPrebuiltWorkspacesRow{},
|
||||
expired: []database.GetRunningPrebuiltWorkspacesRow{},
|
||||
inProgress: []database.CountInProgressPrebuildsRow{},
|
||||
pendingCount: 0,
|
||||
backoff: &database.GetPresetsBackoffRow{
|
||||
NumFailed: 3,
|
||||
LastBuildAt: clock.Now().Add(-1 * time.Minute),
|
||||
},
|
||||
isHardLimited: false,
|
||||
expectedCanSkip: false, // Has backoff
|
||||
expectedActionNoOp: false, // Returns ActionTypeBackoff
|
||||
},
|
||||
{
|
||||
name: "inactive_deleted_template_with_nothing_to_cleanup",
|
||||
preset: database.GetTemplatePresetsWithPrebuildsRow{
|
||||
UsingActiveVersion: false,
|
||||
Deleted: true,
|
||||
Deprecated: false,
|
||||
},
|
||||
running: []database.GetRunningPrebuiltWorkspacesRow{},
|
||||
expired: []database.GetRunningPrebuiltWorkspacesRow{},
|
||||
inProgress: []database.CountInProgressPrebuildsRow{},
|
||||
pendingCount: 0,
|
||||
backoff: nil,
|
||||
isHardLimited: false,
|
||||
expectedCanSkip: true, // Deleted template with nothing to clean up
|
||||
expectedActionNoOp: true, // No actions needed
|
||||
},
|
||||
{
|
||||
name: "inactive_deprecated_template_with_nothing_to_cleanup",
|
||||
preset: database.GetTemplatePresetsWithPrebuildsRow{
|
||||
UsingActiveVersion: false,
|
||||
Deleted: false,
|
||||
Deprecated: true,
|
||||
},
|
||||
running: []database.GetRunningPrebuiltWorkspacesRow{},
|
||||
expired: []database.GetRunningPrebuiltWorkspacesRow{},
|
||||
inProgress: []database.CountInProgressPrebuildsRow{},
|
||||
pendingCount: 0,
|
||||
backoff: nil,
|
||||
isHardLimited: false,
|
||||
expectedCanSkip: true, // Deprecated template with nothing to clean up
|
||||
expectedActionNoOp: true, // No actions needed
|
||||
},
|
||||
{
|
||||
name: "inactive_hard_limited",
|
||||
preset: database.GetTemplatePresetsWithPrebuildsRow{
|
||||
UsingActiveVersion: false,
|
||||
Deleted: false,
|
||||
Deprecated: false,
|
||||
},
|
||||
running: []database.GetRunningPrebuiltWorkspacesRow{},
|
||||
expired: []database.GetRunningPrebuiltWorkspacesRow{},
|
||||
inProgress: []database.CountInProgressPrebuildsRow{},
|
||||
pendingCount: 0,
|
||||
backoff: nil,
|
||||
isHardLimited: true,
|
||||
expectedCanSkip: true, // Hard limited but nothing to clean up
|
||||
expectedActionNoOp: true, // No actions needed
|
||||
},
|
||||
{
|
||||
name: "active_with_desired_instances",
|
||||
preset: database.GetTemplatePresetsWithPrebuildsRow{
|
||||
UsingActiveVersion: true,
|
||||
Deleted: false,
|
||||
Deprecated: false,
|
||||
DesiredInstances: sql.NullInt32{Int32: 2, Valid: true},
|
||||
},
|
||||
running: []database.GetRunningPrebuiltWorkspacesRow{
|
||||
{ID: uuid.New()},
|
||||
{ID: uuid.New()},
|
||||
},
|
||||
expired: []database.GetRunningPrebuiltWorkspacesRow{},
|
||||
inProgress: []database.CountInProgressPrebuildsRow{},
|
||||
pendingCount: 0,
|
||||
backoff: nil,
|
||||
isHardLimited: false,
|
||||
expectedCanSkip: false, // Active presets are never skipped
|
||||
expectedActionNoOp: true, // Already at desired count
|
||||
},
|
||||
{
|
||||
name: "active_with_no_workspaces",
|
||||
preset: database.GetTemplatePresetsWithPrebuildsRow{
|
||||
UsingActiveVersion: true,
|
||||
Deleted: false,
|
||||
Deprecated: false,
|
||||
DesiredInstances: sql.NullInt32{Int32: 5, Valid: true},
|
||||
},
|
||||
running: []database.GetRunningPrebuiltWorkspacesRow{},
|
||||
expired: []database.GetRunningPrebuiltWorkspacesRow{},
|
||||
inProgress: []database.CountInProgressPrebuildsRow{},
|
||||
pendingCount: 0,
|
||||
backoff: nil,
|
||||
isHardLimited: false,
|
||||
expectedCanSkip: false, // Active presets are never skipped
|
||||
expectedActionNoOp: false, // Returns ActionTypeCreate
|
||||
},
|
||||
{
|
||||
name: "active_with_backoff",
|
||||
preset: database.GetTemplatePresetsWithPrebuildsRow{
|
||||
UsingActiveVersion: true,
|
||||
Deleted: false,
|
||||
Deprecated: false,
|
||||
DesiredInstances: sql.NullInt32{Int32: 5, Valid: true},
|
||||
},
|
||||
running: []database.GetRunningPrebuiltWorkspacesRow{},
|
||||
expired: []database.GetRunningPrebuiltWorkspacesRow{},
|
||||
inProgress: []database.CountInProgressPrebuildsRow{},
|
||||
pendingCount: 0,
|
||||
backoff: &database.GetPresetsBackoffRow{
|
||||
NumFailed: 3,
|
||||
LastBuildAt: clock.Now().Add(-1 * time.Minute),
|
||||
},
|
||||
isHardLimited: false,
|
||||
expectedCanSkip: false, // Active presets are never skipped
|
||||
expectedActionNoOp: false, // Returns ActionTypeBackoff
|
||||
},
|
||||
{
|
||||
name: "active_hard_limited",
|
||||
preset: database.GetTemplatePresetsWithPrebuildsRow{
|
||||
UsingActiveVersion: true,
|
||||
Deleted: false,
|
||||
Deprecated: false,
|
||||
DesiredInstances: sql.NullInt32{Int32: 5, Valid: true},
|
||||
},
|
||||
running: []database.GetRunningPrebuiltWorkspacesRow{},
|
||||
expired: []database.GetRunningPrebuiltWorkspacesRow{},
|
||||
inProgress: []database.CountInProgressPrebuildsRow{},
|
||||
pendingCount: 0,
|
||||
backoff: nil,
|
||||
isHardLimited: true,
|
||||
expectedCanSkip: false, // Active presets are never skipped
|
||||
expectedActionNoOp: false, // Returns ActionTypeCreate (skipped in executeReconciliationAction)
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ps := prebuilds.NewPresetSnapshot(
|
||||
tt.preset,
|
||||
[]database.TemplateVersionPresetPrebuildSchedule{},
|
||||
tt.running,
|
||||
tt.expired,
|
||||
tt.inProgress,
|
||||
tt.pendingCount,
|
||||
tt.backoff,
|
||||
tt.isHardLimited,
|
||||
clock,
|
||||
logger,
|
||||
)
|
||||
|
||||
canSkip := ps.CanSkipReconciliation()
|
||||
require.Equal(t, tt.expectedCanSkip, canSkip)
|
||||
|
||||
actions, err := ps.CalculateActions(backoffInterval)
|
||||
require.NoError(t, err)
|
||||
|
||||
actionNoOp := true
|
||||
for _, action := range actions {
|
||||
if !action.IsNoop() {
|
||||
actionNoOp = false
|
||||
break
|
||||
}
|
||||
}
|
||||
require.Equal(t, tt.expectedActionNoOp, actionNoOp,
|
||||
"CalculateActions() isNoOp mismatch")
|
||||
|
||||
// IMPORTANT: If CanSkipReconciliation is true, CalculateActions must return no actions
|
||||
if canSkip {
|
||||
require.True(t, actionNoOp)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func mustParseTime(t *testing.T, layout, value string) time.Time {
|
||||
t.Helper()
|
||||
parsedTime, err := time.Parse(layout, value)
|
||||
|
||||
Reference in New Issue
Block a user