Skip to content

Commit db0acab

Browse files
committed
fix(webapp,clickhouse): stop one un-ingestable JSON row dropping its whole ClickHouse batch
When a single run output, trace span, or payload contained JSON ClickHouse could not ingest (for example nesting past its depth limit), the whole insert batch was rejected and those runs and spans silently vanished from the runs list, traces, and logs. Recovery is now per-table: - Runs: follow ClickHouse's failing-row hint to strip just the un-ingestable JSON column(s) so the run still lands with its status, up to a configurable limit (RUN_REPLICATION_MAX_POISON_STRIPS_PER_BATCH, default 1). Past the limit, land the rest with allow_errors and skip the remainder, so recovery cost stays flat on large flushes instead of re-sending the batch per row. - Trace events and payloads: land the batch with allow_errors so the good rows land in one pass and only the un-ingestable rows are skipped. Reading the failing-row hint needs a patch to @clickhouse/client-common, whose error parser otherwise discards the row number from the server response.
1 parent c5f1734 commit db0acab

15 files changed

Lines changed: 730 additions & 273 deletions

.server-changes/fix-runs-list-batch-drop-on-unparseable-json.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@ area: webapp
33
type: fix
44
---
55

6-
Fixed a rare case where a single run or span carrying data that could not be ingested would make other runs or trace events in the same batch go missing from the runs list, traces, and logs. Now the whole batch is kept: the affected item still appears (a run keeps its status, a span keeps its place in the trace) with only its un-ingestable content dropped, and everything else is stored normally.
6+
Fixed a rare case where a single run or span carrying data that could not be ingested would make other runs or trace events in the same batch go missing from the runs list, traces, and logs. Now the rest of the batch is always kept: an affected run still appears with its status (only its un-ingestable output is dropped), and an affected trace event or payload is skipped instead of taking down everything around it.

