Skip to content

adapter: Stop waiting for a peek's optimization when the client goes away - #39584

Closed
ggevay wants to merge 7 commits into
MaterializeInc:mainfrom
ggevay:gabor/peek-seq-r4-frontend-peek-cancel
Closed

adapter: Stop waiting for a peek's optimization when the client goes away#39584
ggevay wants to merge 7 commits into
MaterializeInc:mainfrom
ggevay:gabor/peek-seq-r4-frontend-peek-cancel

Conversation

@ggevay

@ggevay ggevay commented Oct 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Stacked on #39579, #39580, #39581, #39582 and #39583. Review only the last commit.

execute_attempts already has a future that resolves when the client connection closes (the coordinator path selects on it to cancel its work), but the frontend peek sequencing ignored it (TODO(peek-seq): wire up cancel_future). Waiting for the optimizer now gives up when the client goes away, the same way it gives up on a statement timeout: the statement ends as canceled, and the optimization task finishes in the background.

Deliberately out of scope, both need a decision first:

  • CancelRequest during sequencing. A user's cancel request reaches the coordinator, which cancels registered peeks. A frontend peek that is still sequencing only sees it once it registers. Observing it earlier needs a coordinator round trip per SELECT (RegisterConnectionCancelWatch, as read-then-write does), which is a QPS cost.
  • statement_timeout for SELECT execution (database-issues#9947). The variable defaults to 60 s and is documented as bounding INSERT ... SELECT, UPDATE and DELETE. Applying it to SELECTs would cancel every SELECT longer than a minute, which is a product change.

🤖 Generated with Claude Code

@ggevay ggevay mentioned this pull request Oct 6, 2026
Closed
@ggevay
ggevay force-pushed the gabor/peek-seq-r4-frontend-peek-cancel branch from 6077bd1 to 4045ba4 Compare October 6, 2026 12:51
ggevay and others added 7 commits October 7, 2026 09:55
@ggevay @claude
adapter: Remove the coordinator peek sequencing
1c9e6fe 
Frontend peek sequencing has been the default since December 2025, so the
coordinator's peek path only ran for sessions started with
`enable_frontend_peek_sequencing` turned off. Remove the flag and the path.

Every statement kind that plans to a SELECT, SHOW COLUMNS, COPY TO, a
side-effecting function, or an EXPLAIN (FILTER PUSHDOWN) of a SELECT is
taken over by the frontend, so the coordinator arms for them now retire with
an internal error. Delete `sequence_peek`, `sequence_copy_to`,
`explain_peek`, `sequence_side_effecting_func`, the `PeekStage` state machine
and the optimizer wrapper only it used. The coordinator keeps what the
frontend still calls into: `implement_peek_plan` for slow-path peeks,
`implement_copy_to`, `execute_side_effecting_func`, and
`sequence_peek_timestamp`, which EXPLAIN TIMESTAMP uses.

Tests that switched the flag off now run the frontend path:
`test_expected_spans` looks for frontend spans, with a new debug-level span
on `try_frontend_peek_inner`, and `test_github_25388` drops its SELECT half,
which only held for the coordinator's post-optimization dependency check.
`transactions.slt` enables its test of an AS OF query outside the
transaction's timedomain, which was commented out until CI ran the frontend
sequencing. mzcompose pins the flag for older binaries.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ggevay @claude
adapter: Sequence EXPLAIN SUBSCRIBE in the frontend, remove coordinat…
fb280c8 
…or SUBSCRIBE sequencing

Frontend subscribes have been on by default since MaterializeInc#35756, and with the
coordinator peek path gone the only SUBSCRIBE-related statement the
coordinator still sequenced was EXPLAIN ... SUBSCRIBE. Sequence it in the
frontend peek path, alongside EXPLAIN of a SELECT, and remove the
coordinator's SUBSCRIBE state machine, keeping `implement_subscribe`, which
the frontend uses to ship the dataflow.

EXPLAIN SUBSCRIBE keeps the coordinator's behavior: it adds no transaction
operation and does not require a replica, EXPLAIN BROKEN renders despite a
failed LIR optimization, and an `UP TO` equal to the `AS OF` still raises the
`EqualSubscribeBounds` notice. `explain_subscribe_inner` renders the rows
with the features the subscribe was optimized with, as `explain_plan_inner`
does for a SELECT.

Remove the `enable_frontend_subscribes` dyncfg. mzcompose pins it for older
binaries.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ggevay @claude
adapter: Sequence EXPLAIN TIMESTAMP in the frontend, remove plan defe…
7edcea4 
…rral

EXPLAIN TIMESTAMP was the last statement that the coordinator sequenced
through timestamp selection. Sequence it in the frontend peek path, which
already does everything it needs: it optimizes the query locally (so that,
as before, the dependencies that decide the timestamp are those of the
optimized query), determines the timestamp with the frontend's read holds,
and asks the coordinator for the frontier explanation through the
`ExplainTimestamp` command that the timestamp notice already uses. As on the
coordinator, it requires no replica, skips the introspection-read checks,
does not require linearization, and rejects `AS DOT` before doing anything.

Remove the coordinator's EXPLAIN TIMESTAMP state machine and
`sequence_peek_timestamp`. With that, no plan that reaches the coordinator
reads a builtin table written at session startup, so `sequence_plan` no
longer defers plans for those writes: remove `DeferredPlan`, `defer_plan`,
`sequence_deferred_plan` and `Message::DeferredPlanReady`, and drop the
dependency set from `waiting_on_startup_appends`, which only the frontend
paths call now.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ggevay @claude
adapter: Log a peek's execution timestamp as soon as it is determined
89c22b0 
With the coordinator peek sequencing gone, the frontend no longer has to
mirror where that path logged `execution_timestamp`. Log it once, right after
timestamp determination, for every peek and SUBSCRIBE that executes at a
timestamp:

- Constant peeks that have a timestamp now record it. Constant queries without
  a timeline still have none.
- Queries without a timestamp that execute on a cluster (timestamp-independent
  ones, e.g. over a large `generate_series` view) record none instead of the
  maximum timestamp, like constant queries without a timeline.
- Slow-path peeks no longer rely on `implement_peek_plan` to log it.
- SUBSCRIBEs record their `AS OF` again, which the coordinator path did and
  the frontend path did not.
- A statement that fails after timestamp determination keeps its timestamp.

EXPLAIN and COPY TO still record none.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ggevay @claude
adapter: Merge implement_peek_plan into implement_slow_path_peek
6710bfa 
The frontend implements fast-path and constant peeks itself, so the
coordinator's `implement_peek_plan` only ever received slow-path plans from
`implement_slow_path_peek`. Fold its slow-path arm into
`implement_slow_path_peek`, delete its fast-path and constant arms and
`PlannedPeek`, and create the statement-logging guard only once the peek is
registered, so that no error path has to defuse it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ggevay @claude
adapter: Move create_peek_response_stream out of the coordinator
b5f250a 
It is a free function that both the frontend's fast path and the
coordinator's slow path use, and it touches no coordinator state. Move it to
`peek_client`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ggevay @claude
adapter: Stop waiting for a peek's optimization when the client goes …
77a25bc 
…away

`execute_attempts` passes the frontend peek sequencing the future that
resolves when the client connection closes, which the coordinator path
already selects on. Waiting for the optimizer now gives up on it, the same
way it gives up on a statement timeout: the statement ends as canceled and
the optimization task finishes in the background.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ggevay
ggevay force-pushed the gabor/peek-seq-r4-frontend-peek-cancel branch from 4045ba4 to 77a25bc Compare October 7, 2026 08:06
@ggevay ggevay closed this Oct 7, 2026
@ggevay

ggevay commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #39629, which is part of a native GitHub stack.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ggevay