Repository navigation
Conversation
This was referenced Oct 6, 2026
ggevay
force-pushed
the
gabor/peek-seq-r2-merge-implement-peek-plan
branch
from
October 6, 2026 12:51
22e78ec to
7a84a97
Compare
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>
…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>
…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>
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>
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>
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
force-pushed
the
gabor/peek-seq-r2-merge-implement-peek-plan
branch
from
October 7, 2026 08:06
7a84a97 to
b5f250a
Compare
Contributor
Author
|
Superseded by #39628, which is part of a native GitHub stack. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #39579, #39580, #39581 and #39582. Review only the last 2 commits.
implement_peek_planintoimplement_slow_path_peek. The frontend implements fast-path and constant peeks itself, so the coordinator'simplement_peek_planonly ever received slow-path plans. Its slow-path arm moves intoimplement_slow_path_peek. Its fast-path and constant arms andPlannedPeekare deleted. The statement-logging guard is created only once the peek is registered, so no error path has to defuse it.create_peek_response_streamtopeek_client. It is a free function used by both the frontend fast path and the coordinator slow path, and touches no coordinator state. Pure move (git show --color-moved).No behavior change intended.
🤖 Generated with Claude Code