Skip to content

Commit 0fe1404

Browse files
ibetitsmikeibetitsmike
committed
WIP
1 parent 18b809c commit 0fe1404

File tree

9 files changed

+99
-13
lines changed

9 files changed

+99
-13
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2300,6 +2300,13 @@ func (q *querier) GetProvisionerJobByID(ctx context.Context, id uuid.UUID) (data
23002300
return job, nil
23012301
}
23022302

2303+
func (q *querier) GetProvisionerJobByIDForUpdate(ctx context.Context, id uuid.UUID) (database.ProvisionerJob, error) {
2304+
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceProvisionerJobs); err != nil {
2305+
return database.ProvisionerJob{}, err
2306+
}
2307+
return q.db.GetProvisionerJobByIDForUpdate(ctx, id)
2308+
}
2309+
23032310
func (q *querier) GetProvisionerJobTimingsByJobID(ctx context.Context, jobID uuid.UUID) ([]database.ProvisionerJobTiming, error) {
23042311
_, err := q.GetProvisionerJobByID(ctx, jobID)
23052312
if err != nil {

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4444,6 +4444,9 @@ func (s *MethodTestSuite) TestSystemFunctions() {
44444444
VapidPrivateKey: "test",
44454445
}).Asserts(rbac.ResourceDeploymentConfig, policy.ActionUpdate)
44464446
}))
4447+
s.Run("GetProvisionerJobByIDForUpdate", s.Subtest(func(db database.Store, check *expects) {
4448+
check.Args(uuid.New()).Asserts(rbac.ResourceProvisionerJobs, policy.ActionRead).Errors(sql.ErrNoRows)
4449+
}))
44474450
}
44484451

44494452
func (s *MethodTestSuite) TestNotifications() {

‎coderd/database/dbmem/dbmem.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4625,6 +4625,13 @@ func (q *FakeQuerier) GetProvisionerJobByID(ctx context.Context, id uuid.UUID) (
46254625
return q.getProvisionerJobByIDNoLock(ctx, id)
46264626
}
46274627

4628+
func (q *FakeQuerier) GetProvisionerJobByIDForUpdate(ctx context.Context, id uuid.UUID) (database.ProvisionerJob, error) {
4629+
q.mutex.RLock()
4630+
defer q.mutex.RUnlock()
4631+
4632+
return q.getProvisionerJobByIDNoLock(ctx, id)
4633+
}
4634+
46284635
func (q *FakeQuerier) GetProvisionerJobTimingsByJobID(_ context.Context, jobID uuid.UUID) ([]database.ProvisionerJobTiming, error) {
46294636
q.mutex.RLock()
46304637
defer q.mutex.RUnlock()

‎coderd/database/dbmetrics/querymetrics.go

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

‎coderd/database/dbmock/dbmock.go

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

‎coderd/database/querier.go

Lines changed: 3 additions & 0 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: 40 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/provisionerjobs.sql

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,18 @@ FROM
4141
WHERE
4242
id = $1;
4343

44+
-- name: GetProvisionerJobByIDForUpdate :one
45+
-- Gets a single provisioner job by ID for update.
46+
-- This is used to securely reap jobs that have been hung/pending for a long time.
47+
SELECT
48+
*
49+
FROM
50+
provisioner_jobs
51+
WHERE
52+
id = $1
53+
FOR UPDATE
54+
SKIP LOCKED;
55+
4456
-- name: GetProvisionerJobsByIDs :many
4557
SELECT
4658
*
@@ -288,8 +300,7 @@ WHERE
288300
AND completed_at IS NULL
289301
)
290302
ORDER BY random()
291-
LIMIT @max_jobs
292-
FOR UPDATE SKIP LOCKED;
303+
LIMIT @max_jobs;
293304

294305
-- name: InsertProvisionerJobTimings :many
295306
INSERT INTO provisioner_job_timings (job_id, started_at, ended_at, stage, source, action, resource)

‎coderd/jobreaper/detector.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -239,18 +239,12 @@ func reapJob(ctx context.Context, log slog.Logger, db database.Store, pub pubsub
239239
var lowestLogID int64
240240

241241
err := db.InTx(func(db database.Store) error {
242-
locked, err := db.TryAcquireLock(ctx, database.GenLockID(fmt.Sprintf("reaper:%s", jobToReap.ID)))
243-
if err != nil {
244-
return xerrors.Errorf("acquire lock: %w", err)
245-
}
246-
if !locked {
247-
// This error is ignored.
248-
return acquireLockError{}
249-
}
250-
251242
// Refetch the job while we hold the lock.
252-
job, err := db.GetProvisionerJobByID(ctx, jobToReap.ID)
243+
job, err := db.GetProvisionerJobByIDForUpdate(ctx, jobToReap.ID)
253244
if err != nil {
245+
if xerrors.Is(err, sql.ErrNoRows) {
246+
return acquireLockError{}
247+
}
254248
return xerrors.Errorf("get provisioner job: %w", err)
255249
}
256250

0 commit comments

Comments
 (0)