clickhouse: tenant data can no longer forge an error code (#412)

[hshimizu]

Closes #412.

A successful query could be delivered to the caller as a fabricated error, with its rows lost.

Measured: a query returning 30,000 rows, no server-side error at all, came back as got 0 of 30000 rows and Code: 210. DB::Exception: …. The 210 is not the server's — it is tenant data, read out of a log line.

How

extract_exception runs per chunk. Its length-slicing arm requires the chunk to end with the exception trailer, true only of the final chunk, so every other chunk fell through to a text search for Code: over bytes that are tenant data. A row ending ))\n at a chunk end was enough.

Consequences, all measured:

• The rows are lost, not merely relabelled.
• 210 is retryable and the real codes usually are not, so the fabrication was retried.
• It reaches mark_unhealthy — tenant data could demote a healthy endpoint.
• Both production shapes reach it: traces/sql.rs projects payload last, so one span payload ending ))\n needs no alignment at all; the LogQL row shape reaches it via a RowBinary varint length prefix.

The realistic innocent case is a log store holding ClickHouse's own error text — those messages end ) and contain the exact prefix the search looked for.

The fix

Reassemble the exception frame in the vendored client, anchored on its opening, scanning at any offset:

opening   \r\n__exception__\r\n<tag>\r\nCode: N. DB::Exception: …
closing   …\n<len> <tag>\r\n__exception__\r\n

The two ends run in opposite orders — a fact that cost this issue two wrong claims, both withdrawn on the issue, before anyone captured a frame and looked at the bytes.

Bytes before the anchor are emitted as data, so rows already produced still reach the caller. A frame that never closes surfaces as an error with the server's own Code: N at byte 0. extract_exception_old stays, because a tag-absent server is still reachable when PULSUS_SKIP_DDL bypasses the version gate.

Faster, too: the anchor scan costs 44.7 µs per MiB against the 223.4 µs rfind it removes.

What the review changed

The first design checked only the start of each chunk — the same shape as the defect it fixed, a check that inspects one position. A frame opening after result data in the same chunk was missed entirely. Now it scans anywhere, with a straddle check so the anchor cannot hide across a boundary.

Gates, and what they are worth

Ten hermetic client-parser gates plus five live. Against the old parser, 5 of 10 hermetic and 3 of 13 live fail; AC4 and AC13 pass either way, which is what makes the rest meaningful.

AC13 validates the mock against the real server on every CI run — a raw-TCP capture asserting the shared frame builder reproduces a live frame byte-for-byte. It fails rather than skips on a buffered response, and a one-byte drift in the builder fails it. That exists because a mock checked only against our own reading of our own code is one edit from being a false gate.

AC14 uses the discriminating fixture. The obvious shape — data, then a frame straddling a boundary — passes on the broken design, because the second block ends with the close marker and the existing extractor recovers from that alone.

Stated, not closed

• Tag non-reuse is unestablished. ClickHouse documents no uniqueness contract; a 200-response census found 200 distinct tags but is observational and says so. A tenant storing a tag seen on an earlier response is the case this rests on.
• No measurement shows ClickHouse emitting <data><frame opening> in one chunk — AC14 gates the parser against a constructed shape and is labelled a client-parser gate.
• The cap path drops its buffer at 16 MiB.

PATCHES.md §2 records the re-vendor rule: re-check upstream's tag gating on every bump and drop this patch if upstream supplies it.