cockroachdb / cockroachdb/cockroach

sql: refactor multiple active portals implementation

Open
#139,971 0 comments 0 reactions 1 assignee Claimed by @yuzefovich View on GitHub
A-pausable-portals C-cleanup O-support P-3 T-sql-queries
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

The current implementation of "multiple active portals" feature turned out to be brittle and error-prone:
- we've seen many issues arising from it (#136516, #137597, #138112, #139647, etc), including on CC, that could lead to node crashes
- it complicated the already-complex connExecutor and had non-trivial overhead on the main query path, so we ended up duplicating some of it in #136861.

When discussing #136861 @michae2 shared an idea for alternative implementation for the multiple active portals feature: achieve the blocking / resuming behavior by spinning of `execStmtInOpenState` into a separate goroutine that communicates via a channel. I prototyped it in #136940, and I think it's quite promising. The code becomes cleaner, and we probably will be able to lift restrictions on the types of stmts that can be executed via the pausable portals model. The main complication with that approach is that we need to think through how to handle objects that are bound to the connExecutor to allow for concurrent access. For example, `connExecutor.planner` state needs to be copied ("forked") for the pausable portal (perhaps we'll also need to "reconcile" the forked planner used by the pausable portal with the main one, when the portal is closed).

Jira issue: CRDB-46936

Epic CRDB-25183

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.