feat: workspace activity bump by 1 hour

Will bump by ttl if crosses an autostart threshold
coder · Emyrk · Nov 15, 2023 · Nov 14, 2023 · Nov 14, 2023 · Nov 15, 2023
commit 7bd21238d0d9f5659bc62c8430c45e9f0aa54276
diff --git a/coderd/activitybump.go b/coderd/activitybump.go
@@ -12,13 +12,38 @@ import (
 )
 
 // activityBumpWorkspace automatically bumps the workspace's auto-off timer
-// if it is set to expire soon.
-func activityBumpWorkspace(ctx context.Context, log slog.Logger, db database.Store, workspaceID uuid.UUID) {
+// if it is set to expire soon. The deadline will be bumped by 1 hour*.
+// If the bump crosses over an autostart time, the workspace will be
+// bumped by the workspace ttl instead.
+//
+// Autostart is optional if bumping by 1 hour is sufficient.
+// It handles the edge case in the example:
+//  1. Autostart is set to 9am.
+//  2. User works all day, and leaves a terminal open to the workspace overnight.
+//  3. The open terminal continually bumps the workspace deadline.
+//  4. 9am the next day, the activity bump pushes to 10am.
+//  5. If the user goes inactive for 1 hour during the day, the workspace will
+//     now stop, because it has been extended by 1 hour durations. Despite the TTL
+//     being set to 8hrs from the autostart time.
+//
+// So the issue is that when the workspace is bumped across an autostart
+// deadline, we should treat the workspace as being "started" again and
+// extend the deadline by the autostart time + workspace ttl instead.
+//
+// The issue still remains with build_max_deadline. We need to respect the original
+// maximum deadline, so that will need to be handled separately.
+// A way to avoid this is to configure the max deadline to something that will not
+// span more than 1 day. This will force the workspace to restart and reset the deadline
+// each morning when it autostarts.
+func activityBumpWorkspace(ctx context.Context, log slog.Logger, db database.Store, workspaceID uuid.UUID, nextAutostart time.Time) {
 	// We set a short timeout so if the app is under load, these
 	// low priority operations fail first.
 	ctx, cancel := context.WithTimeout(ctx, time.Second*15)
 	defer cancel()
-	if err := db.ActivityBumpWorkspace(ctx, workspaceID); err != nil {
+	if err := db.ActivityBumpWorkspace(ctx, database.ActivityBumpWorkspaceParams{
+		NextAutostart: nextAutostart,
+		WorkspaceID:   workspaceID,
+	}); err != nil {
 		if !xerrors.Is(err, context.Canceled) && !database.IsQueryCanceledError(err) {
 			// Bump will fail if the context is canceled, but this is ok.
 			log.Error(ctx, "bump failed", slog.Error(err),

diff --git a/coderd/activitybump_internal_test.go b/coderd/activitybump_internal_test.go
@@ -42,6 +42,7 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
 		templateTTL                   time.Duration
 		templateDisallowsUserAutostop bool
 		expectedBump                  time.Duration
+		nextAutostart                 time.Time
 	}{
 		{
 			name:                "NotFinishedYet",
@@ -72,7 +73,7 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
 			jobCompletedAt:      sql.NullTime{Valid: true, Time: dbtime.Now().Add(-24 * time.Minute)},
 			buildDeadlineOffset: ptr.Ref(8*time.Hour - 24*time.Minute),
 			workspaceTTL:        8 * time.Hour,
-			expectedBump:        8 * time.Hour,
+			expectedBump:        time.Hour, //8 * time.Hour,
 		},
 		{
 			name:                "MaxDeadline",
@@ -81,7 +82,7 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
 			buildDeadlineOffset: ptr.Ref(time.Minute), // last chance to bump!
 			maxDeadlineOffset:   ptr.Ref(time.Hour),
 			workspaceTTL:        8 * time.Hour,
-			expectedBump:        1 * time.Hour,
+			expectedBump:        time.Hour, //1 * time.Hour,
 		},
 		{
 			// A workspace that is still running, has passed its deadline, but has not
@@ -91,15 +92,15 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
 			jobCompletedAt:      sql.NullTime{Valid: true, Time: dbtime.Now().Add(-24 * time.Minute)},
 			buildDeadlineOffset: ptr.Ref(-time.Minute),
 			workspaceTTL:        8 * time.Hour,
-			expectedBump:        8 * time.Hour,
+			expectedBump:        time.Hour, //8 * time.Hour,
 		},
 		{
 			// A stopped workspace should never bump.
 			name:                "StoppedWorkspace",
 			transition:          database.WorkspaceTransitionStop,
 			jobCompletedAt:      sql.NullTime{Valid: true, Time: dbtime.Now().Add(-time.Minute)},
 			buildDeadlineOffset: ptr.Ref(-time.Minute),
-			workspaceTTL:        8 * time.Hour,
+			workspaceTTL:        time.Hour, //8 * time.Hour,
 		},
 		{
 			// A workspace built from a template that disallows user autostop should bump
@@ -111,7 +112,7 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
 			workspaceTTL:                  6 * time.Hour,
 			templateTTL:                   8 * time.Hour,
 			templateDisallowsUserAutostop: true,
-			expectedBump:                  8 * time.Hour,
+			expectedBump:                  time.Hour, //8 * time.Hour,
 		},
 	} {
 		tt := tt
@@ -215,7 +216,7 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
 
 				// Bump duration is measured from the time of the bump, so we measure from here.
 				start := dbtime.Now()
-				activityBumpWorkspace(ctx, log, db, bld.WorkspaceID)
+				activityBumpWorkspace(ctx, log, db, bld.WorkspaceID, tt.nextAutostart)
 				end := dbtime.Now()
 
 				// Validate our state after bump

diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go
@@ -660,9 +660,9 @@ func (q *querier) AcquireProvisionerJob(ctx context.Context, arg database.Acquir
 	return q.db.AcquireProvisionerJob(ctx, arg)
 }
 
-func (q *querier) ActivityBumpWorkspace(ctx context.Context, arg uuid.UUID) error {
-	fetch := func(ctx context.Context, arg uuid.UUID) (database.Workspace, error) {
-		return q.db.GetWorkspaceByID(ctx, arg)
+func (q *querier) ActivityBumpWorkspace(ctx context.Context, arg database.ActivityBumpWorkspaceParams) error {
+	fetch := func(ctx context.Context, arg database.ActivityBumpWorkspaceParams) (database.Workspace, error) {
+		return q.db.GetWorkspaceByID(ctx, arg.WorkspaceID)
 	}
 	return update(q.log, q.auth, fetch, q.db.ActivityBumpWorkspace)(ctx, arg)
 }

diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go
@@ -775,20 +775,20 @@ func (q *FakeQuerier) AcquireProvisionerJob(_ context.Context, arg database.Acqu
 	return database.ProvisionerJob{}, sql.ErrNoRows
 }
 
-func (q *FakeQuerier) ActivityBumpWorkspace(ctx context.Context, workspaceID uuid.UUID) error {
-	err := validateDatabaseType(workspaceID)
+func (q *FakeQuerier) ActivityBumpWorkspace(ctx context.Context, arg database.ActivityBumpWorkspaceParams) error {
+	err := validateDatabaseType(arg)
 	if err != nil {
 		return err
 	}
 
 	q.mutex.Lock()
 	defer q.mutex.Unlock()
 
-	workspace, err := q.getWorkspaceByIDNoLock(ctx, workspaceID)
+	workspace, err := q.getWorkspaceByIDNoLock(ctx, arg.WorkspaceID)
 	if err != nil {
 		return err
 	}
-	latestBuild, err := q.getLatestWorkspaceBuildByWorkspaceIDNoLock(ctx, workspaceID)
+	latestBuild, err := q.getLatestWorkspaceBuildByWorkspaceIDNoLock(ctx, arg.WorkspaceID)
 	if err != nil {
 		return err
 	}

diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go
diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go
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/activitybump.sql b/coderd/database/queries/activitybump.sql
@@ -12,9 +12,29 @@ WITH latest AS (
 		provisioner_jobs.completed_at::timestamp with time zone AS job_completed_at,
 		(
 			CASE
-				WHEN templates.allow_user_autostop
-				THEN (workspaces.ttl / 1000 / 1000 / 1000 || ' seconds')::interval
-				ELSE (templates.default_ttl / 1000 / 1000 / 1000 || ' seconds')::interval
+				-- If the extension would push us over the next_autostart
+				-- interval, then extend the deadline by the full ttl from
+				-- the autostart time. This will essentially be as if the
+				-- workspace auto started at the given time and the original
+				-- TTL was applied.
+				WHEN NOW() + ('60 minutes')::interval > @next_autostart :: timestamptz
+				    -- If the autostart is behind the created_at, then the
+					-- autostart schedule is either the 0 time and not provided,
+					-- or it was the autostart in the past, which is no longer
+					-- relevant. If a past autostart is being passed in,
+					-- that is a mistake by the caller.
+					AND @next_autostart > workspace_builds.created_at
+					THEN
+					-- Extend to the autostart, then add the TTL
+					((@next_autostart :: timestamptz) - NOW()) + CASE
+						WHEN templates.allow_user_autostop
+					    	THEN (workspaces.ttl / 1000 / 1000 / 1000 || ' seconds')::interval
+							ELSE (templates.default_ttl / 1000 / 1000 / 1000 || ' seconds')::interval
+					END
+
+				-- Default to 60 minutes.
+				ELSE
+					('60 minutes')::interval
 			END
 		) AS ttl_interval
 	FROM workspace_builds

diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go
@@ -1671,7 +1671,7 @@ func (api *API) workspaceAgentReportStats(rw http.ResponseWriter, r *http.Reques
 	)
 
 	if req.ConnectionCount > 0 {
-		activityBumpWorkspace(ctx, api.Logger.Named("activity_bump"), api.Database, workspace.ID)
+		activityBumpWorkspace(ctx, api.Logger.Named("activity_bump"), api.Database, workspace.ID, time.Time{})
 	}
 
 	now := dbtime.Now()