Skip to content

Commit 45e4141

Browse files
mafredrimafredri
committed
chore: Refactor autostart check, do it in Go
1 parent 3a345aa commit 45e4141

File tree

5 files changed

+23
-161
lines changed

5 files changed

+23
-161
lines changed

‎coderd/autobuild/executor/lifecycle_executor.go

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package executor
22

33
import (
44
"context"
5-
"database/sql"
65
"encoding/json"
76
"time"
87

@@ -101,36 +100,41 @@ func (e *Executor) runOnce(t time.Time) Stats {
101100
// NOTE: If a workspace build is created with a given TTL and then the user either
102101
// changes or unsets the TTL, the deadline for the workspace build will not
103102
// have changed. This behavior is as expected per #2229.
104-
eligibleWorkspaces, err := e.db.GetWorkspacesAutostart(e.ctx)
103+
workspaces, err := e.db.GetWorkspaces(e.ctx, database.GetWorkspacesParams{
104+
Deleted: false,
105+
})
105106
if err != nil {
106-
e.log.Error(e.ctx, "get eligible workspaces for autostart or autostop", slog.Error(err))
107+
e.log.Error(e.ctx, "get workspaces for autostart or autostop", slog.Error(err))
107108
return stats
108109
}
109110

111+
var eligibleWorkspaceIDs []uuid.UUID
112+
for _, ws := range workspaces {
113+
if isEligibleForAutoStartStop(ws) {
114+
eligibleWorkspaceIDs = append(eligibleWorkspaceIDs, ws.ID)
115+
}
116+
}
117+
110118
// We only use errgroup here for convenience of API, not for early
111119
// cancellation. This means we only return nil errors in th eg.Go.
112120
eg := errgroup.Group{}
113121
// Limit the concurrency to avoid overloading the database.
114122
eg.SetLimit(10)
115123

116-
for _, ws := range eligibleWorkspaces {
117-
ws := ws
118-
log := e.log.With(slog.F("workspace_id", ws.ID))
124+
for _, wsID := range eligibleWorkspaceIDs {
125+
wsID := wsID
126+
log := e.log.With(slog.F("workspace_id", wsID))
119127

120128
eg.Go(func() error {
121129
err := e.db.InTx(func(db database.Store) error {
122-
var err error
123-
124130
// Re-check eligibility since the first check was outside the
125131
// transaction and the workspace settings may have changed.
126-
ws, err = db.GetWorkspaceAutostart(e.ctx, ws.ID)
132+
ws, err := db.GetWorkspaceByID(e.ctx, wsID)
127133
if err != nil {
128-
// Receiving ErrNoRows means the workspace settings changed
129-
// and it is no longer eligible for autostart. Other errors
130-
// means something went wrong.
131-
if !xerrors.Is(err, sql.ErrNoRows) {
132-
log.Error(e.ctx, "get workspace autostart failed", slog.Error(err))
133-
}
134+
log.Error(e.ctx, "get workspace autostart failed", slog.Error(err))
135+
return nil
136+
}
137+
if !isEligibleForAutoStartStop(ws) {
134138
return nil
135139
}
136140

@@ -191,6 +195,10 @@ func (e *Executor) runOnce(t time.Time) Stats {
191195
return stats
192196
}
193197

198+
func isEligibleForAutoStartStop(ws database.Workspace) bool {
199+
return ws.AutostartSchedule.String != "" || ws.Ttl.Int64 > 0
200+
}
201+
194202
func getNextTransition(
195203
ws database.Workspace,
196204
priorHistory database.WorkspaceBuild,

‎coderd/database/databasefake/databasefake.go

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -588,33 +588,6 @@ func (q *fakeQuerier) GetWorkspaceAppsByAgentIDs(_ context.Context, ids []uuid.U
588588
return apps, nil
589589
}
590590

591-
func (q *fakeQuerier) GetWorkspaceAutostart(_ context.Context, workspaceID uuid.UUID) (database.Workspace, error) {
592-
q.mutex.RLock()
593-
defer q.mutex.RUnlock()
594-
for _, ws := range q.workspaces {
595-
if ws.ID == workspaceID {
596-
if ws.AutostartSchedule.String != "" || ws.Ttl.Valid {
597-
return ws, nil
598-
}
599-
}
600-
}
601-
return database.Workspace{}, sql.ErrNoRows
602-
}
603-
604-
func (q *fakeQuerier) GetWorkspacesAutostart(_ context.Context) ([]database.Workspace, error) {
605-
q.mutex.RLock()
606-
defer q.mutex.RUnlock()
607-
workspaces := make([]database.Workspace, 0)
608-
for _, ws := range q.workspaces {
609-
if ws.AutostartSchedule.String != "" {
610-
workspaces = append(workspaces, ws)
611-
} else if ws.Ttl.Valid {
612-
workspaces = append(workspaces, ws)
613-
}
614-
}
615-
return workspaces, nil
616-
}
617-
618591
func (q *fakeQuerier) GetWorkspaceOwnerCountsByTemplateIDs(_ context.Context, templateIDs []uuid.UUID) ([]database.GetWorkspaceOwnerCountsByTemplateIDsRow, error) {
619592
q.mutex.RLock()
620593
defer q.mutex.RUnlock()

‎coderd/database/querier.go

Lines changed: 0 additions & 2 deletions
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: 0 additions & 86 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: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -50,37 +50,6 @@ WHERE
5050
END
5151
;
5252

53-
-- name: GetWorkspaceAutostart :one
54-
SELECT
55-
*
56-
FROM
57-
workspaces
58-
WHERE
59-
deleted = false
60-
AND
61-
(
62-
id = @workspace_id
63-
AND (
64-
(autostart_schedule IS NOT NULL AND autostart_schedule <> '')
65-
OR
66-
(ttl IS NOT NULL AND ttl > 0)
67-
)
68-
);
69-
70-
-- name: GetWorkspacesAutostart :many
71-
SELECT
72-
*
73-
FROM
74-
workspaces
75-
WHERE
76-
deleted = false
77-
AND
78-
(
79-
(autostart_schedule IS NOT NULL AND autostart_schedule <> '')
80-
OR
81-
(ttl IS NOT NULL AND ttl > 0)
82-
);
83-
8453
-- name: GetWorkspaceByOwnerIDAndName :one
8554
SELECT
8655
*

0 commit comments

Comments
 (0)