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

Commit290180b

Browse files
authored
feat!: bump workspace activity by 1 hour (#10704)
Marked as a breaking change as the previous activity bump was always the TTL duration of the workspace/template.This change is more cost conservative, only bumping by 1 hour for workspace activity. To accommodate wrap around, eg bumping a workspace into the next autostart, the deadline is bumped by the TTL if the workspace crosses the autostart threshold.This is a niche case that is likely caused by an idle terminal making a workspace survive through a night. The next morning, the workspace will get activity bumped the default TTL on the autostart, being similar to as if the workspace was autostarted again.In practice, a good way to avoid this is to set a max_deadline of <24hrs to avoid wrap around entirely.
1 parent6085b92 commit290180b

File tree

12 files changed

+235
-88
lines changed

12 files changed

+235
-88
lines changed

‎coderd/activitybump.go

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,39 @@ import (
1212
)
1313

1414
// activityBumpWorkspace automatically bumps the workspace's auto-off timer
15-
// if it is set to expire soon.
16-
funcactivityBumpWorkspace(ctx context.Context,log slog.Logger,db database.Store,workspaceID uuid.UUID) {
15+
// if it is set to expire soon. The deadline will be bumped by 1 hour*.
16+
// If the bump crosses over an autostart time, the workspace will be
17+
// bumped by the workspace ttl instead.
18+
//
19+
// If nextAutostart is the zero value or in the past, the workspace
20+
// will be bumped by 1 hour.
21+
// It handles the edge case in the example:
22+
// 1. Autostart is set to 9am.
23+
// 2. User works all day, and leaves a terminal open to the workspace overnight.
24+
// 3. The open terminal continually bumps the workspace deadline.
25+
// 4. 9am the next day, the activity bump pushes to 10am.
26+
// 5. If the user goes inactive for 1 hour during the day, the workspace will
27+
// now stop, because it has been extended by 1 hour durations. Despite the TTL
28+
// being set to 8hrs from the autostart time.
29+
//
30+
// So the issue is that when the workspace is bumped across an autostart
31+
// deadline, we should treat the workspace as being "started" again and
32+
// extend the deadline by the autostart time + workspace ttl instead.
33+
//
34+
// The issue still remains with build_max_deadline. We need to respect the original
35+
// maximum deadline, so that will need to be handled separately.
36+
// A way to avoid this is to configure the max deadline to something that will not
37+
// span more than 1 day. This will force the workspace to restart and reset the deadline
38+
// each morning when it autostarts.
39+
funcactivityBumpWorkspace(ctx context.Context,log slog.Logger,db database.Store,workspaceID uuid.UUID,nextAutostart time.Time) {
1740
// We set a short timeout so if the app is under load, these
1841
// low priority operations fail first.
1942
ctx,cancel:=context.WithTimeout(ctx,time.Second*15)
2043
defercancel()
21-
iferr:=db.ActivityBumpWorkspace(ctx,workspaceID);err!=nil {
44+
iferr:=db.ActivityBumpWorkspace(ctx, database.ActivityBumpWorkspaceParams{
45+
NextAutostart:nextAutostart.UTC(),
46+
WorkspaceID:workspaceID,
47+
});err!=nil {
2248
if!xerrors.Is(err,context.Canceled)&&!database.IsQueryCanceledError(err) {
2349
// Bump will fail if the context is canceled, but this is ok.
2450
log.Error(ctx,"bump failed",slog.Error(err),

‎coderd/activitybump_internal_test.go

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
4242
templateTTL time.Duration
4343
templateDisallowsUserAutostopbool
4444
expectedBump time.Duration
45+
nextAutostart time.Time
4546
}{
4647
{
4748
name:"NotFinishedYet",
@@ -66,22 +67,41 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
6667
workspaceTTL:8*time.Hour,
6768
expectedBump:0,
6869
},
70+
{
71+
// Expected bump is 0 because the original deadline is more than 1 hour
72+
// out, so a bump would decrease the deadline.
73+
name:"BumpLessThanDeadline",
74+
transition:database.WorkspaceTransitionStart,
75+
jobCompletedAt: sql.NullTime{Valid:true,Time:dbtime.Now().Add(-30*time.Minute)},
76+
buildDeadlineOffset:ptr.Ref(8*time.Hour-30*time.Minute),
77+
workspaceTTL:8*time.Hour,
78+
expectedBump:0,
79+
},
6980
{
7081
name:"TimeToBump",
7182
transition:database.WorkspaceTransitionStart,
72-
jobCompletedAt: sql.NullTime{Valid:true,Time:dbtime.Now().Add(-24*time.Minute)},
73-
buildDeadlineOffset:ptr.Ref(8*time.Hour-24*time.Minute),
83+
jobCompletedAt: sql.NullTime{Valid:true,Time:dbtime.Now().Add(-30*time.Minute)},
84+
buildDeadlineOffset:ptr.Ref(-30*time.Minute),
85+
workspaceTTL:8*time.Hour,
86+
expectedBump:time.Hour,
87+
},
88+
{
89+
name:"TimeToBumpNextAutostart",
90+
transition:database.WorkspaceTransitionStart,
91+
jobCompletedAt: sql.NullTime{Valid:true,Time:dbtime.Now().Add(-30*time.Minute)},
92+
buildDeadlineOffset:ptr.Ref(-30*time.Minute),
7493
workspaceTTL:8*time.Hour,
75-
expectedBump:8*time.Hour,
94+
expectedBump:8*time.Hour+30*time.Minute,
95+
nextAutostart:time.Now().Add(time.Minute*30),
7696
},
7797
{
7898
name:"MaxDeadline",
7999
transition:database.WorkspaceTransitionStart,
80100
jobCompletedAt: sql.NullTime{Valid:true,Time:dbtime.Now().Add(-24*time.Minute)},
81101
buildDeadlineOffset:ptr.Ref(time.Minute),// last chance to bump!
82-
maxDeadlineOffset:ptr.Ref(time.Hour),
102+
maxDeadlineOffset:ptr.Ref(time.Minute*30),
83103
workspaceTTL:8*time.Hour,
84-
expectedBump:1*time.Hour,
104+
expectedBump:time.Minute*30,
85105
},
86106
{
87107
// A workspace that is still running, has passed its deadline, but has not
@@ -91,7 +111,7 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
91111
jobCompletedAt: sql.NullTime{Valid:true,Time:dbtime.Now().Add(-24*time.Minute)},
92112
buildDeadlineOffset:ptr.Ref(-time.Minute),
93113
workspaceTTL:8*time.Hour,
94-
expectedBump:8*time.Hour,
114+
expectedBump:time.Hour,
95115
},
96116
{
97117
// A stopped workspace should never bump.
@@ -106,12 +126,13 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
106126
// by the template TTL instead.
107127
name:"TemplateDisallowsUserAutostop",
108128
transition:database.WorkspaceTransitionStart,
109-
jobCompletedAt: sql.NullTime{Valid:true,Time:dbtime.Now().Add(-24*time.Minute)},
110-
buildDeadlineOffset:ptr.Ref(8*time.Hour-24*time.Minute),
111-
workspaceTTL:6*time.Hour,
112-
templateTTL:8*time.Hour,
129+
jobCompletedAt: sql.NullTime{Valid:true,Time:dbtime.Now().Add(-7*time.Hour)},
130+
buildDeadlineOffset:ptr.Ref(-30*time.Minute),
131+
workspaceTTL:2*time.Hour,
132+
templateTTL:10*time.Hour,
113133
templateDisallowsUserAutostop:true,
114-
expectedBump:8*time.Hour,
134+
expectedBump:10*time.Hour+ (time.Minute*30),
135+
nextAutostart:time.Now().Add(time.Minute*30),
115136
},
116137
} {
117138
tt:=tt
@@ -215,7 +236,7 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
215236

216237
// Bump duration is measured from the time of the bump, so we measure from here.
217238
start:=dbtime.Now()
218-
activityBumpWorkspace(ctx,log,db,bld.WorkspaceID)
239+
activityBumpWorkspace(ctx,log,db,bld.WorkspaceID,tt.nextAutostart)
219240
end:=dbtime.Now()
220241

221242
// Validate our state after bump
@@ -233,9 +254,9 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
233254
return
234255
}
235256

236-
// Assert that the bump occurred between start and end.
237-
expectedDeadlineStart:=start.Add(tt.expectedBump)
238-
expectedDeadlineEnd:=end.Add(tt.expectedBump)
257+
// Assert that the bump occurred between start and end. 1min buffer on either side.
258+
expectedDeadlineStart:=start.Add(tt.expectedBump).Add(time.Minute*-1)
259+
expectedDeadlineEnd:=end.Add(tt.expectedBump).Add(time.Minute)
239260
require.GreaterOrEqual(t,updatedBuild.Deadline,expectedDeadlineStart,"new deadline should be greater than or equal to start")
240261
require.LessOrEqual(t,updatedBuild.Deadline,expectedDeadlineEnd,"new deadline should be lesser than or equal to end")
241262
})

‎coderd/activitybump_test.go

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func TestWorkspaceActivityBump(t *testing.T) {
3030
// max_deadline on the build directly in the database.
3131
setupActivityTest:=func(t*testing.T,deadline...time.Duration) (client*codersdk.Client,workspace codersdk.Workspace,assertBumpedfunc(wantbool)) {
3232
t.Helper()
33-
constttl=time.Minute
33+
constttl=time.Hour
3434
maxTTL:=time.Duration(0)
3535
iflen(deadline)>0 {
3636
maxTTL=deadline[0]
@@ -71,28 +71,29 @@ func TestWorkspaceActivityBump(t *testing.T) {
7171
})
7272
coderdtest.AwaitWorkspaceBuildJobCompleted(t,client,workspace.LatestBuild.ID)
7373

74+
varmaxDeadline time.Time
7475
// Update the max deadline.
7576
ifmaxTTL!=0 {
76-
dbBuild,err:=db.GetWorkspaceBuildByID(ctx,workspace.LatestBuild.ID)
77-
require.NoError(t,err)
78-
79-
err=db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{
80-
ID:workspace.LatestBuild.ID,
81-
UpdatedAt:dbtime.Now(),
82-
Deadline:dbBuild.Deadline,
83-
MaxDeadline:dbtime.Now().Add(maxTTL),
84-
})
85-
require.NoError(t,err)
77+
maxDeadline=dbtime.Now().Add(maxTTL)
8678
}
8779

80+
err:=db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{
81+
ID:workspace.LatestBuild.ID,
82+
UpdatedAt:dbtime.Now(),
83+
// Make the deadline really close so it needs to be bumped immediately.
84+
Deadline:dbtime.Now().Add(time.Minute),
85+
MaxDeadline:maxDeadline,
86+
})
87+
require.NoError(t,err)
88+
8889
_=agenttest.New(t,client.URL,agentToken)
8990
coderdtest.AwaitWorkspaceAgents(t,client,workspace.ID)
9091

91-
// Sanity-check that deadline isnear.
92-
workspace,err:=client.Workspace(ctx,workspace.ID)
92+
// Sanity-check that deadline isnearing requiring a bump.
93+
workspace,err=client.Workspace(ctx,workspace.ID)
9394
require.NoError(t,err)
9495
require.WithinDuration(t,
95-
time.Now().Add(time.Duration(ttlMillis)*time.Millisecond),
96+
time.Now().Add(time.Minute),
9697
workspace.LatestBuild.Deadline.Time,
9798
testutil.WaitMedium,
9899
)
@@ -133,7 +134,7 @@ func TestWorkspaceActivityBump(t *testing.T) {
133134
workspace,err=client.Workspace(ctx,workspace.ID)
134135
require.NoError(t,err)
135136
updatedAfter=dbtime.Now()
136-
ifworkspace.LatestBuild.Deadline.Time==firstDeadline {
137+
ifworkspace.LatestBuild.Deadline.Time.Equal(firstDeadline) {
137138
updatedAfter=time.Now()
138139
returnfalse
139140
}
@@ -151,6 +152,13 @@ func TestWorkspaceActivityBump(t *testing.T) {
151152
require.LessOrEqual(t,workspace.LatestBuild.Deadline.Time,workspace.LatestBuild.MaxDeadline.Time)
152153
return
153154
}
155+
now:=dbtime.Now()
156+
zone,offset:=time.Now().Zone()
157+
t.Logf("[Zone=%s %d] originDeadline: %s, deadline: %s, now %s, (now-deadline)=%s",
158+
zone,offset,
159+
firstDeadline,workspace.LatestBuild.Deadline.Time,now,
160+
now.Sub(workspace.LatestBuild.Deadline.Time),
161+
)
154162
require.WithinDuration(t,dbtime.Now().Add(ttl),workspace.LatestBuild.Deadline.Time,testutil.WaitShort)
155163
}
156164
}
@@ -192,9 +200,9 @@ func TestWorkspaceActivityBump(t *testing.T) {
192200
t.Run("NotExceedMaxDeadline",func(t*testing.T) {
193201
t.Parallel()
194202

195-
// Set the max deadline to be in61 seconds. We bump by 1minute, so we
203+
// Set the max deadline to be in30min. We bump by 1hour, so we
196204
// should expect the deadline to match the max deadline exactly.
197-
client,workspace,assertBumped:=setupActivityTest(t,61*time.Second)
205+
client,workspace,assertBumped:=setupActivityTest(t,time.Minute*30)
198206

199207
// Bump by dialing the workspace and sending traffic.
200208
resources:=coderdtest.AwaitWorkspaceAgents(t,client,workspace.ID)

‎coderd/autobuild/lifecycle_executor.go

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -357,26 +357,36 @@ func isEligibleForAutostart(ws database.Workspace, build database.WorkspaceBuild
357357
returnfalse
358358
}
359359

360-
sched,err:=cron.Weekly(ws.AutostartSchedule.String)
361-
iferr!=nil {
360+
nextTransition,allowed:=NextAutostartSchedule(build.CreatedAt,ws.AutostartSchedule.String,templateSchedule)
361+
if!allowed {
362362
returnfalse
363363
}
364+
365+
// Must use '.Before' vs '.After' so equal times are considered "valid for autostart".
366+
return!currentTick.Before(nextTransition)
367+
}
368+
369+
// NextAutostartSchedule takes the workspace and template schedule and returns the next autostart schedule
370+
// after "at". The boolean returned is if the autostart should be allowed to start based on the template
371+
// schedule.
372+
funcNextAutostartSchedule(at time.Time,wsSchedulestring,templateSchedule schedule.TemplateScheduleOptions) (time.Time,bool) {
373+
sched,err:=cron.Weekly(wsSchedule)
374+
iferr!=nil {
375+
return time.Time{},false
376+
}
377+
364378
// Round down to the nearest minute, as this is the finest granularity cron supports.
365379
// Truncate is probably not necessary here, but doing it anyway to be sure.
366-
nextTransition:=sched.Next(build.CreatedAt).Truncate(time.Minute)
380+
nextTransition:=sched.Next(at).Truncate(time.Minute)
367381

368382
// The nextTransition is when the auto start should kick off. If it lands on a
369383
// forbidden day, do not allow the auto start. We use the time location of the
370384
// schedule to determine the weekday. So if "Saturday" is disallowed, the
371385
// definition of "Saturday" depends on the location of the schedule.
372386
zonedTransition:=nextTransition.In(sched.Location())
373387
allowed:=templateSchedule.AutostartRequirement.DaysMap()[zonedTransition.Weekday()]
374-
if!allowed {
375-
returnfalse
376-
}
377388

378-
// Must used '.Before' vs '.After' so equal times are considered "valid for autostart".
379-
return!currentTick.Before(nextTransition)
389+
returnzonedTransition,allowed
380390
}
381391

382392
// isEligibleForAutostart returns true if the workspace should be autostopped.

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -660,9 +660,9 @@ func (q *querier) AcquireProvisionerJob(ctx context.Context, arg database.Acquir
660660
returnq.db.AcquireProvisionerJob(ctx,arg)
661661
}
662662

663-
func (q*querier)ActivityBumpWorkspace(ctx context.Context,arguuid.UUID)error {
664-
fetch:=func(ctx context.Context,arguuid.UUID) (database.Workspace,error) {
665-
returnq.db.GetWorkspaceByID(ctx,arg)
663+
func (q*querier)ActivityBumpWorkspace(ctx context.Context,argdatabase.ActivityBumpWorkspaceParams)error {
664+
fetch:=func(ctx context.Context,argdatabase.ActivityBumpWorkspaceParams) (database.Workspace,error) {
665+
returnq.db.GetWorkspaceByID(ctx,arg.WorkspaceID)
666666
}
667667
returnupdate(q.log,q.auth,fetch,q.db.ActivityBumpWorkspace)(ctx,arg)
668668
}

‎coderd/database/dbmem/dbmem.go

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -678,6 +678,13 @@ func (q *FakeQuerier) GetActiveDBCryptKeys(_ context.Context) ([]database.DBCryp
678678
returnks,nil
679679
}
680680

681+
funcmaxTime(t,u time.Time) time.Time {
682+
ift.After(u) {
683+
returnt
684+
}
685+
returnu
686+
}
687+
681688
funcminTime(t,u time.Time) time.Time {
682689
ift.Before(u) {
683690
returnt
@@ -775,20 +782,20 @@ func (q *FakeQuerier) AcquireProvisionerJob(_ context.Context, arg database.Acqu
775782
return database.ProvisionerJob{},sql.ErrNoRows
776783
}
777784

778-
func (q*FakeQuerier)ActivityBumpWorkspace(ctx context.Context,workspaceID uuid.UUID)error {
779-
err:=validateDatabaseType(workspaceID)
785+
func (q*FakeQuerier)ActivityBumpWorkspace(ctx context.Context,arg database.ActivityBumpWorkspaceParams)error {
786+
err:=validateDatabaseType(arg)
780787
iferr!=nil {
781788
returnerr
782789
}
783790

784791
q.mutex.Lock()
785792
deferq.mutex.Unlock()
786793

787-
workspace,err:=q.getWorkspaceByIDNoLock(ctx,workspaceID)
794+
workspace,err:=q.getWorkspaceByIDNoLock(ctx,arg.WorkspaceID)
788795
iferr!=nil {
789796
returnerr
790797
}
791-
latestBuild,err:=q.getLatestWorkspaceBuildByWorkspaceIDNoLock(ctx,workspaceID)
798+
latestBuild,err:=q.getLatestWorkspaceBuildByWorkspaceIDNoLock(ctx,arg.WorkspaceID)
792799
iferr!=nil {
793800
returnerr
794801
}
@@ -822,15 +829,17 @@ func (q *FakeQuerier) ActivityBumpWorkspace(ctx context.Context, workspaceID uui
822829
}
823830

824831
varttlDur time.Duration
825-
ifworkspace.Ttl.Valid {
826-
ttlDur=time.Duration(workspace.Ttl.Int64)
827-
}
828-
if!template.AllowUserAutostop {
829-
ttlDur=time.Duration(template.DefaultTTL)
830-
}
831-
ifttlDur<=0 {
832-
// There's no TTL set anymore, so we don't know the bump duration.
833-
returnnil
832+
ifnow.Add(time.Hour).After(arg.NextAutostart)&&arg.NextAutostart.After(now) {
833+
// Extend to TTL
834+
add:=arg.NextAutostart.Sub(now)
835+
ifworkspace.Ttl.Valid&&template.AllowUserAutostop {
836+
add+=time.Duration(workspace.Ttl.Int64)
837+
}else {
838+
add+=time.Duration(template.DefaultTTL)
839+
}
840+
ttlDur=add
841+
}else {
842+
ttlDur=time.Hour
834843
}
835844

836845
// Only bump if 5% of the deadline has passed.
@@ -842,6 +851,8 @@ func (q *FakeQuerier) ActivityBumpWorkspace(ctx context.Context, workspaceID uui
842851

843852
// Bump.
844853
newDeadline:=now.Add(ttlDur)
854+
// Never decrease deadlines from a bump
855+
newDeadline=maxTime(newDeadline,q.workspaceBuilds[i].Deadline)
845856
q.workspaceBuilds[i].UpdatedAt=now
846857
if!q.workspaceBuilds[i].MaxDeadline.IsZero() {
847858
q.workspaceBuilds[i].Deadline=minTime(newDeadline,q.workspaceBuilds[i].MaxDeadline)

‎coderd/database/dbmetrics/dbmetrics.go

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

‎coderd/database/dbmock/dbmock.go

Lines changed: 1 addition & 1 deletion
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