mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat: cancel pending prebuilds from non-active template versions (#20387)
## Description This PR introduces an optimization to automatically cancel pending prebuild-related jobs from non-active template versions in the reconciliation loop. ## Problem Currently, when a template is configured with more prebuild instances than available provisioners, the provisioner queue can become flooded with pending prebuild jobs. This issue is worsened when provisioning/deprovisioning operations take a long time. When the prebuild reconciliation loop generates jobs faster than provisioners can process them, pending jobs accumulate in the queue. Since prebuilt workspaces should always run the latest active template version, pending prebuild jobs from non-active versions become obsolete once a new version is promoted. ## Solution The reconciliation loop cancels pending prebuild-related jobs from non-active template versions that match the following criteria: * Build number: 1 (initial build created by the reconciliation loop) * Job status: `pending` * Not yet picked up by a provisioner (`worker_id` is `NULL`) * Owned by the prebuilds system user * Workspace transition: `start` This prevents the queue from being cluttered with stale prebuild jobs that would provision workspaces on an outdated template version that would consequently need to be deprovisioned. ## Changes * Added new SQL query `CountPendingNonActivePrebuilds` to identify presets with pending jobs from non-active versions * Added new SQL query `UpdatePrebuildProvisionerJobWithCancel` to cancel jobs for a specific preset * New reconciliation action type `ActionTypeCancelPending` handles the cancellation logic * Cancellation is non-blocking: failures to cancel prebuild jobs are logged as errors and don't prevent other reconciliation actions ## Follow-up PR Canceling pending prebuild jobs leaves workspaces in a Canceled state. While no Terraform resources need to be destroyed (since jobs were canceled before provisioning started), these database records should still be cleaned up. This will be addressed in a follow-up PR. Closes: https://github.com/coder/coder/issues/20242
This commit is contained in:
@@ -8,10 +8,9 @@ import (
|
||||
|
||||
"cdr.dev/slog"
|
||||
|
||||
"github.com/coder/quartz"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/database"
|
||||
"github.com/coder/coder/v2/coderd/util/slice"
|
||||
"github.com/coder/quartz"
|
||||
)
|
||||
|
||||
// GlobalSnapshot represents a full point-in-time snapshot of state relating to prebuilds across all templates.
|
||||
@@ -20,6 +19,7 @@ type GlobalSnapshot struct {
|
||||
PrebuildSchedules []database.TemplateVersionPresetPrebuildSchedule
|
||||
RunningPrebuilds []database.GetRunningPrebuiltWorkspacesRow
|
||||
PrebuildsInProgress []database.CountInProgressPrebuildsRow
|
||||
PendingPrebuilds []database.CountPendingNonActivePrebuildsRow
|
||||
Backoffs []database.GetPresetsBackoffRow
|
||||
HardLimitedPresetsMap map[uuid.UUID]database.GetPresetsAtFailureLimitRow
|
||||
clock quartz.Clock
|
||||
@@ -31,6 +31,7 @@ func NewGlobalSnapshot(
|
||||
prebuildSchedules []database.TemplateVersionPresetPrebuildSchedule,
|
||||
runningPrebuilds []database.GetRunningPrebuiltWorkspacesRow,
|
||||
prebuildsInProgress []database.CountInProgressPrebuildsRow,
|
||||
pendingPrebuilds []database.CountPendingNonActivePrebuildsRow,
|
||||
backoffs []database.GetPresetsBackoffRow,
|
||||
hardLimitedPresets []database.GetPresetsAtFailureLimitRow,
|
||||
clock quartz.Clock,
|
||||
@@ -46,6 +47,7 @@ func NewGlobalSnapshot(
|
||||
PrebuildSchedules: prebuildSchedules,
|
||||
RunningPrebuilds: runningPrebuilds,
|
||||
PrebuildsInProgress: prebuildsInProgress,
|
||||
PendingPrebuilds: pendingPrebuilds,
|
||||
Backoffs: backoffs,
|
||||
HardLimitedPresetsMap: hardLimitedPresetsMap,
|
||||
clock: clock,
|
||||
@@ -76,10 +78,20 @@ func (s GlobalSnapshot) FilterByPreset(presetID uuid.UUID) (*PresetSnapshot, err
|
||||
// Separate running workspaces into non-expired and expired based on the preset's TTL
|
||||
nonExpired, expired := filterExpiredWorkspaces(preset, running)
|
||||
|
||||
// Includes in-progress prebuilds only for active template versions.
|
||||
// In-progress prebuilds correspond to workspace statuses: 'pending', 'starting', 'stopping', and 'deleting'
|
||||
inProgress := slice.Filter(s.PrebuildsInProgress, func(prebuild database.CountInProgressPrebuildsRow) bool {
|
||||
return prebuild.PresetID.UUID == preset.ID
|
||||
})
|
||||
|
||||
// Includes count of pending prebuilds only for non-active template versions
|
||||
pendingCount := 0
|
||||
if found, ok := slice.Find(s.PendingPrebuilds, func(prebuild database.CountPendingNonActivePrebuildsRow) bool {
|
||||
return prebuild.PresetID.UUID == preset.ID
|
||||
}); ok {
|
||||
pendingCount = int(found.Count)
|
||||
}
|
||||
|
||||
var backoffPtr *database.GetPresetsBackoffRow
|
||||
backoff, found := slice.Find(s.Backoffs, func(row database.GetPresetsBackoffRow) bool {
|
||||
return row.PresetID == preset.ID
|
||||
@@ -96,6 +108,7 @@ func (s GlobalSnapshot) FilterByPreset(presetID uuid.UUID) (*PresetSnapshot, err
|
||||
nonExpired,
|
||||
expired,
|
||||
inProgress,
|
||||
pendingCount,
|
||||
backoffPtr,
|
||||
isHardLimited,
|
||||
s.clock,
|
||||
|
||||
@@ -34,6 +34,9 @@ const (
|
||||
|
||||
// ActionTypeBackoff indicates that prebuild creation should be delayed.
|
||||
ActionTypeBackoff
|
||||
|
||||
// ActionTypeCancelPending indicates that pending prebuilds should be canceled.
|
||||
ActionTypeCancelPending
|
||||
)
|
||||
|
||||
// PresetSnapshot is a filtered view of GlobalSnapshot focused on a single preset.
|
||||
@@ -49,6 +52,7 @@ type PresetSnapshot struct {
|
||||
Running []database.GetRunningPrebuiltWorkspacesRow
|
||||
Expired []database.GetRunningPrebuiltWorkspacesRow
|
||||
InProgress []database.CountInProgressPrebuildsRow
|
||||
PendingCount int
|
||||
Backoff *database.GetPresetsBackoffRow
|
||||
IsHardLimited bool
|
||||
clock quartz.Clock
|
||||
@@ -61,6 +65,7 @@ func NewPresetSnapshot(
|
||||
running []database.GetRunningPrebuiltWorkspacesRow,
|
||||
expired []database.GetRunningPrebuiltWorkspacesRow,
|
||||
inProgress []database.CountInProgressPrebuildsRow,
|
||||
pendingCount int,
|
||||
backoff *database.GetPresetsBackoffRow,
|
||||
isHardLimited bool,
|
||||
clock quartz.Clock,
|
||||
@@ -72,6 +77,7 @@ func NewPresetSnapshot(
|
||||
Running: running,
|
||||
Expired: expired,
|
||||
InProgress: inProgress,
|
||||
PendingCount: pendingCount,
|
||||
Backoff: backoff,
|
||||
IsHardLimited: isHardLimited,
|
||||
clock: clock,
|
||||
@@ -115,7 +121,7 @@ type ReconciliationActions struct {
|
||||
}
|
||||
|
||||
func (ra *ReconciliationActions) IsNoop() bool {
|
||||
return ra.Create == 0 && len(ra.DeleteIDs) == 0 && ra.BackoffUntil.IsZero()
|
||||
return ra.ActionType != ActionTypeCancelPending && ra.Create == 0 && len(ra.DeleteIDs) == 0 && ra.BackoffUntil.IsZero()
|
||||
}
|
||||
|
||||
// MatchesCron interprets a cron spec as a continuous time range,
|
||||
@@ -345,18 +351,30 @@ func (p PresetSnapshot) handleActiveTemplateVersion() (actions []*Reconciliation
|
||||
return actions, nil
|
||||
}
|
||||
|
||||
// handleInactiveTemplateVersion deletes all running prebuilds except those already being deleted
|
||||
// to avoid duplicate deletion attempts.
|
||||
func (p PresetSnapshot) handleInactiveTemplateVersion() ([]*ReconciliationActions, error) {
|
||||
prebuildsToDelete := len(p.Running)
|
||||
deleteIDs := p.getOldestPrebuildIDs(prebuildsToDelete)
|
||||
// handleInactiveTemplateVersion handles prebuilds from inactive template versions:
|
||||
// 1. If the preset has pending prebuild jobs from an inactive template version, create a cancel reconciliation action.
|
||||
// This cancels all pending prebuild jobs for this preset's template version.
|
||||
// 2. If the preset has prebuilt workspaces currently running from an inactive template version,
|
||||
// create a delete reconciliation action to remove all running prebuilt workspaces.
|
||||
func (p PresetSnapshot) handleInactiveTemplateVersion() (actions []*ReconciliationActions, err error) {
|
||||
// Cancel pending initial prebuild jobs from inactive version
|
||||
if p.PendingCount > 0 {
|
||||
actions = append(actions,
|
||||
&ReconciliationActions{
|
||||
ActionType: ActionTypeCancelPending,
|
||||
})
|
||||
}
|
||||
|
||||
return []*ReconciliationActions{
|
||||
{
|
||||
ActionType: ActionTypeDelete,
|
||||
DeleteIDs: deleteIDs,
|
||||
},
|
||||
}, nil
|
||||
// Delete prebuilds running in inactive version
|
||||
deleteIDs := p.getOldestPrebuildIDs(len(p.Running))
|
||||
if len(deleteIDs) > 0 {
|
||||
actions = append(actions,
|
||||
&ReconciliationActions{
|
||||
ActionType: ActionTypeDelete,
|
||||
DeleteIDs: deleteIDs,
|
||||
})
|
||||
}
|
||||
return actions, nil
|
||||
}
|
||||
|
||||
// needsBackoffPeriod checks if we should delay prebuild creation due to recent failures.
|
||||
|
||||
@@ -6,16 +6,14 @@ import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/coder/quartz"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/database"
|
||||
"github.com/coder/coder/v2/coderd/prebuilds"
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
"github.com/coder/quartz"
|
||||
)
|
||||
|
||||
type options struct {
|
||||
@@ -86,7 +84,7 @@ func TestNoPrebuilds(t *testing.T) {
|
||||
preset(true, 0, current),
|
||||
}
|
||||
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, nil, nil, nil, nil, clock, testutil.Logger(t))
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, nil, nil, nil, nil, nil, clock, testutil.Logger(t))
|
||||
ps, err := snapshot.FilterByPreset(current.presetID)
|
||||
require.NoError(t, err)
|
||||
|
||||
@@ -108,7 +106,7 @@ func TestNetNew(t *testing.T) {
|
||||
preset(true, 1, current),
|
||||
}
|
||||
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, nil, nil, nil, nil, clock, testutil.Logger(t))
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, nil, nil, nil, nil, nil, clock, testutil.Logger(t))
|
||||
ps, err := snapshot.FilterByPreset(current.presetID)
|
||||
require.NoError(t, err)
|
||||
|
||||
@@ -150,7 +148,7 @@ func TestOutdatedPrebuilds(t *testing.T) {
|
||||
var inProgress []database.CountInProgressPrebuildsRow
|
||||
|
||||
// WHEN: calculating the outdated preset's state.
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, nil, quartz.NewMock(t), testutil.Logger(t))
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, nil, nil, quartz.NewMock(t), testutil.Logger(t))
|
||||
ps, err := snapshot.FilterByPreset(outdated.presetID)
|
||||
require.NoError(t, err)
|
||||
|
||||
@@ -216,7 +214,7 @@ func TestDeleteOutdatedPrebuilds(t *testing.T) {
|
||||
}
|
||||
|
||||
// WHEN: calculating the outdated preset's state.
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, nil, quartz.NewMock(t), testutil.Logger(t))
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, nil, nil, quartz.NewMock(t), testutil.Logger(t))
|
||||
ps, err := snapshot.FilterByPreset(outdated.presetID)
|
||||
require.NoError(t, err)
|
||||
|
||||
@@ -238,6 +236,74 @@ func TestDeleteOutdatedPrebuilds(t *testing.T) {
|
||||
}, actions)
|
||||
}
|
||||
|
||||
func TestCancelPendingPrebuilds(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Setup
|
||||
current := opts[optionSet3]
|
||||
clock := quartz.NewMock(t)
|
||||
|
||||
t.Run("CancelPendingPrebuildsNonActiveVersion", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Given: a preset from a non-active version
|
||||
defaultPreset := preset(false, 0, current)
|
||||
presets := []database.GetTemplatePresetsWithPrebuildsRow{
|
||||
defaultPreset,
|
||||
}
|
||||
|
||||
// Given: 2 pending prebuilt workspaces for the preset
|
||||
pending := []database.CountPendingNonActivePrebuildsRow{{
|
||||
PresetID: uuid.NullUUID{
|
||||
UUID: defaultPreset.ID,
|
||||
Valid: true,
|
||||
},
|
||||
Count: 2,
|
||||
}}
|
||||
|
||||
// When: calculating the current preset's state
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, nil, nil, pending, nil, nil, clock, testutil.Logger(t))
|
||||
ps, err := snapshot.FilterByPreset(current.presetID)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Then: it should create a cancel reconciliation action
|
||||
actions, err := ps.CalculateActions(backoffInterval)
|
||||
require.NoError(t, err)
|
||||
expectedAction := []*prebuilds.ReconciliationActions{{ActionType: prebuilds.ActionTypeCancelPending}}
|
||||
require.Equal(t, expectedAction, actions)
|
||||
})
|
||||
|
||||
t.Run("NotCancelPendingPrebuildsActiveVersion", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Given: a preset from an active version
|
||||
defaultPreset := preset(true, 0, current)
|
||||
presets := []database.GetTemplatePresetsWithPrebuildsRow{
|
||||
defaultPreset,
|
||||
}
|
||||
|
||||
// Given: 2 pending prebuilt workspaces for the preset
|
||||
pending := []database.CountPendingNonActivePrebuildsRow{{
|
||||
PresetID: uuid.NullUUID{
|
||||
UUID: defaultPreset.ID,
|
||||
Valid: true,
|
||||
},
|
||||
Count: 2,
|
||||
}}
|
||||
|
||||
// When: calculating the current preset's state
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, nil, nil, pending, nil, nil, clock, testutil.Logger(t))
|
||||
ps, err := snapshot.FilterByPreset(current.presetID)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Then: it should not create a cancel reconciliation action
|
||||
actions, err := ps.CalculateActions(backoffInterval)
|
||||
require.NoError(t, err)
|
||||
var expectedAction []*prebuilds.ReconciliationActions
|
||||
require.Equal(t, expectedAction, actions)
|
||||
})
|
||||
}
|
||||
|
||||
// A new template version is created with a preset with prebuilds configured; while a prebuild is provisioning up or down,
|
||||
// the calculated actions should indicate the state correctly.
|
||||
func TestInProgressActions(t *testing.T) {
|
||||
@@ -460,7 +526,7 @@ func TestInProgressActions(t *testing.T) {
|
||||
}
|
||||
|
||||
// WHEN: calculating the current preset's state.
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, nil, quartz.NewMock(t), testutil.Logger(t))
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, nil, nil, quartz.NewMock(t), testutil.Logger(t))
|
||||
ps, err := snapshot.FilterByPreset(current.presetID)
|
||||
require.NoError(t, err)
|
||||
|
||||
@@ -503,7 +569,7 @@ func TestExtraneous(t *testing.T) {
|
||||
var inProgress []database.CountInProgressPrebuildsRow
|
||||
|
||||
// WHEN: calculating the current preset's state.
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, nil, quartz.NewMock(t), testutil.Logger(t))
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, nil, nil, quartz.NewMock(t), testutil.Logger(t))
|
||||
ps, err := snapshot.FilterByPreset(current.presetID)
|
||||
require.NoError(t, err)
|
||||
|
||||
@@ -683,7 +749,7 @@ func TestExpiredPrebuilds(t *testing.T) {
|
||||
}
|
||||
|
||||
// WHEN: calculating the current preset's state.
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, nil, nil, nil, clock, testutil.Logger(t))
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, nil, nil, nil, nil, clock, testutil.Logger(t))
|
||||
ps, err := snapshot.FilterByPreset(current.presetID)
|
||||
require.NoError(t, err)
|
||||
|
||||
@@ -719,7 +785,7 @@ func TestDeprecated(t *testing.T) {
|
||||
var inProgress []database.CountInProgressPrebuildsRow
|
||||
|
||||
// WHEN: calculating the current preset's state.
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, nil, quartz.NewMock(t), testutil.Logger(t))
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, nil, nil, quartz.NewMock(t), testutil.Logger(t))
|
||||
ps, err := snapshot.FilterByPreset(current.presetID)
|
||||
require.NoError(t, err)
|
||||
|
||||
@@ -772,7 +838,7 @@ func TestLatestBuildFailed(t *testing.T) {
|
||||
}
|
||||
|
||||
// WHEN: calculating the current preset's state.
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, backoffs, nil, clock, testutil.Logger(t))
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, backoffs, nil, clock, testutil.Logger(t))
|
||||
psCurrent, err := snapshot.FilterByPreset(current.presetID)
|
||||
require.NoError(t, err)
|
||||
|
||||
@@ -865,7 +931,7 @@ func TestMultiplePresetsPerTemplateVersion(t *testing.T) {
|
||||
},
|
||||
}
|
||||
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, nil, inProgress, nil, nil, clock, testutil.Logger(t))
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, nil, inProgress, nil, nil, nil, clock, testutil.Logger(t))
|
||||
|
||||
// Nothing has to be created for preset 1.
|
||||
{
|
||||
@@ -985,7 +1051,7 @@ func TestPrebuildScheduling(t *testing.T) {
|
||||
schedule(presets[1].ID, "* 14-16 * * 1-5", 5),
|
||||
}
|
||||
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, schedules, nil, nil, nil, nil, clock, testutil.Logger(t))
|
||||
snapshot := prebuilds.NewGlobalSnapshot(presets, schedules, nil, nil, nil, nil, nil, clock, testutil.Logger(t))
|
||||
|
||||
// Check 1st preset.
|
||||
{
|
||||
@@ -1093,6 +1159,7 @@ func TestCalculateDesiredInstances(t *testing.T) {
|
||||
nil,
|
||||
nil,
|
||||
nil,
|
||||
0,
|
||||
nil,
|
||||
false,
|
||||
quartz.NewMock(t),
|
||||
|
||||
Reference in New Issue
Block a user