Apr 17, 2025 · Mar 12, 2025 · Mar 13, 2025 · Mar 13, 2025 · Mar 13, 2025 · Mar 13, 2025
diff --git a/coderd/database/lock.go b/coderd/database/lock.go
 LockIDDBPurge
 LockIDNotificationsReportGenerator
 LockIDCryptoKeyRotation
 LockIDReconcileTemplatePrebuilds
 LockIDDeterminePrebuildsState
 LockIDReconcilePrebuilds
 )

 // GenLockID generates a unique and consistent lock ID from a given string.
diff --git a/coderd/database/querier.go b/coderd/database/querier.go
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/prebuilds.sql b/coderd/database/queries/prebuilds.sql
 AND b.job_status = 'succeeded'::provisioner_job_status);

 -- name: CountInProgressPrebuilds :many
 -- CountInProgressPrebuilds returns the number of in-progress prebuilds, grouped bytemplate version ID and transition.
 -- CountInProgressPrebuilds returns the number of in-progress prebuilds, grouped bypreset ID and transition.
 -- Prebuild considered in-progress if it's in the "starting", "stopping", or "deleting" state.
 SELECT t.id AS template_id, wpb.template_version_id, wpb.transition, COUNT(wpb.transition)::int AS count
 SELECT t.id AS template_id, wpb.template_version_id, wpb.transition, COUNT(wpb.transition)::int AS count, wlb.template_version_preset_id as preset_id
 FROM workspace_latest_builds wlb
 INNER JOIN workspace_prebuild_builds wpb ON wpb.id = wlb.id
 -- We only need these counts for active template versions.
 -- prebuilds that are still building.
 INNER JOIN templates t ON t.active_version_id = wlb.template_version_id
 WHERE wlb.job_status IN ('pending'::provisioner_job_status, 'running'::provisioner_job_status)
 GROUP BY t.id, wpb.template_version_id, wpb.transition;
 GROUP BY t.id, wpb.template_version_id, wpb.transition, wlb.template_version_preset_id;

 -- GetPresetsBackoff groups workspace builds by preset ID.
 -- Each preset is associated with exactly one template version ID.
diff --git a/coderd/prebuilds/api.go b/coderd/prebuilds/api.go
 package prebuilds

 import (
 "context"
 )

 // ReconciliationOrchestrator manages the lifecycle of prebuild reconciliation.
 // It runs a continuous loop to check and reconcile prebuild states, and can be stopped gracefully.
 type ReconciliationOrchestrator interface {
 Reconciler

 // RunLoop starts a continuous reconciliation loop that periodically calls ReconcileAll
 // to ensure all prebuilds are in their desired states. The loop runs until the context
 // is canceled or Stop is called.
 RunLoop(ctx context.Context)

 // Stop gracefully shuts down the orchestrator with the given cause.
 // The cause is used for logging and error reporting.
 Stop(ctx context.Context, cause error)
 }

 type Reconciler interface {
 // ReconcileAll orchestrates the reconciliation of all prebuilds across all templates.
 // It takes a global snapshot of the system state and then reconciles each preset
 // in parallel, creating or deleting prebuilds as needed to reach their desired states.
 ReconcileAll(ctx context.Context) error
 }
diff --git a/coderd/prebuilds/global_snapshot.go b/coderd/prebuilds/global_snapshot.go
 package prebuilds

 import (
 "github.com/google/uuid"
 "golang.org/x/xerrors"

 "github.com/coder/coder/v2/coderd/database"
 "github.com/coder/coder/v2/coderd/util/slice"
 )

 // GlobalSnapshot represents a full point-in-time snapshot of state relating to prebuilds across all templates.
 type GlobalSnapshot struct {
 Presets             []database.GetTemplatePresetsWithPrebuildsRow
 RunningPrebuilds    []database.GetRunningPrebuiltWorkspacesRow
 PrebuildsInProgress []database.CountInProgressPrebuildsRow
 Backoffs            []database.GetPresetsBackoffRow
 }

 func NewGlobalSnapshot(
 presets []database.GetTemplatePresetsWithPrebuildsRow,
 runningPrebuilds []database.GetRunningPrebuiltWorkspacesRow,
 prebuildsInProgress []database.CountInProgressPrebuildsRow,
 backoffs []database.GetPresetsBackoffRow,
 ) GlobalSnapshot {
 return GlobalSnapshot{
 Presets:             presets,
 RunningPrebuilds:    runningPrebuilds,
 PrebuildsInProgress: prebuildsInProgress,
 Backoffs:            backoffs,
 }
 }

 func (s GlobalSnapshot) FilterByPreset(presetID uuid.UUID) (*PresetSnapshot, error) {
 preset, found := slice.Find(s.Presets, func(preset database.GetTemplatePresetsWithPrebuildsRow) bool {
 return preset.ID == presetID
 })
 if !found {
 return nil, xerrors.Errorf("no preset found with ID %q", presetID)
 }

 running := slice.Filter(s.RunningPrebuilds, func(prebuild database.GetRunningPrebuiltWorkspacesRow) bool {
 if !prebuild.CurrentPresetID.Valid {
 return false
 }
 return prebuild.CurrentPresetID.UUID == preset.ID
 })

 inProgress := slice.Filter(s.PrebuildsInProgress, func(prebuild database.CountInProgressPrebuildsRow) bool {
 return prebuild.PresetID.UUID == preset.ID
 })

 var backoffPtr *database.GetPresetsBackoffRow
 backoff, found := slice.Find(s.Backoffs, func(row database.GetPresetsBackoffRow) bool {
 return row.PresetID == preset.ID
 })
 if found {
 backoffPtr = &backoff
 }

 return &PresetSnapshot{
 Preset:     preset,
 Running:    running,
 InProgress: inProgress,
 Backoff:    backoffPtr,
 }, nil
 }
diff --git a/coderd/prebuilds/noop.go b/coderd/prebuilds/noop.go
 package prebuilds

 import (
 "context"

 "github.com/coder/coder/v2/coderd/database"
 )

 type NoopReconciler struct{}

 func NewNoopReconciler() *NoopReconciler {
 return &NoopReconciler{}
 }

 func (NoopReconciler) RunLoop(context.Context) {}

 func (NoopReconciler) Stop(context.Context, error) {}

 func (NoopReconciler) ReconcileAll(context.Context) error {
 return nil
 }

 func (NoopReconciler) SnapshotState(context.Context, database.Store) (*GlobalSnapshot, error) {
 return &GlobalSnapshot{}, nil
 }

 func (NoopReconciler) ReconcilePreset(context.Context, PresetSnapshot) error {
 return nil
 }

 func (NoopReconciler) CalculateActions(context.Context, PresetSnapshot) (*ReconciliationActions, error) {
 return &ReconciliationActions{}, nil
 }

 var _ ReconciliationOrchestrator = NoopReconciler{}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -12,8 +12,7 @@ const (
		LockIDDBPurge
		LockIDNotificationsReportGenerator
		LockIDCryptoKeyRotation
		LockIDReconcileTemplatePrebuilds
		LockIDDeterminePrebuildsState
		LockIDReconcilePrebuilds
		)

		// GenLockID generates a unique and consistent lock ID from a given string.
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -57,9 +57,9 @@ WHERE (b.transition = 'start'::workspace_transition
		AND b.job_status = 'succeeded'::provisioner_job_status);

		-- name: CountInProgressPrebuilds :many
		-- CountInProgressPrebuilds returns the number of in-progress prebuilds, grouped bytemplate version ID and transition.
		-- CountInProgressPrebuilds returns the number of in-progress prebuilds, grouped bypreset ID and transition.
		-- Prebuild considered in-progress if it's in the "starting", "stopping", or "deleting" state.
		SELECT t.id AS template_id, wpb.template_version_id, wpb.transition, COUNT(wpb.transition)::int AS count
		SELECT t.id AS template_id, wpb.template_version_id, wpb.transition, COUNT(wpb.transition)::int AS count, wlb.template_version_preset_id as preset_id
		FROM workspace_latest_builds wlb
		INNER JOIN workspace_prebuild_builds wpb ON wpb.id = wlb.id
		-- We only need these counts for active template versions.
Expand All		@@ -70,7 +70,7 @@ FROM workspace_latest_builds wlb
		-- prebuilds that are still building.
		INNER JOIN templates t ON t.active_version_id = wlb.template_version_id
		WHERE wlb.job_status IN ('pending'::provisioner_job_status, 'running'::provisioner_job_status)
		GROUP BY t.id, wpb.template_version_id, wpb.transition;
		GROUP BY t.id, wpb.template_version_id, wpb.transition, wlb.template_version_preset_id;

		-- GetPresetsBackoff groups workspace builds by preset ID.
		-- Each preset is associated with exactly one template version ID.
Expand Down
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,27 @@
		package prebuilds
