Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .server-changes/clamp-run-priority.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
area: webapp
type: fix
---

Triggering a run with a very large `priority` no longer fails. The priority is now capped to the highest supported value instead of erroring out.
5 changes: 4 additions & 1 deletion apps/webapp/app/runEngine/services/triggerTask.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import type {
TriggerTaskServiceResult,
} from "../../v3/services/triggerTask.server";
import { clampMaxDuration } from "../../v3/utils/maxDuration";
import { clampPriorityMs } from "../../v3/utils/priority";
import {
type IdempotencyKeyConcern,
type ClaimedIdempotency,
Expand Down Expand Up @@ -887,7 +888,9 @@ export class RunEngineTriggerTaskService {
? clampMaxDuration(args.body.options.maxDuration)
: undefined,
machine: args.body.options?.machine,
priorityMs: args.body.options?.priority ? args.body.options.priority * 1_000 : undefined,
priorityMs: args.body.options?.priority
? clampPriorityMs(args.body.options.priority)
: undefined,
queueTimestamp:
args.options.queueTimestamp ??
(args.parentRun && args.body.options?.resumeParentOnCompletion
Expand Down
41 changes: 41 additions & 0 deletions apps/webapp/app/v3/utils/priority.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { describe, expect, it } from "vitest";
import { clampPriorityMs } from "./priority";

const INT4_MIN = -2_147_483_648;
const INT4_MAX = 2_147_483_647;

describe("clampPriorityMs", () => {
it("converts seconds to milliseconds for in-range values", () => {
expect(clampPriorityMs(10)).toBe(10_000);
expect(clampPriorityMs(0.5)).toBe(500);
});

it("rounds a sub-millisecond fractional priority to an integer", () => {
expect(clampPriorityMs(0.0005)).toBe(1);
expect(clampPriorityMs(0.00049)).toBe(0);
expect(Number.isInteger(clampPriorityMs(0.0005))).toBe(true);
});

it("clamps a value that would overflow INT4 down to the column max", () => {
const priority = 31_536_000;
expect(priority * 1_000).toBeGreaterThan(INT4_MAX);
expect(clampPriorityMs(priority)).toBe(INT4_MAX);
});

it("leaves the largest safe priority untouched", () => {
expect(clampPriorityMs(2_147_483)).toBe(2_147_483_000);
});

it("clamps a large negative priority to the column min", () => {
expect(clampPriorityMs(-3_000_000)).toBe(INT4_MIN);
});

it("keeps every result inside the INT4 range", () => {
for (const priority of [-1e12, -5, -0.3, 0, 0.7, 5, 1234.5678, 1e12]) {
const result = clampPriorityMs(priority);
expect(Number.isInteger(result)).toBe(true);
expect(result).toBeGreaterThanOrEqual(INT4_MIN);
expect(result).toBeLessThanOrEqual(INT4_MAX);
}
});
});
6 changes: 6 additions & 0 deletions apps/webapp/app/v3/utils/priority.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
const INT4_MIN = -2_147_483_648;
const INT4_MAX = 2_147_483_647;

export function clampPriorityMs(priority: number): number {
return Math.min(Math.max(Math.round(priority * 1_000), INT4_MIN), INT4_MAX);
}
Comment thread
ericallam marked this conversation as resolved.
Loading