Skip to content

Commit 3418d9d

Browse files
committed
revert: keep backfill cursor decoder throwing on malformed input
Reverts 525e363 (graceful legacy-cursor handling). The legacy bare-id cursor only matters for a backfill in flight across the single deploy that changed the cursor format, and that backfill is internal and admin-triggered, so the one-time transition is handled operationally: do not run a backfill during the rollout deploy. Keeping decodeBackfillCursor throwing surfaces genuine cursor corruption loudly instead of silently restarting the window. The old keyset was id-ordered and the new one createdAt-ordered, so a stale cursor cannot be safely resumed anyway, only restarted.
1 parent 525e363 commit 3418d9d

2 files changed

Lines changed: 5 additions & 43 deletions

File tree

apps/webapp/app/services/runsBackfiller.server.ts

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,6 @@ export class RunsBackfillerService {
4747
// different ranges. RunStore merges the two tables only on a time-based
4848
// key, so order by createdAt and tiebreak on id within a timestamp.
4949
const keyset = cursor ? decodeBackfillCursor(cursor) : undefined;
50-
if (cursor && !keyset) {
51-
// Legacy/corrupt cursor: ignore it and restart the window (idempotent
52-
// re-backfill). Self-recovers a backfill in flight across the cursor
53-
// format change instead of throwing on every batch.
54-
this.logger.warn(
55-
"RunsBackfillerService: unparsable backfill cursor, restarting from window start",
56-
{ cursor }
57-
);
58-
}
5950

6051
const runs = await runStore.findRuns(
6152
{
@@ -131,19 +122,15 @@ export function encodeBackfillCursor(createdAt: Date, id: string): string {
131122
return `${createdAt.toISOString()}${BACKFILL_CURSOR_SEPARATOR}${id}`;
132123
}
133124

134-
export function decodeBackfillCursor(cursor: string): { createdAt: Date; id: string } | undefined {
125+
export function decodeBackfillCursor(cursor: string): { createdAt: Date; id: string } {
135126
const separatorIndex = cursor.indexOf(BACKFILL_CURSOR_SEPARATOR);
136127
const createdAt = separatorIndex === -1 ? new Date(NaN) : new Date(cursor.slice(0, separatorIndex));
137128
const id = separatorIndex === -1 ? "" : cursor.slice(separatorIndex + 1);
138129

139-
// A cursor with no separator is the pre-(createdAt, id) format (a bare run id,
140-
// e.g. a backfill that was in flight across this change), or otherwise corrupt.
141-
// The old id-only keyset can't be translated to the new (createdAt, id) order,
142-
// so return undefined and let the caller restart the window. Re-backfilling is
143-
// idempotent (ClickHouse ReplacingMergeTree keyed by run id), so the only cost
144-
// is redoing the already-done portion once.
145130
if (Number.isNaN(createdAt.getTime()) || id.length === 0) {
146-
return undefined;
131+
throw new Error(
132+
`RunsBackfillerService: malformed cursor "${cursor}" (expected "<createdAt>_<id>")`
133+
);
147134
}
148135

149136
return { createdAt, id };

apps/webapp/test/runsBackfiller.test.ts

Lines changed: 1 addition & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -9,38 +9,13 @@ vi.mock("~/db.server", () => ({
99
import { ClickHouse } from "@internal/clickhouse";
1010
import { replicationContainerTest } from "@internal/testcontainers";
1111
import { z } from "zod";
12-
import {
13-
RunsBackfillerService,
14-
decodeBackfillCursor,
15-
encodeBackfillCursor,
16-
} from "~/services/runsBackfiller.server";
12+
import { RunsBackfillerService } from "~/services/runsBackfiller.server";
1713
import { RunsReplicationService } from "~/services/runsReplicationService.server";
1814
import { createInMemoryTracing } from "./utils/tracing";
1915
import { TestReplicationClickhouseFactory } from "./utils/testReplicationClickhouseFactory";
2016

2117
vi.setConfig({ testTimeout: 60_000 });
2218

23-
describe("backfill cursor", () => {
24-
it("round-trips createdAt + id", () => {
25-
const createdAt = new Date("2026-06-23T00:00:00.000Z");
26-
const decoded = decodeBackfillCursor(encodeBackfillCursor(createdAt, "cmqpwioyy0009unul63v3mxw2"));
27-
expect(decoded?.createdAt.toISOString()).toBe(createdAt.toISOString());
28-
expect(decoded?.id).toBe("cmqpwioyy0009unul63v3mxw2");
29-
});
30-
31-
it("treats a legacy bare-id cursor (no separator) as undefined so the window restarts", () => {
32-
// Pre-(createdAt, id) format: a bare run id. Decoding must not throw — it
33-
// returns undefined so an in-flight backfill restarts the window instead of
34-
// failing every batch after the cursor-format change.
35-
expect(decodeBackfillCursor("cmqpwioyy0009unul63v3mxw2")).toBeUndefined();
36-
});
37-
38-
it("returns undefined for a corrupt cursor instead of throwing", () => {
39-
expect(decodeBackfillCursor("not-a-date_cmqpwioyy0009unul63v3mxw2")).toBeUndefined();
40-
expect(decodeBackfillCursor("_cmqpwioyy0009unul63v3mxw2")).toBeUndefined();
41-
});
42-
});
43-
4419
describe("RunsBackfillerService", () => {
4520
replicationContainerTest(
4621
"should backfill completed runs to clickhouse",

0 commit comments

Comments
 (0)