Skip to content

Commit fb857e0

Browse files
DanielleMaywoodDanielleMaywood
committed
fix: rewrite GetWorkspacesEligibleForTransition query
1 parent cee6b1e commit fb857e0

File tree

7 files changed

+128
-95
lines changed

7 files changed

+128
-95
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2801,7 +2801,7 @@ func (q *querier) GetWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerID u
28012801
return q.db.GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx, ownerID, prep)
28022802
}
28032803

2804-
func (q *querier) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]database.WorkspaceTable, error) {
2804+
func (q *querier) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]database.GetWorkspacesEligibleForTransitionRow, error) {
28052805
return q.db.GetWorkspacesEligibleForTransition(ctx, now)
28062806
}
28072807

‎coderd/database/dbmem/dbmem.go

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6868,11 +6868,11 @@ func (q *FakeQuerier) GetWorkspacesAndAgentsByOwnerID(ctx context.Context, owner
68686868
return q.GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx, ownerID, nil)
68696869
}
68706870

6871-
func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]database.WorkspaceTable, error) {
6871+
func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]database.GetWorkspacesEligibleForTransitionRow, error) {
68726872
q.mutex.RLock()
68736873
defer q.mutex.RUnlock()
68746874

6875-
workspaces := []database.WorkspaceTable{}
6875+
workspaces := []database.GetWorkspacesEligibleForTransitionRow{}
68766876
for _, workspace := range q.workspaces {
68776877
build, err := q.getLatestWorkspaceBuildByWorkspaceIDNoLock(ctx, workspace.ID)
68786878
if err != nil {
@@ -6883,14 +6883,20 @@ func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, no
68836883
!build.Deadline.IsZero() &&
68846884
build.Deadline.Before(now) &&
68856885
!workspace.DormantAt.Valid {
6886-
workspaces = append(workspaces, workspace)
6886+
workspaces = append(workspaces, database.GetWorkspacesEligibleForTransitionRow{
6887+
ID: workspace.ID,
6888+
Name: workspace.Name,
6889+
})
68876890
continue
68886891
}
68896892

68906893
if build.Transition == database.WorkspaceTransitionStop &&
68916894
workspace.AutostartSchedule.Valid &&
68926895
!workspace.DormantAt.Valid {
6893-
workspaces = append(workspaces, workspace)
6896+
workspaces = append(workspaces, database.GetWorkspacesEligibleForTransitionRow{
6897+
ID: workspace.ID,
6898+
Name: workspace.Name,
6899+
})
68946900
continue
68956901
}
68966902

@@ -6899,7 +6905,10 @@ func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, no
68996905
return nil, xerrors.Errorf("get provisioner job by ID: %w", err)
69006906
}
69016907
if codersdk.ProvisionerJobStatus(job.JobStatus) == codersdk.ProvisionerJobFailed {
6902-
workspaces = append(workspaces, workspace)
6908+
workspaces = append(workspaces, database.GetWorkspacesEligibleForTransitionRow{
6909+
ID: workspace.ID,
6910+
Name: workspace.Name,
6911+
})
69036912
continue
69046913
}
69056914

@@ -6908,11 +6917,17 @@ func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, no
69086917
return nil, xerrors.Errorf("get template by ID: %w", err)
69096918
}
69106919
if !workspace.DormantAt.Valid && template.TimeTilDormant > 0 {
6911-
workspaces = append(workspaces, workspace)
6920+
workspaces = append(workspaces, database.GetWorkspacesEligibleForTransitionRow{
6921+
ID: workspace.ID,
6922+
Name: workspace.Name,
6923+
})
69126924
continue
69136925
}
69146926
if workspace.DormantAt.Valid && template.TimeTilDormantAutoDelete > 0 {
6915-
workspaces = append(workspaces, workspace)
6927+
workspaces = append(workspaces, database.GetWorkspacesEligibleForTransitionRow{
6928+
ID: workspace.ID,
6929+
Name: workspace.Name,
6930+
})
69166931
continue
69176932
}
69186933

@@ -6921,7 +6936,10 @@ func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, no
69216936
return nil, xerrors.Errorf("get user by ID: %w", err)
69226937
}
69236938
if user.Status == database.UserStatusSuspended && build.Transition == database.WorkspaceTransitionStart {
6924-
workspaces = append(workspaces, workspace)
6939+
workspaces = append(workspaces, database.GetWorkspacesEligibleForTransitionRow{
6940+
ID: workspace.ID,
6941+
Name: workspace.Name,
6942+
})
69256943
continue
69266944
}
69276945
}

‎coderd/database/dbmetrics/querymetrics.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/querier.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 53 additions & 50 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/workspaces.sql

Lines changed: 44 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,8 @@ FROM pending_workspaces, building_workspaces, running_workspaces, failed_workspa
557557

