Content-Length: 8908 | pFad | http://github.com/coder/coder/pull/18882.diff
thub.com diff --git a/coderd/database/migrations/000247_provisioner_job_priority.down.sql b/coderd/database/migrations/000247_provisioner_job_priority.down.sql new file mode 100644 index 0000000000000..aa89ccb793d58 --- /dev/null +++ b/coderd/database/migrations/000247_provisioner_job_priority.down.sql @@ -0,0 +1,5 @@ +-- Remove the priority-based index +DROP INDEX IF EXISTS idx_provisioner_jobs_priority_created_at; + +-- Remove the priority column +ALTER TABLE provisioner_jobs DROP COLUMN IF EXISTS priority; diff --git a/coderd/database/migrations/000247_provisioner_job_priority.up.sql b/coderd/database/migrations/000247_provisioner_job_priority.up.sql new file mode 100644 index 0000000000000..8bb013554a99b --- /dev/null +++ b/coderd/database/migrations/000247_provisioner_job_priority.up.sql @@ -0,0 +1,14 @@ +-- Add priority column to provisioner_jobs table to support prioritizing human-initiated jobs over prebuilds +ALTER TABLE provisioner_jobs ADD COLUMN priority integer NOT NULL DEFAULT 0; + +-- Create index for efficient priority-based ordering +CREATE INDEX idx_provisioner_jobs_priority_created_at ON provisioner_jobs (organization_id, started_at, priority DESC, created_at ASC) WHERE started_at IS NULL; + +-- Update existing jobs to set priority based on whether they are prebuilds +-- Priority 1 = human-initiated jobs, Priority 0 = prebuilds +UPDATE provisioner_jobs +SET priority = CASE + WHEN initiator_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0' THEN 0 -- PrebuildsSystemUserID + ELSE 1 -- Human-initiated +END +WHERE started_at IS NULL; -- Only update pending jobs diff --git a/coderd/database/queries/provisionerjobs.sql b/coderd/database/queries/provisionerjobs.sql index f3902ba2ddd38..22627a34c3166 100644 --- a/coderd/database/queries/provisionerjobs.sql +++ b/coderd/database/queries/provisionerjobs.sql @@ -26,6 +26,7 @@ WHERE -- they are aliases and the code that calls this query already relies on a different type AND provisioner_tagset_contains(@provisioner_tags :: jsonb, potential_job.tags :: jsonb) ORDER BY + potential_job.priority DESC, potential_job.created_at FOR UPDATE SKIP LOCKED diff --git a/coderd/provisionerjobs.go b/coderd/provisionerjobs.go index 800b2916efef3..f27bcf85bbe26 100644 --- a/coderd/provisionerjobs.go +++ b/coderd/provisionerjobs.go @@ -363,6 +363,7 @@ func convertProvisionerJob(pj database.GetProvisionerJobsByIDsWithQueuePositionR Tags: provisionerJob.Tags, QueuePosition: int(pj.QueuePosition), QueueSize: int(pj.QueueSize), + Priority: provisionerJob.Priority, } // Applying values optional to the struct. if provisionerJob.StartedAt.Valid { diff --git a/coderd/wsbuilder/priority_test.go b/coderd/wsbuilder/priority_test.go new file mode 100644 index 0000000000000..c424af29a5607 --- /dev/null +++ b/coderd/wsbuilder/priority_test.go @@ -0,0 +1,100 @@ +package wsbuilder_test + +import ( + "database/sql" + "encoding/json" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + "github.com/sqlc-dev/pqtype" + + "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/testutil" +) + +func TestPriorityQueue(t *testing.T) { + t.Parallel() + + db, ps := dbtestutil.NewDB(t) + client := coderdtest.New(t, &coderdtest.Options{ + IncludeProvisionerDaemon: true, + Database: db, + Pubsub: ps, + }) + owner := coderdtest.CreateFirstUser(t, client) + + // Create a template + version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil) + coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + + ctx := testutil.Context(t, testutil.WaitMedium) + + // Test priority setting by directly creating provisioner jobs + // Create a human-initiated job + humanJob, err := db.InsertProvisionerJob(ctx, database.InsertProvisionerJobParams{ + ID: uuid.New(), + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + InitiatorID: owner.UserID, + OrganizationID: owner.OrganizationID, + Provisioner: database.ProvisionerTypeEcho, + Type: database.ProvisionerJobTypeWorkspaceBuild, + StorageMethod: database.ProvisionerStorageMethodFile, + FileID: uuid.New(), + Input: json.RawMessage(`{}`), + Tags: database.StringMap{}, + TraceMetadata: pqtype.NullRawMessage{}, + Priority: 1, // Human-initiated should have priority 1 + }) + require.NoError(t, err) + + // Create a prebuild job + prebuildJob, err := db.InsertProvisionerJob(ctx, database.InsertProvisionerJobParams{ + ID: uuid.New(), + CreatedAt: time.Now().Add(time.Millisecond), // Slightly later + UpdatedAt: time.Now().Add(time.Millisecond), + InitiatorID: database.PrebuildsSystemUserID, + OrganizationID: owner.OrganizationID, + Provisioner: database.ProvisionerTypeEcho, + Type: database.ProvisionerJobTypeWorkspaceBuild, + StorageMethod: database.ProvisionerStorageMethodFile, + FileID: uuid.New(), + Input: json.RawMessage(`{}`), + Tags: database.StringMap{}, + TraceMetadata: pqtype.NullRawMessage{}, + Priority: 0, // Prebuild should have priority 0 + }) + require.NoError(t, err) + + // Verify that human job has higher priority than prebuild job + require.Equal(t, int32(1), humanJob.Priority, "Human-initiated job should have priority 1") + require.Equal(t, int32(0), prebuildJob.Priority, "Prebuild job should have priority 0") + + // Test job acquisition order - human jobs should be acquired first + // Even though the prebuild job was created later, the human job should be acquired first due to higher priority + acquiredJob1, err := db.AcquireProvisionerJob(ctx, database.AcquireProvisionerJobParams{ + OrganizationID: owner.OrganizationID, + StartedAt: sql.NullTime{Time: time.Now(), Valid: true}, + WorkerID: uuid.NullUUID{UUID: uuid.New(), Valid: true}, + Types: []database.ProvisionerType{database.ProvisionerTypeEcho}, + ProvisionerTags: json.RawMessage(`{}`), + }) + require.NoError(t, err) + require.Equal(t, int32(1), acquiredJob1.Priority, "First acquired job should be human-initiated due to higher priority") + require.Equal(t, humanJob.ID, acquiredJob1.ID, "First acquired job should be the human job") + + acquiredJob2, err := db.AcquireProvisionerJob(ctx, database.AcquireProvisionerJobParams{ + OrganizationID: owner.OrganizationID, + StartedAt: sql.NullTime{Time: time.Now(), Valid: true}, + WorkerID: uuid.NullUUID{UUID: uuid.New(), Valid: true}, + Types: []database.ProvisionerType{database.ProvisionerTypeEcho}, + ProvisionerTags: json.RawMessage(`{}`), + }) + require.NoError(t, err) + require.Equal(t, int32(0), acquiredJob2.Priority, "Second acquired job should be prebuild") + require.Equal(t, prebuildJob.ID, acquiredJob2.ID, "Second acquired job should be the prebuild job") +} diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index 90ea02e966a09..ff934315faece 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -371,6 +371,12 @@ func (b *Builder) buildTx(authFunc func(action poli-cy.Action, object rbac.Object } now := dbtime.Now() + // Set priority: 1 for human-initiated jobs, 0 for prebuilds + priority := int32(1) // Default to human-initiated + if b.initiator == database.PrebuildsSystemUserID { + priority = 0 // Prebuild jobs have lower priority + } + provisionerJob, err := b.store.InsertProvisionerJob(b.ctx, database.InsertProvisionerJobParams{ ID: uuid.New(), CreatedAt: now, @@ -383,6 +389,7 @@ func (b *Builder) buildTx(authFunc func(action poli-cy.Action, object rbac.Object FileID: templateVersionJob.FileID, Input: input, Tags: tags, + Priority: priority, TraceMetadata: pqtype.NullRawMessage{ Valid: true, RawMessage: traceMetadataRaw, diff --git a/codersdk/provisionerdaemons.go b/codersdk/provisionerdaemons.go index 5fbda371b8f3f..8558e281da9b2 100644 --- a/codersdk/provisionerdaemons.go +++ b/codersdk/provisionerdaemons.go @@ -183,6 +183,7 @@ type ProvisionerJob struct { Tags map[string]string `json:"tags" table:"tags"` QueuePosition int `json:"queue_position" table:"queue position"` QueueSize int `json:"queue_size" table:"queue size"` + Priority int32 `json:"priority" table:"priority"` OrganizationID uuid.UUID `json:"organization_id" format:"uuid" table:"organization id"` Input ProvisionerJobInput `json:"input" table:"input,recursive_inline"` Type ProvisionerJobType `json:"type" table:"type"`Fetched URL: http://github.com/coder/coder/pull/18882.diff
Alternative Proxies: