Skip to content

Commit e9ecce0

Browse files
committed
fix(webapp): strip only a poison run's output, not its error
Recovery emptied both `output` and `error` on the un-ingestable row while keeping `error_fingerprint`, so a stripped failed run still matched the error materialized views with no error content left. Those views pick their display columns with `any()` over the fingerprint group, so a single stripped run could retitle every run sharing that fingerprint to a generic type and message. In practice the un-ingestable JSON is the run's output, so the strip now touches only that column and leaves `error` and its fingerprint consistent with each other. A run whose error is itself un-ingestable no longer makes progress from the strip and is skipped by the allow_errors bail instead of landing without the error content its fingerprint promises.
1 parent 9da98a3 commit e9ecce0

2 files changed

Lines changed: 19 additions & 7 deletions

File tree

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,9 @@ export class RunsReplicationService {
205205
private _recoveryCapHits = 0;
206206

207207
/**
208-
* Counts rows that landed with their un-ingestable JSON column(s) stripped
209-
* (the run kept its status, only the output/payload content was lost).
208+
* Counts rows that landed with their un-ingestable `output` emptied (the run
209+
* kept its status, only the output content was lost; it still reads from
210+
* Postgres on the run detail page).
210211
*/
211212
private _rowsStripped = 0;
212213

@@ -1603,9 +1604,18 @@ function landedRowCount(groupSize: number, outcome: JsonParseRecoveryOutcome): n
16031604

16041605
const STRIPPED_JSON: { data: unknown } = { data: undefined };
16051606

1607+
/**
1608+
* Empties `output`, the run JSON that in practice exceeds what ClickHouse can
1609+
* ingest (a large or deeply nested task return value). `error` is deliberately
1610+
* left alone: emptying it while keeping `error_fingerprint` would let the row
1611+
* match the error materialized views with no error content, and since those
1612+
* views pick their display columns with `any()` over the fingerprint group, one
1613+
* stripped run could retitle every run sharing that fingerprint. A run whose
1614+
* `error` is itself un-ingestable therefore makes no progress here and falls
1615+
* through to the `allow_errors` bail, which skips just that row.
1616+
*/
16061617
function stripTaskRunJsonColumns(row: TaskRunInsertArray): TaskRunInsertArray {
16071618
const stripped = [...row] as TaskRunInsertArray;
16081619
stripped[TASK_RUN_INDEX.output] = STRIPPED_JSON;
1609-
stripped[TASK_RUN_INDEX.error] = STRIPPED_JSON;
16101620
return stripped;
16111621
}

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -232,10 +232,12 @@ export const DEFAULT_MAX_POISON_STRIPS = 1;
232232
* 2. On a parse error, `sanitizeRows` losslessly repairs what it can in place
233233
* (lone UTF-16 surrogates, out-of-range integers) and retries once.
234234
* 3. If the sanitizer can't help, follow ClickHouse's `at row N` hint to the
235-
* un-ingestable row and re-insert with that row's JSON column(s) emptied
236-
* via `stripJsonColumns`, up to `maxPoisonStrips` rows. Each stripped run
237-
* still lands (keeps its terminal status); only its un-ingestable JSON is
238-
* lost. `insertSync` disables parallel parsing so `at row N` is reliable.
235+
* un-ingestable row and re-insert with the caller-selected JSON column(s)
236+
* emptied via `stripJsonColumns`, up to `maxPoisonStrips` rows. Each
237+
* stripped run still lands (keeps its terminal status); only the emptied
238+
* content is lost. A row the caller's strip cannot make ingestable is
239+
* reported again, makes no progress, and falls through to step 4.
240+
* `insertSync` disables parallel parsing so `at row N` is reliable.
239241
* 4. Cost bound: once `maxPoisonStrips` rows have been stripped and the batch
240242
* STILL fails (or the failing row can't be located), stop stripping and
241243
* land the batch with one `allow_errors` insert — the stripped rows and

0 commit comments

Comments
 (0)