compute: add configurable peek row iteration limit

[aljoscha]

Motivation

Compute workers synchronously iterate arrangements while fulfilling index-backed peeks. A query that scans far more rows than it returns can hold a worker for a long time and delay everything else on the cluster. Persist fast-path peeks have the same shape, since filtering happens after the rows have been read.

This change adds an off-by-default failsafe that bounds how many rows a worker may examine for one peek.

Description

Two dynamic configuration parameters: a feature gate that defaults to false, and a per-worker row threshold that defaults to 1,000. Both are read through handles, so an UpdateConfiguration reaches peeks that are already in flight without resetting work they have already performed.

The budget covers the index result trace, the index error trace and the Persist fast path, and it counts rows before literal and MFP filtering: a row that is read and then discarded costs the same scan time as one that is returned. Exactly the configured number of rows may be examined, and a peek fails only when it asks for the row after that.

The limit deliberately stops at the peek stash. A stashed peek restarts its scan and produces in bounded bursts, so bounding it means carrying the count across the hand-off, and the restart then charges the same rows twice. That is worth doing properly rather than quickly, and it can be added later. The peeks that motivate the failsafe, large filtered scans, fail well before they reach the stash threshold.

Structured peek errors. Reporting the limit needs an error type that survives the trip from the worker. PeekResponse::Error carried a bare String, so every peek failure reached the adapter as AdapterError::Unstructured and was reported as XX000. It now carries a PeekError (Dataflow / Unstructured / RowIterationLimitExceeded) and PeekResponseUnary::Error carries an AdapterError, so the conversion happens once instead of once per frontend. The limit reports SQLSTATE 54000 (PROGRAM_LIMIT_EXCEEDED) with detail about the per-worker scan and a hint naming the threshold parameter. Worker responses merge by precedence: cancellation, then ordinary errors, then the limit.

Two user-visible consequences of carrying the dataflow error structurally:

• Evaluation errors raised while reading a collection get a precise SQLSTATE. SELECT a / b FROM t reports 22012, matching its constant-folded counterpart, instead of XX000.
• Such an error renders the way DataflowError renders it, so one that used to come back bare from an index or Persist fast-path MFP gains the Evaluation error: prefix that the error-trace path already produced.

The compute protocol is bincode, which cannot skip an unknown variant, so PeekResponse serializes through a mirror type that keeps Error(String) in place for the unstructured case and appends the structured one. Existing frames, Canceled in particular, encode exactly as before, and a test asserts that.

The test and CI configuration enables the feature with a high threshold, so existing suites exercise the guarded path without constraining ordinary queries.

Verification

sqllogictest coverage exercises enabled and disabled behavior, the exact bound, filtered rows, and index-backed and Persist fast-path peeks. Unit coverage verifies dynamic configuration updates, structured error metadata, deterministic multi-worker response precedence, and that the wire encoding of the pre-existing response variants is unchanged. test_dataflow_error_codes covers the fast-path and error-trace SQLSTATEs end to end.

The full suite is left to CI, which is also where the Evaluation error: message change will surface if any golden still expects the bare form.

[aljoscha]

this all feels a bit too bespoke, and maybe nudges us in the direction of instead making the existing error type properly typed, and we fix this once and for all instead of glomming on workaround solutions

[aljoscha]

Agreed, and done in 380031f — the bespoke accumulator is gone.

PeekResponse::Error now carries a PeekError (Dataflow / Unstructured / RowIterationLimitExceeded) and PeekResponseUnary::Error carries an AdapterError, so compute-to-adapter conversion happens once instead of once per frontend. The row limit is just one variant of that type now, so PendingPeek is back to eager merging plus error-vs-error precedence.

One thing that did have to stay: the running inline_byte_len. Merging drops a worker's rows as soon as any worker errors, so without tracking the byte total separately, whether a peek reports the aggregate max-result-size error depends on the order the responses arrive in. The permutation test covers that.

Falling out of this: evaluation errors from reading a collection now get a real SQLSTATE (SELECT a / b FROM t -> 22012 instead of XX000), which is what the typed error was worth doing for.

[aljoscha]

Scope trimmed and the branch is now a single commit.

The row iteration limit no longer follows a peek into the response stash. Bounding a stashed peek means carrying the count across the hand-off, and since the stash restarts the scan from the beginning, the prefix then gets charged twice. Doing that properly means handing the existing iterator and its buffered prefix to StashingPeek instead of building a fresh cursor, which is a bigger change than the failsafe itself. The queries this protects against, large filtered scans, fail long before they reach the stash threshold, so the guard still does its job. There is a NOTE at the hand-off and a sentence in the dyncfg description recording the gap.

That also removed the speculative-production bound in pump_rows, which only existed so that stash batching could not trip the limit on rows the finishing did not need.

Also folded the structured-error work into the same commit, since it reads as an add-then-rewrite otherwise: the typed PeekError is what makes the limit reportable, so they belong together.