Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix: limit concurrent database connections in prebuild reconciliation#20908

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
ssncferreira wants to merge1 commit intomain
base:main
Choose a base branch
Loading
fromssncferreira/fix-limit-prebuild-db-connections
Draft
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletionscoderd/prebuilds/preset_snapshot.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -258,6 +258,29 @@ func (p PresetSnapshot) CalculateState() *ReconciliationState {
}
}

// 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.
//
// Presets from active template versions always go through the reconciliation loop
// to ensure desired_instances is maintained correctly.
func (p PresetSnapshot) CanSkipReconciliation() bool {
// Only skip presets from inactive template versions that have absolutely nothing to clean up
if !p.isActive() &&
len(p.Running) == 0 &&
len(p.Expired) == 0 &&
// len(p.InProgress) == 0 CountInProgressPrebuilds only queries active template versions
p.PendingCount == 0 &&
p.Backoff == nil {
return true
}
return false
}

// CalculateActions determines what actions are needed to reconcile the current state with the desired state.
// The function:
// 1. First checks if a backoff period is needed (if previous builds failed)
Expand Down
148 changes: 148 additions & 0 deletionscoderd/prebuilds/preset_snapshot_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,8 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"cdr.dev/slog/sloggers/slogtest"

"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/prebuilds"
"github.com/coder/coder/v2/testutil"
Expand DownExpand Up@@ -1527,6 +1529,152 @@ 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 := slogtest.Make(t, nil)
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
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,
expectedCanSkip: true,
expectedActionNoOp: true,
},
{
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,
expectedCanSkip: false,
expectedActionNoOp: false,
},
{
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,
expectedCanSkip: false,
expectedActionNoOp: false,
},
{
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,
expectedCanSkip: false,
expectedActionNoOp: false, // Should create 5 workspaces
},
{
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),
},
expectedCanSkip: false,
expectedActionNoOp: false, // Should backoff
},
}

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,
false,
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)
Expand Down
13 changes: 13 additions & 0 deletionsenterprise/coderd/prebuilds/reconcile.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -341,13 +341,26 @@ func (c *StoreReconciler) ReconcileAll(ctx context.Context) (stats prebuilds.Rec
}

var eg errgroup.Group
// Limit concurrent goroutines to avoid exhausting the database connection pool.
// Each preset reconciliation may perform multiple sequential database operations
// (creates/deletes), and with a pool limit of 10 connections, allowing unlimited
// concurrency would cause connection contention and thrashing. A limit of 5 ensures
// we stay below the pool limit while maintaining reasonable parallelism.
eg.SetLimit(5)

// Reconcile presets in parallel. Each preset in its own goroutine.
for _, preset := range snapshot.Presets {
ps, err := snapshot.FilterByPreset(preset.ID)
if err != nil {
logger.Warn(ctx, "failed to find preset snapshot", slog.Error(err), slog.F("preset_id", preset.ID.String()))
continue
}
// Performance optimization: Skip presets from inactive template versions that have
// no running workspaces, pending jobs, or in-progress builds. This avoids spawning
// goroutines for presets that don't need reconciliation actions.
if ps.CanSkipReconciliation() {
continue
}

eg.Go(func() error {
// Pass outer context.
Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp