coder · deansheather · Jun 25, 2023 · Jun 9, 2023 · Jun 9, 2023 · Jun 9, 2023
diff --git a/cli/server.go b/cli/server.go
@@ -77,6 +77,7 @@ import (
 	"github.com/coder/coder/coderd/schedule"
 	"github.com/coder/coder/coderd/telemetry"
 	"github.com/coder/coder/coderd/tracing"
+	"github.com/coder/coder/coderd/unhanger"
 	"github.com/coder/coder/coderd/updatecheck"
 	"github.com/coder/coder/coderd/util/slice"
 	"github.com/coder/coder/coderd/workspaceapps"
@@ -892,11 +893,16 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 				return xerrors.Errorf("notify systemd: %w", err)
 			}
 
-			autobuildPoller := time.NewTicker(cfg.AutobuildPollInterval.Value())
-			defer autobuildPoller.Stop()
-			autobuildExecutor := executor.New(ctx, options.Database, coderAPI.TemplateScheduleStore, logger, autobuildPoller.C)
+			autobuildTicker := time.NewTicker(cfg.AutobuildPollInterval.Value())
+			defer autobuildTicker.Stop()
+			autobuildExecutor := executor.New(ctx, options.Database, coderAPI.TemplateScheduleStore, logger, autobuildTicker.C)
 			autobuildExecutor.Run()
 
+			hangDetectorTicker := time.NewTicker(cfg.JobHangDetectorInterval.Value())
+			defer hangDetectorTicker.Stop()
+			hangDetector := unhanger.New(ctx, options.Database, options.Pubsub, logger, hangDetectorTicker.C)
+			hangDetector.Run()
+
 			// Currently there is no way to ask the server to shut
 			// itself down, so any exit signal will result in a non-zero
 			// exit of the server.

diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden
@@ -141,6 +141,9 @@ networking:
 # Interval to poll for scheduled workspace builds.
 # (default: 1m0s, type: duration)
 autobuildPollInterval: 1m0s
+# Interval to poll for hung jobs and automatically terminate them.
+# (default: 1m0s, type: duration)
+jobHangDetectorInterval: 1m0s
 introspection:
   prometheus:
     # Serve prometheus metrics on the address defined by prometheus address.

diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go
diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json
diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go
@@ -67,6 +67,7 @@ import (
 	"github.com/coder/coder/coderd/rbac"
 	"github.com/coder/coder/coderd/schedule"
 	"github.com/coder/coder/coderd/telemetry"
+	"github.com/coder/coder/coderd/unhanger"
 	"github.com/coder/coder/coderd/updatecheck"
 	"github.com/coder/coder/coderd/util/ptr"
 	"github.com/coder/coder/coderd/workspaceapps"
@@ -250,6 +251,11 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
 	).WithStatsChannel(options.AutobuildStats)
 	lifecycleExecutor.Run()
 
+	hangDetectorTicker := time.NewTicker(options.DeploymentValues.JobHangDetectorInterval.Value())
+	defer hangDetectorTicker.Stop()
+	hangDetector := unhanger.New(ctx, options.Database, options.Pubsub, slogtest.Make(t, nil).Named("unhanger.detector"), hangDetectorTicker.C)
+	hangDetector.Run()
+
 	var mutex sync.RWMutex
 	var handler http.Handler
 	srv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

diff --git a/coderd/database/db2sdk/db2sdk.go b/coderd/database/db2sdk/db2sdk.go
@@ -3,7 +3,6 @@ package db2sdk
 
 import (
 	"encoding/json"
-	"time"
 
 	"github.com/coder/coder/coderd/database"
 	"github.com/coder/coder/coderd/parameter"
@@ -78,6 +77,9 @@ func TemplateVersionParameter(param database.TemplateVersionParameter) (codersdk
 }
 
 func ProvisionerJobStatus(provisionerJob database.ProvisionerJob) codersdk.ProvisionerJobStatus {
+	// The case where jobs are hung is handled by the unhang package. We can't
+	// just return Failed here when it's hung because that doesn't reflect in
+	// the database.
 	switch {
 	case provisionerJob.CanceledAt.Valid:
 		if !provisionerJob.CompletedAt.Valid {
@@ -94,8 +96,6 @@ func ProvisionerJobStatus(provisionerJob database.ProvisionerJob) codersdk.Provi
 			return codersdk.ProvisionerJobSucceeded
 		}
 		return codersdk.ProvisionerJobFailed
-	case database.Now().Sub(provisionerJob.UpdatedAt) > 30*time.Second:
-		return codersdk.ProvisionerJobFailed
 	default:
 		return codersdk.ProvisionerJobRunning
 	}

diff --git a/coderd/database/db2sdk/db2sdk_test.go b/coderd/database/db2sdk/db2sdk_test.go
@@ -96,17 +96,6 @@ func TestProvisionerJobStatus(t *testing.T) {
 			},
 			status: codersdk.ProvisionerJobFailed,
 		},
-		{
-			name: "not_updated",
-			job: database.ProvisionerJob{
-				StartedAt: sql.NullTime{
-					Time:  database.Now().Add(-time.Minute),
-					Valid: true,
-				},
-				UpdatedAt: database.Now().Add(-31 * time.Second),
-			},
-			status: codersdk.ProvisionerJobFailed,
-		},
 		{
 			name: "updated",
 			job: database.ProvisionerJob{

diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go
@@ -172,6 +172,25 @@ var (
 		Scope: rbac.ScopeAll,
 	}.WithCachedASTValue()
 
+	// See unhanger package.
+	subjectHangDetector = rbac.Subject{
+		ID: uuid.Nil.String(),
+		Roles: rbac.Roles([]rbac.Role{
+			{
+				Name:        "hangdetector",
+				DisplayName: "Hang Detector Daemon",
+				Site: rbac.Permissions(map[string][]rbac.Action{
+					rbac.ResourceSystem.Type:    {rbac.WildcardSymbol},
+					rbac.ResourceTemplate.Type:  {rbac.ActionRead},
+					rbac.ResourceWorkspace.Type: {rbac.ActionRead, rbac.ActionUpdate},
+				}),
+				Org:  map[string][]rbac.Permission{},
+				User: []rbac.Permission{},
+			},
+		}),
+		Scope: rbac.ScopeAll,
+	}.WithCachedASTValue()
+
 	subjectSystemRestricted = rbac.Subject{
 		ID: uuid.Nil.String(),
 		Roles: rbac.Roles([]rbac.Role{
@@ -213,6 +232,12 @@ func AsAutostart(ctx context.Context) context.Context {
 	return context.WithValue(ctx, authContextKey{}, subjectAutostart)
 }
 
+// AsHangDetector returns a context with an actor that has permissions required
+// for unhanger.Detector to function.
+func AsHangDetector(ctx context.Context) context.Context {
+	return context.WithValue(ctx, authContextKey{}, subjectHangDetector)
+}
+
 // AsSystemRestricted returns a context with an actor that has permissions
 // required for various system operations (login, logout, metrics cache).
 func AsSystemRestricted(ctx context.Context) context.Context {

diff --git a/coderd/database/dbauthz/system.go b/coderd/database/dbauthz/system.go
@@ -392,6 +392,14 @@ func (q *querier) InsertProvisionerJobLogs(ctx context.Context, arg database.Ins
 	return q.db.InsertProvisionerJobLogs(ctx, arg)
 }
 
+// TODO: We need to create a ProvisionerJob resource type
+func (q *querier) GetHungProvisionerJobs(ctx context.Context, hungSince time.Time) ([]database.ProvisionerJob, error) {
+	// if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil {
+	// return nil, err
+	// }
+	return q.db.GetHungProvisionerJobs(ctx, hungSince)
+}
+
 func (q *querier) InsertWorkspaceAgentStartupLogs(ctx context.Context, arg database.InsertWorkspaceAgentStartupLogsParams) ([]database.WorkspaceAgentStartupLog, error) {
 	return q.db.InsertWorkspaceAgentStartupLogs(ctx, arg)
 }

diff --git a/coderd/database/dbfake/databasefake.go b/coderd/database/dbfake/databasefake.go
@@ -2561,6 +2561,19 @@ func (q *fakeQuerier) getProvisionerJobByIDNoLock(_ context.Context, id uuid.UUI
 	return database.ProvisionerJob{}, sql.ErrNoRows
 }
 
+func (q *fakeQuerier) GetHungProvisionerJobs(_ context.Context, hungSince time.Time) ([]database.ProvisionerJob, error) {
+	q.mutex.RLock()
+	defer q.mutex.RUnlock()
+
+	hungJobs := []database.ProvisionerJob{}
+	for _, provisionerJob := range q.provisionerJobs {
+		if provisionerJob.StartedAt.Valid && !provisionerJob.CompletedAt.Valid && provisionerJob.UpdatedAt.Before(hungSince) {
+			hungJobs = append(hungJobs, provisionerJob)
+		}
+	}
+	return hungJobs, nil
+}
+
 func (q *fakeQuerier) GetWorkspaceResourceByID(_ context.Context, id uuid.UUID) (database.WorkspaceResource, error) {
 	q.mutex.RLock()
 	defer q.mutex.RUnlock()

diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go
@@ -1532,3 +1532,10 @@ func (m metricsStore) GetDefaultProxyConfig(ctx context.Context) (database.GetDe
 	m.queryLatencies.WithLabelValues("GetDefaultProxyConfig").Observe(time.Since(start).Seconds())
 	return resp, err
 }
+
+func (m metricsStore) GetHungProvisionerJobs(ctx context.Context, hungSince time.Time) ([]database.ProvisionerJob, error) {
+	start := time.Now()
+	jobs, err := m.s.GetHungProvisionerJobs(ctx, hungSince)
+	m.queryLatencies.WithLabelValues("GetHungProvisionerJobs").Observe(time.Since(start).Seconds())
+	return jobs, err
+}
diff --git a/coderd/database/dbmock/store.go b/coderd/database/dbmock/store.go
diff --git a/coderd/database/lock.go b/coderd/database/lock.go
@@ -1,8 +1,10 @@
 package database
 
 // Well-known lock IDs for lock functions in the database. These should not
-// change. If locks are deprecated, they should be kept to avoid reusing the
-// same ID.
+// change. If locks are deprecated, they should be kept in this list to avoid
+// reusing the same ID.
 const (
-	LockIDDeploymentSetup = iota + 1
+	lockIDUnused          = iota
+	LockIDDeploymentSetup = iota
+	LockIDHangDetector    = iota
 )
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/lock.sql b/coderd/database/queries/lock.sql
@@ -3,15 +3,11 @@
 --
 -- This must be called from within a transaction. The lock will be automatically
 -- released when the transaction ends.
---
--- Use database.LockID() to generate a unique lock ID from a string.
 SELECT pg_advisory_xact_lock($1);
 
 -- name: TryAcquireLock :one
 -- Non blocking lock. Returns true if the lock was acquired, false otherwise.
 --
 -- This must be called from within a transaction. The lock will be automatically
 -- released when the transaction ends.
---
--- Use database.LockID() to generate a unique lock ID from a string.
 SELECT pg_try_advisory_xact_lock($1);
diff --git a/coderd/database/queries/provisionerjobs.sql b/coderd/database/queries/provisionerjobs.sql
@@ -96,3 +96,13 @@ SET
 	error_code = $5
 WHERE
 	id = $1;
+
+-- name: GetHungProvisionerJobs :many
+SELECT
+	*
+FROM
+	provisioner_jobs
+WHERE
+	updated_at < $1
+	AND started_at IS NOT NULL
+	AND completed_at IS NULL;