apps/webapp/app/env.server.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1672,6 +1672,7 @@ const EnvironmentSchema = z
16721672
RUN_REPLICATION_MAX_FLUSH_CONCURRENCY: z.coerce.number().int().default(2),
16731673
RUN_REPLICATION_FLUSH_INTERVAL_MS: z.coerce.number().int().default(1000),
16741674
RUN_REPLICATION_FLUSH_BATCH_SIZE: z.coerce.number().int().default(100),
1675+
RUN_REPLICATION_MAX_POISON_STRIPS_PER_BATCH: z.coerce.number().int().default(1),
16751676
RUN_REPLICATION_LEADER_LOCK_TIMEOUT_MS: z.coerce.number().int().default(30_000),
16761677
RUN_REPLICATION_LEADER_LOCK_EXTEND_INTERVAL_MS: z.coerce.number().int().default(10_000),
16771678
RUN_REPLICATION_ACK_INTERVAL_SECONDS: z.coerce.number().int().default(10),

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ function initializeRunsReplicationInstance() {
117117
maxFlushConcurrency: env.RUN_REPLICATION_MAX_FLUSH_CONCURRENCY,
118118
flushIntervalMs: env.RUN_REPLICATION_FLUSH_INTERVAL_MS,
119119
flushBatchSize: env.RUN_REPLICATION_FLUSH_BATCH_SIZE,
120+
maxPoisonStripsPerBatch: env.RUN_REPLICATION_MAX_POISON_STRIPS_PER_BATCH,
120121
leaderLockTimeoutMs: env.RUN_REPLICATION_LEADER_LOCK_TIMEOUT_MS,
121122
leaderLockExtendIntervalMs: env.RUN_REPLICATION_LEADER_LOCK_EXTEND_INTERVAL_MS,
122123
leaderLockAcquireAdditionalTimeMs: env.RUN_REPLICATION_LEADER_LOCK_ADDITIONAL_TIME_MS,

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

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import {
77
composeTaskRunVersion,
88
getPayloadField,
99
getTaskRunField,
10-
PAYLOAD_INDEX,
1110
TASK_RUN_INDEX,
1211
} from "@internal/clickhouse";
1312
import { type RedisOptions } from "@internal/redis";
@@ -44,7 +43,8 @@ import { detectBadJsonStrings } from "~/utils/detectBadJsonStrings";
4443
import { calculateErrorFingerprint } from "~/utils/errorFingerprinting";
4544
import { baseWorkerQueue } from "~/runEngine/concerns/workerQueueSplit.server";
4645
import {
47-
insertWithJsonParseRecovery,
46+
insertWithBadRowSkip,
47+
insertWithLimitedStrip,
4848
type JsonParseRecoveryOutcome,
4949
} from "~/v3/eventRepository/sanitizeRowsOnParseError.server";
5050

@@ -114,6 +114,7 @@ export type RunsReplicationServiceOptions = {
114114
insertMaxDelayMs?: number;
115115
disablePayloadInsert?: boolean;
116116
disableErrorFingerprinting?: boolean;
117+
maxPoisonStripsPerBatch?: number;
117118
};
118119

119120
type PostgresTaskRun = TaskRun & { masterQueue: string };
@@ -1133,14 +1134,24 @@ export class RunsReplicationService {
11331134
return insertResult;
11341135
};
11351136

1136-
const outcome = await insertWithJsonParseRecovery({
1137+
const outcome = await insertWithLimitedStrip({
11371138
rows: taskRunInserts,
11381139
contextLabel: "task_runs_v2",
11391140
logger: this.logger,
11401141
logContext: { attempt },
11411142
insert: (rows) => rawInsert(rows),
1142-
insertSync: (rows) => rawInsert(rows, { async_insert: 0 }),
1143+
insertSync: (rows) =>
1144+
rawInsert(rows, { async_insert: 0, input_format_parallel_parsing: 0 }),
1145+
insertAllowingBadRows: (rows) =>
1146+
rawInsert(rows, {
1147+
async_insert: 0,
1148+
input_format_parallel_parsing: 0,
1149+
input_format_allow_errors_num: String(rows.length),
1150+
input_format_allow_errors_ratio: 1,
1151+
}),
11431152
stripJsonColumns: stripTaskRunJsonColumns,
1153+
maxPoisonStrips: this.options.maxPoisonStripsPerBatch,
1154+
hasMaterializedViews: true,
11441155
});
11451156
this.#recordRecoveryOutcome(outcome, "task_runs_v2", taskRunInserts.length);
11461157
return outcome;
@@ -1176,14 +1187,20 @@ export class RunsReplicationService {
11761187
return insertResult;
11771188
};
11781189

1179-
const outcome = await insertWithJsonParseRecovery({
1190+
const outcome = await insertWithBadRowSkip({
11801191
rows: payloadInserts,
11811192
contextLabel: "raw_task_runs_payload_v1",
11821193
logger: this.logger,
11831194
logContext: { attempt },
11841195
insert: (rows) => rawInsert(rows),
1185-
insertSync: (rows) => rawInsert(rows, { async_insert: 0 }),
1186-
stripJsonColumns: stripPayloadJsonColumns,
1196+
insertAllowingBadRows: (rows) =>
1197+
rawInsert(rows, {
1198+
async_insert: 0,
1199+
input_format_parallel_parsing: 0,
1200+
input_format_allow_errors_num: String(rows.length),
1201+
input_format_allow_errors_ratio: 1,
1202+
}),
1203+
hasMaterializedViews: false,
11871204
});
11881205
this.#recordRecoveryOutcome(outcome, "raw_task_runs_payload_v1", payloadInserts.length);
11891206
return outcome;
@@ -1587,9 +1604,3 @@ function stripTaskRunJsonColumns(row: TaskRunInsertArray): TaskRunInsertArray {
15871604
stripped[TASK_RUN_INDEX.error] = STRIPPED_JSON;
15881605
return stripped;
15891606
}
1590-
1591-
function stripPayloadJsonColumns(row: PayloadInsertArray): PayloadInsertArray {
1592-
const stripped = [...row] as PayloadInsertArray;
1593-
stripped[PAYLOAD_INDEX.payload] = STRIPPED_JSON;
1594-
return stripped;
1595-
}

apps/webapp/app/v3/eventRepository/clickhouseEventRepository.server.ts

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ import type {
6767
TraceSummary,
6868
} from "./eventRepository.types";
6969
import {
70-
insertWithJsonParseRecovery,
70+
insertWithBadRowSkip,
7171
type JsonParseRecoveryOutcome,
7272
} from "./sanitizeRowsOnParseError.server";
7373

@@ -345,14 +345,19 @@ export class ClickhouseEventRepository implements IEventRepository {
345345
return insertResult;
346346
};
347347

348-
const outcome = await insertWithJsonParseRecovery({
348+
const outcome = await insertWithBadRowSkip({
349349
rows: events,
350350
contextLabel,
351351
logger,
352352
logContext: { flushId, version: this._version },
353353
insert: (rows) => rawInsert(rows),
354-
insertSync: (rows) => rawInsert(rows, { async_insert: 0 }),
355-
stripJsonColumns: stripTaskEventJsonColumns,
354+
insertAllowingBadRows: (rows) =>
355+
rawInsert(rows, {
356+
async_insert: 0,
357+
input_format_parallel_parsing: 0,
358+
input_format_allow_errors_num: String(rows.length),
359+
input_format_allow_errors_ratio: 1,
360+
}),
356361
});
357362
this.#recordRecoveryOutcome(outcome, contextLabel, events.length);
358363

@@ -377,14 +382,19 @@ export class ClickhouseEventRepository implements IEventRepository {
377382
return insertResult;
378383
};
379384

380-
const outcome = await insertWithJsonParseRecovery({
385+
const outcome = await insertWithBadRowSkip({
381386
rows,
382387
contextLabel: "llm_metrics_v1",
383388
logger,
384389
logContext: { flushId },
385390
insert: (batch) => rawInsert(batch),
386-
insertSync: (batch) => rawInsert(batch, { async_insert: 0 }),
387-
stripJsonColumns: (row) => row,
391+
insertAllowingBadRows: (batch) =>
392+
rawInsert(batch, {
393+
async_insert: 0,
394+
input_format_parallel_parsing: 0,
395+
input_format_allow_errors_num: String(batch.length),
396+
input_format_allow_errors_ratio: 1,
397+
}),
388398
});
389399
this.#recordRecoveryOutcome(outcome, "llm_metrics_v1", rows.length);
390400

@@ -2945,7 +2955,3 @@ function formatClickhouseUnsignedIntegerString(value: number | bigint): string {
29452955

29462956
return Math.floor(value).toString();
29472957
}
2948-
2949-
function stripTaskEventJsonColumns<T extends { attributes?: unknown }>(row: T): T {
2950-
return { ...row, attributes: {} };
2951-
}

0 commit comments

Comments
 (0)