558558
-- name: GetWorkspacesEligibleForTransition :many
559559
SELECT
560-
workspaces.*
560+
workspaces.id,
561+
workspaces.name
561562
FROM
562563
workspaces
563564
LEFT JOIN
@@ -579,52 +580,65 @@ WHERE
579580
) AND
580581

581582
(
582-
-- If the workspace build was a start transition, the workspace is
583-
-- potentially eligible for autostop if it's past the deadline. The
584-
-- deadline is computed at build time upon success and is bumped based
585-
-- on activity (up the max deadline if set). We don't need to check
586-
-- license here since that's done when the values are written to the build.
583+
-- isEligibleForAutostop
587584
(
588-
workspace_builds.transition = 'start'::workspace_transition AND
589-
workspace_builds.deadline IS NOT NULL AND
590-
workspace_builds.deadline < @now :: timestamptz
585+
provisioner_jobs.job_status != 'failed'::provisioner_job_status AND
586+
workspaces.dormant_at IS NULL AND
587+
workspace_builds.transition = 'start'::workspace_transition AND (
588+
users.status = 'suspended'::user_status OR (
589+
workspace_builds.deadline != '0001-01-01 00:00:00+00'::timestamp AND
590+
workspace_builds.deadline < @now :: timestamptz
591+
)
592+
)
591593
) OR
592594

593-
-- If the workspace build was a stop transition, the workspace is
594-
-- potentially eligible for autostart if it has a schedule set. The
595-
-- caller must check if the template allows autostart in a license-aware
596-
-- fashion as we cannot check it here.
595+
-- isEligibleForAutostart
597596
(
597+
users.status = 'active'::user_status AND
598+
provisioner_jobs.job_status != 'failed'::provisioner_job_status AND
598599
workspace_builds.transition = 'stop'::workspace_transition AND
599600
workspaces.autostart_schedule IS NOT NULL
600601
) OR
601602

602-
-- If the workspace's most recent job resulted in an error
603-
-- it may be eligible for failed stop.
604-
(
605-
provisioner_jobs.error IS NOT NULL AND
606-
provisioner_jobs.error != '' AND
607-
workspace_builds.transition = 'start'::workspace_transition
608-
) OR
609-
610-
-- If the workspace's template has an inactivity_ttl set
611-
-- it may be eligible for dormancy.
603+
-- isEligibleForDormantStop
612604
(
605+
workspaces.dormant_at IS NULL AND
613606
templates.time_til_dormant > 0 AND
614-
workspaces.dormant_at IS NULL
607+
(@now :: timestamptz) - workspaces.last_used_at > (INTERVAL '1 millisecond' * (templates.time_til_dormant / 1000000))
615608
) OR
616609

617-
-- If the workspace's template has a time_til_dormant_autodelete set
618-
-- and the workspace is already dormant.
610+
-- isEligibleForDelete
619611
(
612+
workspaces.dormant_at IS NOT NULL AND
613+
workspaces.deleting_at IS NOT NULL AND
614+
workspaces.deleting_at < @now :: timestamptz AND
620615
templates.time_til_dormant_autodelete > 0 AND
621-
workspaces.dormant_at IS NOT NULL
616+
CASE
617+
WHEN (
618+
workspace_builds.transition = 'delete'::workspace_transition AND
619+
provisioner_jobs.job_status = 'failed'::provisioner_job_status
620+
) THEN (
621+
(
622+
provisioner_jobs.canceled_at IS NOT NULL OR
623+
provisioner_jobs.completed_at IS NOT NULL
624+
) AND (
625+
(@now :: timestamptz) - (CASE
626+
WHEN provisioner_jobs.canceled_at IS NOT NULL THEN provisioner_jobs.canceled_at
627+
ELSE provisioner_jobs.completed_at
628+
END) > INTERVAL '24 hours'
629+
)
630+
)
631+
ELSE true
632+
END
622633
) OR
623634

624-
-- If the user account is suspended, and the workspace is running.
635+
-- isEligibleForFailedStop
625636
(
626-
users.status = 'suspended'::user_status AND
627-
workspace_builds.transition = 'start'::workspace_transition
637+
templates.failure_ttl > 0 AND
638+
provisioner_jobs.job_status = 'failed'::provisioner_job_status AND
639+
workspace_builds.transition = 'start'::workspace_transition AND
640+
provisioner_jobs.completed_at IS NOT NULL AND
641+
(@now :: timestamptz) - provisioner_jobs.completed_at > (INTERVAL '1 millisecond' * (templates.failure_ttl / 1000000))
628642
)
629643
) AND workspaces.deleted = 'false';
630644

@@ -727,5 +741,3 @@ WHERE
727741
-- Authorize Filter clause will be injected below in GetAuthorizedWorkspacesAndAgentsByOwnerID
728742
-- @authorize_filter
729743
GROUP BY workspaces.id, workspaces.name, latest_build.job_status, latest_build.job_id, latest_build.transition;
730-
731-

0 commit comments

Comments
 (0)