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

Commite21a301

Browse files
fix: make GetWorkspacesEligibleForTransition return even less false positives (#15594)
Relates to#15082Further to#15429, this reduces theamount of false-positives returned by the 'is eligible for autostart'part of the query. We achieve this by calculating the 'next start at'time of the workspace, storing it in the database, and using it in our`GetWorkspacesEligibleForTransition` query.The prior implementation of the 'is eligible for autostart' query wouldreturn _all_ workspaces that at some point in the future _might_ beeligible for autostart. This now ensures we only return workspaces that_should_ be eligible for autostart.We also now pass `currentTick` instead of `t` to the`GetWorkspacesEligibleForTransition` query as otherwise we'll have oneround of workspaces that are skipped by `isEligibleForTransition` due to`currentTick` being a truncated version of `t`.
1 parent2b57dcc commite21a301

File tree

35 files changed

+1012
-75
lines changed

35 files changed

+1012
-75
lines changed

‎cli/testdata/coder_list_--output_json.golden

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
},
6666
"automatic_updates": "never",
6767
"allow_renames": false,
68-
"favorite": false
68+
"favorite": false,
69+
"next_start_at": "[timestamp]"
6970
}
7071
]

‎coderd/apidoc/docs.go

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/autobuild/lifecycle_executor.go

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ func (e *Executor) runOnce(t time.Time) Stats {
142142
// NOTE: If a workspace build is created with a given TTL and then the user either
143143
// changes or unsets the TTL, the deadline for the workspace build will not
144144
// have changed. This behavior is as expected per #2229.
145-
workspaces,err:=e.db.GetWorkspacesEligibleForTransition(e.ctx,t)
145+
workspaces,err:=e.db.GetWorkspacesEligibleForTransition(e.ctx,currentTick)
146146
iferr!=nil {
147147
e.log.Error(e.ctx,"get workspaces for autostart or autostop",slog.Error(err))
148148
returnstats
@@ -205,6 +205,23 @@ func (e *Executor) runOnce(t time.Time) Stats {
205205
returnxerrors.Errorf("get template scheduling options: %w",err)
206206
}
207207

208+
// If next start at is not valid we need to re-compute it
209+
if!ws.NextStartAt.Valid&&ws.AutostartSchedule.Valid {
210+
next,err:=schedule.NextAllowedAutostart(currentTick,ws.AutostartSchedule.String,templateSchedule)
211+
iferr==nil {
212+
nextStartAt:= sql.NullTime{Valid:true,Time:dbtime.Time(next.UTC())}
213+
iferr=tx.UpdateWorkspaceNextStartAt(e.ctx, database.UpdateWorkspaceNextStartAtParams{
214+
ID:wsID,
215+
NextStartAt:nextStartAt,
216+
});err!=nil {
217+
returnxerrors.Errorf("update workspace next start at: %w",err)
218+
}
219+
220+
// Save re-fetching the workspace
221+
ws.NextStartAt=nextStartAt
222+
}
223+
}
224+
208225
tmpl,err=tx.GetTemplateByID(e.ctx,ws.TemplateID)
209226
iferr!=nil {
210227
returnxerrors.Errorf("get template by ID: %w",err)
@@ -463,8 +480,8 @@ func isEligibleForAutostart(user database.User, ws database.Workspace, build dat
463480
returnfalse
464481
}
465482

466-
nextTransition,allowed:=schedule.NextAutostart(build.CreatedAt,ws.AutostartSchedule.String,templateSchedule)
467-
if!allowed {
483+
nextTransition,err:=schedule.NextAllowedAutostart(build.CreatedAt,ws.AutostartSchedule.String,templateSchedule)
484+
iferr!=nil {
468485
returnfalse
469486
}
470487

‎coderd/autobuild/lifecycle_executor_test.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1083,6 +1083,10 @@ func TestNotifications(t *testing.T) {
10831083
IncludeProvisionerDaemon:true,
10841084
NotificationsEnqueuer:&notifyEnq,
10851085
TemplateScheduleStore: schedule.MockTemplateScheduleStore{
1086+
SetFn:func(ctx context.Context,db database.Store,template database.Template,options schedule.TemplateScheduleOptions) (database.Template,error) {
1087+
template.TimeTilDormant=int64(options.TimeTilDormant)
1088+
returnschedule.NewAGPLTemplateScheduleStore().Set(ctx,db,template,options)
1089+
},
10861090
GetFn:func(_ context.Context,_ database.Store,_ uuid.UUID) (schedule.TemplateScheduleOptions,error) {
10871091
return schedule.TemplateScheduleOptions{
10881092
UserAutostartEnabled:false,
@@ -1099,7 +1103,9 @@ func TestNotifications(t *testing.T) {
10991103
)
11001104

11011105
coderdtest.AwaitTemplateVersionJobCompleted(t,client,version.ID)
1102-
template:=coderdtest.CreateTemplate(t,client,admin.OrganizationID,version.ID)
1106+
template:=coderdtest.CreateTemplate(t,client,admin.OrganizationID,version.ID,func(ctr*codersdk.CreateTemplateRequest) {
1107+
ctr.TimeTilDormantMillis=ptr.Ref(timeTilDormant.Milliseconds())
1108+
})
11031109
userClient,_:=coderdtest.CreateAnotherUser(t,client,admin.OrganizationID)
11041110
workspace:=coderdtest.CreateWorkspace(t,userClient,template.ID)
11051111
coderdtest.AwaitWorkspaceBuildJobCompleted(t,userClient,workspace.LatestBuild.ID)

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1053,6 +1053,13 @@ func (q *querier) BatchUpdateWorkspaceLastUsedAt(ctx context.Context, arg databa
10531053
returnq.db.BatchUpdateWorkspaceLastUsedAt(ctx,arg)
10541054
}
10551055

1056+
func (q*querier)BatchUpdateWorkspaceNextStartAt(ctx context.Context,arg database.BatchUpdateWorkspaceNextStartAtParams)error {
1057+
iferr:=q.authorizeContext(ctx,policy.ActionUpdate,rbac.ResourceWorkspace.All());err!=nil {
1058+
returnerr
1059+
}
1060+
returnq.db.BatchUpdateWorkspaceNextStartAt(ctx,arg)
1061+
}
1062+
10561063
func (q*querier)BulkMarkNotificationMessagesFailed(ctx context.Context,arg database.BulkMarkNotificationMessagesFailedParams) (int64,error) {
10571064
iferr:=q.authorizeContext(ctx,policy.ActionUpdate,rbac.ResourceNotificationMessage);err!=nil {
10581065
return0,err
@@ -2840,6 +2847,13 @@ func (q *querier) GetWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerID u
28402847
returnq.db.GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx,ownerID,prep)
28412848
}
28422849

2850+
func (q*querier)GetWorkspacesByTemplateID(ctx context.Context,templateID uuid.UUID) ([]database.WorkspaceTable,error) {
2851+
iferr:=q.authorizeContext(ctx,policy.ActionRead,rbac.ResourceSystem);err!=nil {
2852+
returnnil,err
2853+
}
2854+
returnq.db.GetWorkspacesByTemplateID(ctx,templateID)
2855+
}
2856+
28432857
func (q*querier)GetWorkspacesEligibleForTransition(ctx context.Context,now time.Time) ([]database.GetWorkspacesEligibleForTransitionRow,error) {
28442858
returnq.db.GetWorkspacesEligibleForTransition(ctx,now)
28452859
}
@@ -4085,6 +4099,13 @@ func (q *querier) UpdateWorkspaceLastUsedAt(ctx context.Context, arg database.Up
40854099
returnupdate(q.log,q.auth,fetch,q.db.UpdateWorkspaceLastUsedAt)(ctx,arg)
40864100
}
40874101

4102+
func (q*querier)UpdateWorkspaceNextStartAt(ctx context.Context,arg database.UpdateWorkspaceNextStartAtParams)error {
4103+
fetch:=func(ctx context.Context,arg database.UpdateWorkspaceNextStartAtParams) (database.Workspace,error) {
4104+
returnq.db.GetWorkspaceByID(ctx,arg.ID)
4105+
}
4106+
returnupdate(q.log,q.auth,fetch,q.db.UpdateWorkspaceNextStartAt)(ctx,arg)
4107+
}
4108+
40884109
func (q*querier)UpdateWorkspaceProxy(ctx context.Context,arg database.UpdateWorkspaceProxyParams) (database.WorkspaceProxy,error) {
40894110
fetch:=func(ctx context.Context,arg database.UpdateWorkspaceProxyParams) (database.WorkspaceProxy,error) {
40904111
returnq.db.GetWorkspaceProxyByID(ctx,arg.ID)

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1908,6 +1908,19 @@ func (s *MethodTestSuite) TestWorkspace() {
19081908
ID:ws.ID,
19091909
}).Asserts(ws,policy.ActionUpdate).Returns()
19101910
}))
1911+
s.Run("UpdateWorkspaceNextStartAt",s.Subtest(func(db database.Store,check*expects) {
1912+
ws:=dbgen.Workspace(s.T(),db, database.WorkspaceTable{})
1913+
check.Args(database.UpdateWorkspaceNextStartAtParams{
1914+
ID:ws.ID,
1915+
NextStartAt: sql.NullTime{Valid:true,Time:dbtime.Now()},
1916+
}).Asserts(ws,policy.ActionUpdate)
1917+
}))
1918+
s.Run("BatchUpdateWorkspaceNextStartAt",s.Subtest(func(db database.Store,check*expects) {
1919+
check.Args(database.BatchUpdateWorkspaceNextStartAtParams{
1920+
IDs: []uuid.UUID{uuid.New()},
1921+
NextStartAts: []time.Time{dbtime.Now()},
1922+
}).Asserts(rbac.ResourceWorkspace.All(),policy.ActionUpdate)
1923+
}))
19111924
s.Run("BatchUpdateWorkspaceLastUsedAt",s.Subtest(func(db database.Store,check*expects) {
19121925
ws1:=dbgen.Workspace(s.T(),db, database.WorkspaceTable{})
19131926
ws2:=dbgen.Workspace(s.T(),db, database.WorkspaceTable{})
@@ -2784,6 +2797,9 @@ func (s *MethodTestSuite) TestSystemFunctions() {
27842797
s.Run("GetTemplateAverageBuildTime",s.Subtest(func(db database.Store,check*expects) {
27852798
check.Args(database.GetTemplateAverageBuildTimeParams{}).Asserts(rbac.ResourceSystem,policy.ActionRead)
27862799
}))
2800+
s.Run("GetWorkspacesByTemplateID",s.Subtest(func(db database.Store,check*expects) {
2801+
check.Args(uuid.Nil).Asserts(rbac.ResourceSystem,policy.ActionRead)
2802+
}))
27872803
s.Run("GetWorkspacesEligibleForTransition",s.Subtest(func(db database.Store,check*expects) {
27882804
check.Args(time.Time{}).Asserts()
27892805
}))

‎coderd/database/dbgen/dbgen.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,7 @@ func Workspace(t testing.TB, db database.Store, orig database.WorkspaceTable) da
260260
AutostartSchedule:orig.AutostartSchedule,
261261
Ttl:orig.Ttl,
262262
AutomaticUpdates:takeFirst(orig.AutomaticUpdates,database.AutomaticUpdatesNever),
263+
NextStartAt:orig.NextStartAt,
263264
})
264265
require.NoError(t,err,"insert workspace")
265266
returnworkspace

‎coderd/database/dbmem/dbmem.go

Lines changed: 77 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,7 @@ func (q *FakeQuerier) convertToWorkspaceRowsNoLock(ctx context.Context, workspac
475475
DeletingAt:w.DeletingAt,
476476
AutomaticUpdates:w.AutomaticUpdates,
477477
Favorite:w.Favorite,
478+
NextStartAt:w.NextStartAt,
478479

479480
OwnerAvatarUrl:extended.OwnerAvatarUrl,
480481
OwnerUsername:extended.OwnerUsername,
@@ -1431,6 +1432,35 @@ func (q *FakeQuerier) BatchUpdateWorkspaceLastUsedAt(_ context.Context, arg data
14311432
returnnil
14321433
}
14331434

1435+
func (q*FakeQuerier)BatchUpdateWorkspaceNextStartAt(_ context.Context,arg database.BatchUpdateWorkspaceNextStartAtParams)error {
1436+
err:=validateDatabaseType(arg)
1437+
iferr!=nil {
1438+
returnerr
1439+
}
1440+
1441+
q.mutex.Lock()
1442+
deferq.mutex.Unlock()
1443+
1444+
fori,workspace:=rangeq.workspaces {
1445+
forj,workspaceID:=rangearg.IDs {
1446+
ifworkspace.ID!=workspaceID {
1447+
continue
1448+
}
1449+
1450+
nextStartAt:=arg.NextStartAts[j]
1451+
ifnextStartAt.IsZero() {
1452+
q.workspaces[i].NextStartAt= sql.NullTime{}
1453+
}else {
1454+
q.workspaces[i].NextStartAt= sql.NullTime{Valid:true,Time:nextStartAt}
1455+
}
1456+
1457+
break
1458+
}
1459+
}
1460+
1461+
returnnil
1462+
}
1463+
14341464
func (*FakeQuerier)BulkMarkNotificationMessagesFailed(_ context.Context,arg database.BulkMarkNotificationMessagesFailedParams) (int64,error) {
14351465
err:=validateDatabaseType(arg)
14361466
iferr!=nil {
@@ -6908,6 +6938,20 @@ func (q *FakeQuerier) GetWorkspacesAndAgentsByOwnerID(ctx context.Context, owner
69086938
returnq.GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx,ownerID,nil)
69096939
}
69106940

6941+
func (q*FakeQuerier)GetWorkspacesByTemplateID(_ context.Context,templateID uuid.UUID) ([]database.WorkspaceTable,error) {
6942+
q.mutex.RLock()
6943+
deferq.mutex.RUnlock()
6944+
6945+
workspaces:= []database.WorkspaceTable{}
6946+
for_,workspace:=rangeq.workspaces {
6947+
ifworkspace.TemplateID==templateID {
6948+
workspaces=append(workspaces,workspace)
6949+
}
6950+
}
6951+
6952+
returnworkspaces,nil
6953+
}
6954+
69116955
func (q*FakeQuerier)GetWorkspacesEligibleForTransition(ctx context.Context,now time.Time) ([]database.GetWorkspacesEligibleForTransitionRow,error) {
69126956
q.mutex.RLock()
69136957
deferq.mutex.RUnlock()
@@ -6952,7 +6996,13 @@ func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, no
69526996
ifuser.Status==database.UserStatusActive&&
69536997
job.JobStatus!=database.ProvisionerJobStatusFailed&&
69546998
build.Transition==database.WorkspaceTransitionStop&&
6955-
workspace.AutostartSchedule.Valid {
6999+
workspace.AutostartSchedule.Valid&&
7000+
// We do not know if workspace with a zero next start is eligible
7001+
// for autostart, so we accept this false-positive. This can occur
7002+
// when a coder version is upgraded and next_start_at has yet to
7003+
// be set.
7004+
(workspace.NextStartAt.Time.IsZero()||
7005+
!now.Before(workspace.NextStartAt.Time)) {
69567006
workspaces=append(workspaces, database.GetWorkspacesEligibleForTransitionRow{
69577007
ID:workspace.ID,
69587008
Name:workspace.Name,
@@ -6962,7 +7012,7 @@ func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, no
69627012

69637013
if!workspace.DormantAt.Valid&&
69647014
template.TimeTilDormant>0&&
6965-
now.Sub(workspace.LastUsedAt)>time.Duration(template.TimeTilDormant) {
7015+
now.Sub(workspace.LastUsedAt)>=time.Duration(template.TimeTilDormant) {
69667016
workspaces=append(workspaces, database.GetWorkspacesEligibleForTransitionRow{
69677017
ID:workspace.ID,
69687018
Name:workspace.Name,
@@ -7927,6 +7977,7 @@ func (q *FakeQuerier) InsertWorkspace(_ context.Context, arg database.InsertWork
79277977
Ttl:arg.Ttl,
79287978
LastUsedAt:arg.LastUsedAt,
79297979
AutomaticUpdates:arg.AutomaticUpdates,
7980+
NextStartAt:arg.NextStartAt,
79307981
}
79317982
q.workspaces=append(q.workspaces,workspace)
79327983
returnworkspace,nil
@@ -9868,6 +9919,7 @@ func (q *FakeQuerier) UpdateWorkspaceAutostart(_ context.Context, arg database.U
98689919
continue
98699920
}
98709921
workspace.AutostartSchedule=arg.AutostartSchedule
9922+
workspace.NextStartAt=arg.NextStartAt
98719923
q.workspaces[index]=workspace
98729924
returnnil
98739925
}
@@ -10017,6 +10069,29 @@ func (q *FakeQuerier) UpdateWorkspaceLastUsedAt(_ context.Context, arg database.
1001710069
returnsql.ErrNoRows
1001810070
}
1001910071

10072+
func (q*FakeQuerier)UpdateWorkspaceNextStartAt(_ context.Context,arg database.UpdateWorkspaceNextStartAtParams)error {
10073+
err:=validateDatabaseType(arg)
10074+
iferr!=nil {
10075+
returnerr
10076+
}
10077+
10078+
q.mutex.Lock()
10079+
deferq.mutex.Unlock()
10080+
10081+
forindex,workspace:=rangeq.workspaces {
10082+
ifworkspace.ID!=arg.ID {
10083+
continue
10084+
}
10085+
10086+
workspace.NextStartAt=arg.NextStartAt
10087+
q.workspaces[index]=workspace
10088+
10089+
returnnil
10090+
}
10091+
10092+
returnsql.ErrNoRows
10093+
}
10094+
1002010095
func (q*FakeQuerier)UpdateWorkspaceProxy(_ context.Context,arg database.UpdateWorkspaceProxyParams) (database.WorkspaceProxy,error) {
1002110096
q.mutex.Lock()
1002210097
deferq.mutex.Unlock()

‎coderd/database/dbmetrics/querymetrics.go

Lines changed: 21 additions & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp