Skip to content

Commit 0183707

Browse files
committed
fix(webapp): stop dropping the newest bucket from queue backlog sparklines
The sparkline grid floors its start and ceils its end onto the bucket boundary so repeated loads share ClickHouse query-cache entries, but the bucket count was measured from the raw requested range. Since the range starts at "now minus the period", it is almost never already on a boundary, so the aligned grid spans one more bucket than the count allowed and the newest bucket was discarded on every load, for every period. Depth is forward filled, so the sparkline showed the previous bucket's depth rather than looking broken, and a throttle in the newest bucket was invisible. The grid arithmetic moves to a pure module so it can be tested, and the count is now measured across the aligned grid.
1 parent d2656b3 commit 0183707

3 files changed

Lines changed: 129 additions & 12 deletions

File tree

apps/webapp/app/presenters/v3/QueueMetricsPresenter.server.ts

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { type AuthenticatedEnvironment } from "~/services/apiAuth.server";
22
import { clickhouseFactory } from "~/services/clickhouse/clickhouseFactoryInstance.server";
33
import { logger } from "~/services/logger.server";
4+
import { computeSparklineGrid } from "~/v3/queueSparklineGrid";
45

56
export type QueueListMetric = {
67
p50WaitMs: number | null;
@@ -24,8 +25,6 @@ export type QueueListMetrics = {
2425
byQueue: Map<string, QueueListMetric>;
2526
};
2627

27-
const SPARKLINE_POINTS = 48;
28-
2928
function formatClickhouseDateTime(date: Date): string {
3029
return date.toISOString().slice(0, 19).replace("T", " ");
3130
}
@@ -51,13 +50,9 @@ export class QueueMetricsPresenter {
5150
from: Date;
5251
to: Date;
5352
}): Promise<QueueListMetrics> {
54-
const rangeSeconds = Math.max(60, Math.round((to.getTime() - from.getTime()) / 1000));
55-
const bucketSeconds = Math.max(60, Math.round(rangeSeconds / SPARKLINE_POINTS));
56-
const numBuckets = Math.max(1, Math.ceil(rangeSeconds / bucketSeconds));
57-
const gridStartSeconds =
58-
Math.floor(Math.floor(from.getTime() / 1000) / bucketSeconds) * bucketSeconds;
59-
const bucketStartMs = gridStartSeconds * 1000;
60-
const bucketIntervalMs = bucketSeconds * 1000;
53+
const grid = computeSparklineGrid(from, to);
54+
const { bucketSeconds, bucketIntervalMs, bucketStartMs } = grid;
55+
const numBuckets = grid.bucketCount;
6156

6257
const empty: QueueListMetrics = {
6358
bucketStartMs,
@@ -75,9 +70,7 @@ export class QueueMetricsPresenter {
7570
"queueMetrics"
7671
);
7772

78-
// End bound snaps up to the bucket grid so repeated loads within a bucket produce
79-
// identical params and share ClickHouse query-cache entries.
80-
const endMs = Math.ceil(to.getTime() / bucketIntervalMs) * bucketIntervalMs;
73+
const endMs = grid.endMs;
8174
const ids = {
8275
organizationId: environment.organizationId,
8376
projectId: environment.projectId,
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/**
2+
* Bucket grid for the Queues list sparklines. Pure so it can be unit tested: the presenter that
3+
* uses it reaches ClickHouse, and the grid arithmetic is where the off-by-one lives.
4+
*
5+
* The grid start floors to a bucket boundary and the end ceils to one, so repeated loads inside a
6+
* bucket produce identical query params and share ClickHouse query-cache entries. That means the
7+
* span covered by the grid is wider than the requested range whenever `from` is not already on a
8+
* boundary, which is almost always. `bucketCount` is therefore measured from the aligned grid
9+
* start rather than from the raw range, otherwise the newest bucket lands at an index the caller
10+
* treats as out of range and its data is silently dropped.
11+
*/
12+
13+
export const SPARKLINE_POINTS = 48;
14+
15+
const MIN_RANGE_SECONDS = 60;
16+
const MIN_BUCKET_SECONDS = 60;
17+
18+
export type SparklineGrid = {
19+
bucketSeconds: number;
20+
bucketIntervalMs: number;
21+
/** Aligned grid start; at or before the requested `from`. */
22+
bucketStartMs: number;
23+
/** Aligned grid end; at or after the requested `to`. */
24+
endMs: number;
25+
/** Buckets spanning bucketStartMs..endMs, so every bucket the query can return has an index. */
26+
bucketCount: number;
27+
};
28+
29+
export function computeSparklineGrid(from: Date, to: Date): SparklineGrid {
30+
const rangeSeconds = Math.max(
31+
MIN_RANGE_SECONDS,
32+
Math.round((to.getTime() - from.getTime()) / 1000)
33+
);
34+
const bucketSeconds = Math.max(MIN_BUCKET_SECONDS, Math.round(rangeSeconds / SPARKLINE_POINTS));
35+
const bucketIntervalMs = bucketSeconds * 1000;
36+
37+
const gridStartSeconds =
38+
Math.floor(Math.floor(from.getTime() / 1000) / bucketSeconds) * bucketSeconds;
39+
const bucketStartMs = gridStartSeconds * 1000;
40+
const endMs = Math.ceil(to.getTime() / bucketIntervalMs) * bucketIntervalMs;
41+
42+
const bucketCount = Math.max(1, Math.round((endMs - bucketStartMs) / bucketIntervalMs));
43+
44+
return { bucketSeconds, bucketIntervalMs, bucketStartMs, endMs, bucketCount };
45+
}
46+
47+
/** Index of a bucket on the grid, or null when it falls outside it. */
48+
export function bucketIndex(grid: SparklineGrid, bucketMs: number): number | null {
49+
const index = Math.round((bucketMs - grid.bucketStartMs) / grid.bucketIntervalMs);
50+
if (index < 0 || index >= grid.bucketCount) {
51+
return null;
52+
}
53+
return index;
54+
}
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import { describe, expect, it } from "vitest";
2+
import { bucketIndex, computeSparklineGrid, SPARKLINE_POINTS } from "~/v3/queueSparklineGrid";
3+
4+
const PERIODS_MS = {
5+
"30m": 30 * 60 * 1000,
6+
"1h": 60 * 60 * 1000,
7+
"1d": 24 * 60 * 60 * 1000,
8+
"7d": 7 * 24 * 60 * 60 * 1000,
9+
"30d": 30 * 24 * 60 * 60 * 1000,
10+
};
11+
12+
/** Newest bucket the ClickHouse query can return: the last full slot before the grid end. */
13+
function newestBucketMs(grid: ReturnType<typeof computeSparklineGrid>): number {
14+
return grid.endMs - grid.bucketIntervalMs;
15+
}
16+
17+
describe("computeSparklineGrid", () => {
18+
it("keeps the newest bucket for every period, at any start offset", () => {
19+
for (const [label, span] of Object.entries(PERIODS_MS)) {
20+
for (let offset = 0; offset < 120; offset++) {
21+
const toMs = 1785000000000 + offset * 1371;
22+
const grid = computeSparklineGrid(new Date(toMs - span), new Date(toMs));
23+
24+
expect(
25+
bucketIndex(grid, newestBucketMs(grid)),
26+
`${label} dropped its newest bucket at offset ${offset}`
27+
).not.toBeNull();
28+
}
29+
}
30+
});
31+
32+
it("indexes every bucket the query can return, and nothing outside the grid", () => {
33+
const toMs = 1785000000123;
34+
const grid = computeSparklineGrid(new Date(toMs - PERIODS_MS["1d"]), new Date(toMs));
35+
36+
expect(bucketIndex(grid, grid.bucketStartMs)).toBe(0);
37+
expect(bucketIndex(grid, newestBucketMs(grid))).toBe(grid.bucketCount - 1);
38+
expect(bucketIndex(grid, grid.bucketStartMs - grid.bucketIntervalMs)).toBeNull();
39+
expect(bucketIndex(grid, grid.endMs)).toBeNull();
40+
});
41+
42+
it("aligns the grid outward from the requested range", () => {
43+
const from = new Date(1785000000123);
44+
const to = new Date(from.getTime() + PERIODS_MS["1h"]);
45+
const grid = computeSparklineGrid(from, to);
46+
47+
expect(grid.bucketStartMs).toBeLessThanOrEqual(from.getTime());
48+
expect(grid.endMs).toBeGreaterThanOrEqual(to.getTime());
49+
expect(grid.bucketStartMs % grid.bucketIntervalMs).toBe(0);
50+
expect(grid.endMs % grid.bucketIntervalMs).toBe(0);
51+
});
52+
53+
it("targets the sparkline resolution without going below a one minute bucket", () => {
54+
const toMs = 1785000000000;
55+
const dayGrid = computeSparklineGrid(new Date(toMs - PERIODS_MS["1d"]), new Date(toMs));
56+
expect(dayGrid.bucketSeconds).toBe(1800);
57+
expect(dayGrid.bucketCount).toBeGreaterThanOrEqual(SPARKLINE_POINTS);
58+
59+
const shortGrid = computeSparklineGrid(new Date(toMs - 60_000), new Date(toMs));
60+
expect(shortGrid.bucketSeconds).toBe(60);
61+
expect(shortGrid.bucketCount).toBeGreaterThanOrEqual(1);
62+
});
63+
64+
it("never returns a zero-bucket grid for a degenerate range", () => {
65+
const at = new Date(1785000000000);
66+
const grid = computeSparklineGrid(at, at);
67+
expect(grid.bucketCount).toBeGreaterThanOrEqual(1);
68+
expect(grid.bucketSeconds).toBe(60);
69+
});
70+
});

0 commit comments

Comments
 (0)