Copy link Member mtojekApr 15, 2025 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I didn't catch it, but why is`prebuilds` located in`coderd`? Are there any components or interfaces used in non-commercial license mode? What I can see here are mainly interfaces and snapshots. Copy link ContributorAuthor evgeniy-scherbinaApr 15, 2025• edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I guess the idea is we have all interfaces and related types defined in`coderd`. Consequently we have to define methods in the same package. IIRC in goalng you can't define methods outside the package. @dannykopping should know more Copy link Member mtojekApr 16, 2025• edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I guess the idea is we have all interfaces and related types defined in coderd Ok, but since`prebuilds` will be enterprise only, could we just park the entire implementation there? Copy link Contributor dannykoppingApr 16, 2025 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. We'll need at least some of the API in the AGPL code because we need the noop implementation "running" until a premium license is used. Other than that, I think most of this can shift to enterprise. I believe I had it this way because AGPL cannot import enterprise but the reverse is allowed. The interfaces reference the types which are in this package, and if we moved those to enterprise then it would violate the license. This is how the implementation will be swapped out at runtime based on the license: https://github.com/coder/coder/blob/dk/prebuilds/enterprise/coderd/coderd.go#L1165-L1188 johnstcn reacted with thumbs up emoji

		import (
		"context"
		)

		// ReconciliationOrchestrator manages the lifecycle of prebuild reconciliation.
		// It runs a continuous loop to check and reconcile prebuild states, and can be stopped gracefully.
		type ReconciliationOrchestrator interface {
evgeniy-scherbina marked this conversation as resolved. Show resolvedHide resolved
		Reconciler

		// RunLoop starts a continuous reconciliation loop that periodically calls ReconcileAll
		// to ensure all prebuilds are in their desired states. The loop runs until the context
		// is canceled or Stop is called.
		RunLoop(ctx context.Context)

		// Stop gracefully shuts down the orchestrator with the given cause.
		// The cause is used for logging and error reporting.
		Stop(ctx context.Context, cause error)
		}

		type Reconciler interface {
Copy link ContributorAuthor evgeniy-scherbinaApr 8, 2025• edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. The current API feels too low-level. Ideally, we should expose higher-level methods such as: `ReconcileAll()` `ReconcilePreset(preset)` Note: `SnapshotState` is currently used only by`MetricsCollector`. `CalculateActions` is currently unused. Copy link Contributor spikecurtisApr 9, 2025 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I agree with this. Copy link Contributor dannykoppingApr 9, 2025 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. It's been a couple weeks but I believe we only expose these for tests. Copy link Contributor spikecurtisApr 9, 2025 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Maybe make it an internal interface and internal test? Copy link ContributorAuthor evgeniy-scherbinaApr 9, 2025 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I added a detailed comments, explaining how this API can be used, and why it may sense to expose all these API. Copy link ContributorAuthor evgeniy-scherbinaApr 15, 2025 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. @johnstcn I also wanted to do it, but: they will be used in`MetricsCollector`:DO NOT REVIEW/MERGE #16523 they are used for tests So removing them means: I have to find workaround for tests later we have to find workaround for metrics collector Copy link Contributor spikecurtisApr 15, 2025 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. There is no need for this interface to be exported for the metrics collector to work correctly, since it lives in the same package as the`StoreReconciler`. Tests can be inside the same package if they need to access unexported functions. Also, you don't need to export an interface to call exported functions on, say, the`StoreReconciler`. Copy link Contributor dannykoppingApr 16, 2025 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Can we do this refactor in a follow-up? Let's try keep the momentum on this PR. Copy link Contributor spikecurtisApr 17, 2025• edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I don't agree that "momentum" should be a reason to check in code that doesn't meet our standards. What is to be gained by fixing it later rather than now? IMO it just makes it more likely that we don't fix the issue at all. Also, this interface isn't actually used anywhere within this PR, so fixing it within the scope of this PR should be trivial. Copy link ContributorAuthor evgeniy-scherbinaApr 17, 2025• edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. @spikecurtis I reduce number of methods in`Reconciler` interface. Now it looks like this: typeReconcilerinterface {// ReconcileAll orchestrates the reconciliation of all prebuilds across all templates.// It takes a global snapshot of the system state and then reconciles each preset// in parallel, creating or deleting prebuilds as needed to reach their desired states.ReconcileAll(ctx context.Context)error} In future PRs maybe we'll need additional methods, but we can deal with it when we'll be working on those PRs. Also we'll have more information at that point. cc@dannykopping
		// ReconcileAll orchestrates the reconciliation of all prebuilds across all templates.
		// It takes a global snapshot of the system state and then reconciles each preset
		// in parallel, creating or deleting prebuilds as needed to reach their desired states.
		ReconcileAll(ctx context.Context) error
		}
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,66 @@
		package prebuilds

		import (
		"github.com/google/uuid"
		"golang.org/x/xerrors"

		"github.com/coder/coder/v2/coderd/database"
		"github.com/coder/coder/v2/coderd/util/slice"
		)

		// GlobalSnapshot represents a full point-in-time snapshot of state relating to prebuilds across all templates.
		type GlobalSnapshot struct {
		Presets []database.GetTemplatePresetsWithPrebuildsRow
		RunningPrebuilds []database.GetRunningPrebuiltWorkspacesRow
		PrebuildsInProgress []database.CountInProgressPrebuildsRow
		Backoffs []database.GetPresetsBackoffRow
		}

		func NewGlobalSnapshot(
		presets []database.GetTemplatePresetsWithPrebuildsRow,
		runningPrebuilds []database.GetRunningPrebuiltWorkspacesRow,
		prebuildsInProgress []database.CountInProgressPrebuildsRow,
		backoffs []database.GetPresetsBackoffRow,
		) GlobalSnapshot {
		return GlobalSnapshot{
		Presets: presets,
		RunningPrebuilds: runningPrebuilds,
		PrebuildsInProgress: prebuildsInProgress,
		Backoffs: backoffs,
		}
		}
Comment on lines +19 to +31 Copy link Member johnstcnApr 15, 2025 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. nit: this could probably be removed; I only see it used in one other place and it doesn't seem to provide any ergonomic benefits over`snap := GlobalSnapshot{...}`. Copy link ContributorAuthor evgeniy-scherbinaApr 15, 2025 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Generally I like constructors in Go, because if later we'll add new field to`GlobalSnapshot`, we can update constructor and easily track and update all object creations. We can add validation inside constructor, etc...

		func (s GlobalSnapshot) FilterByPreset(presetID uuid.UUID) (*PresetSnapshot, error) {
		preset, found := slice.Find(s.Presets, func(preset database.GetTemplatePresetsWithPrebuildsRow) bool {
		return preset.ID == presetID
		})
		if !found {
		return nil, xerrors.Errorf("no preset found with ID %q", presetID)
		}

		running := slice.Filter(s.RunningPrebuilds, func(prebuild database.GetRunningPrebuiltWorkspacesRow) bool {
		if !prebuild.CurrentPresetID.Valid {
		return false
		}
		return prebuild.CurrentPresetID.UUID == preset.ID
		})

		inProgress := slice.Filter(s.PrebuildsInProgress, func(prebuild database.CountInProgressPrebuildsRow) bool {
		return prebuild.PresetID.UUID == preset.ID
		})

		var backoffPtr *database.GetPresetsBackoffRow
		backoff, found := slice.Find(s.Backoffs, func(row database.GetPresetsBackoffRow) bool {
		return row.PresetID == preset.ID
		})
		if found {
		backoffPtr = &backoff
		}

		return &PresetSnapshot{
		Preset: preset,
		Running: running,
		InProgress: inProgress,
		Backoff: backoffPtr,
		}, nil
		}
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,35 @@
		package prebuilds
evgeniy-scherbina marked this conversation as resolved. Show resolvedHide resolved

		import (
		"context"

		"github.com/coder/coder/v2/coderd/database"
		)

		type NoopReconciler struct{}

		func NewNoopReconciler() *NoopReconciler {
		return &NoopReconciler{}
		}

		func (NoopReconciler) RunLoop(context.Context) {}

		func (NoopReconciler) Stop(context.Context, error) {}
evgeniy-scherbina marked this conversation as resolved. Show resolvedHide resolved

		func (NoopReconciler) ReconcileAll(context.Context) error {
		return nil
		}

		func (NoopReconciler) SnapshotState(context.Context, database.Store) (*GlobalSnapshot, error) {
		return &GlobalSnapshot{}, nil
		}

		func (NoopReconciler) ReconcilePreset(context.Context, PresetSnapshot) error {
		return nil
		}

		func (NoopReconciler) CalculateActions(context.Context, PresetSnapshot) (*ReconciliationActions, error) {
		return &ReconciliationActions{}, nil
		}

		var _ ReconciliationOrchestrator = NoopReconciler{}