Skip to content

Commit 5ec0997

Browse files
committed
fix(webapp,run-engine): clear stale metric rows on query change, document override cap
Address review findings on the queue metrics PR: - useMetricResourceQuery kept the previous query's rows when the signature changed with no cache entry for the new key, so showLoading (isLoading && !rows) stayed false and a chart painted the old time range as if it were the new one. A stale `failed` also survived, rendering an "invalid" chart state over a healthy new query. Reset both when the signature actually changes; interval and on-focus refreshes reuse the signature and are unaffected. - Document in the OpenAPI spec that the queue concurrency override rejects limits above the environment maximum with a 400 instead of capping them. - Correct the enqueueSystem TTL comment: the pending-version promotion does pass includeTtl, so it is not in the "must not add TTL" set.
1 parent 26783dd commit 5ec0997

3 files changed

Lines changed: 20 additions & 8 deletions

File tree

apps/webapp/app/hooks/useMetricResourceQuery.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,14 +83,21 @@ export function useMetricResourceQuery(query: string, opts: MetricResourceQueryO
8383
const [isLoading, setIsLoading] = useState(true);
8484
const [failed, setFailed] = useState(false);
8585
const abortRef = useRef<AbortController | null>(null);
86+
const loadedKeyRef = useRef<string | null>(null);
8687

8788
const load = useCallback(() => {
8889
abortRef.current?.abort();
8990
const controller = new AbortController();
9091
abortRef.current = controller;
91-
// Paint cached rows immediately (no skeleton) on remount / key change while we revalidate.
92-
const cached = responseCache.get(cacheKey);
93-
if (cached) setRows(cached);
92+
// On a new query signature the rows and failure on screen belong to a different query. Paint
93+
// this key's cached rows if we have them (no skeleton on remount / back-navigation), otherwise
94+
// clear them so a genuinely new query shows a loading state instead of another query's data.
95+
// Interval and on-focus refreshes reuse the same signature, so they keep what's on screen.
96+
if (loadedKeyRef.current !== cacheKey) {
97+
loadedKeyRef.current = cacheKey;
98+
setRows(responseCache.get(cacheKey) ?? null);
99+
setFailed(false);
100+
}
94101
setIsLoading(true);
95102
fetch("/resources/metric", {
96103
method: "POST",

docs/v3-openapi.yaml

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3001,7 +3001,10 @@ paths:
30013001
type: integer
30023002
minimum: 0
30033003
maximum: 100000
3004-
description: The new concurrency limit to set for the queue
3004+
description: |
3005+
The new concurrency limit to set for the queue. It may not exceed your
3006+
environment's maximum concurrency limit: a higher value is rejected with a
3007+
400, not capped to the maximum.
30053008
responses:
30063009
"200":
30073010
description: Concurrency limit overridden successfully
@@ -3010,7 +3013,9 @@ paths:
30103013
schema:
30113014
"$ref": "#/components/schemas/QueueObject"
30123015
"400":
3013-
description: Invalid request parameters
3016+
description: |
3017+
Invalid request parameters, or the requested concurrency limit exceeds the
3018+
environment's maximum concurrency limit.
30143019
"401":
30153020
description: Unauthorized request
30163021
"404":

internal-packages/run-engine/src/engine/systems/enqueueSystem.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,9 @@ export class EnqueueSystem {
105105
const timestamp = queuePositionMs - run.priorityMs;
106106
const eligibleAtMs = includeTtl ? queuePositionMs : Date.now();
107107

108-
// Include TTL only when explicitly requested (first enqueue from trigger or the
109-
// delayed-run system). Re-enqueues (waitpoint, checkpoint, pending version) must
110-
// not add TTL.
108+
// Include TTL only when explicitly requested: the first enqueue from trigger, the
109+
// delayed-run system, and the pending-version promotion (each is a run's first real
110+
// entry into the queue). Waitpoint and checkpoint re-enqueues must not add TTL.
111111
let ttlExpiresAt: number | undefined;
112112
if (includeTtl && run.ttl) {
113113
const expireAt = parseNaturalLanguageDuration(run.ttl);

0 commit comments

Comments
 (0)