Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sql: support pgwire query cancellation #41335

Closed
rafiss opened this issue Oct 4, 2019 · 4 comments · Fixed by #67501
Closed

sql: support pgwire query cancellation #41335

rafiss opened this issue Oct 4, 2019 · 4 comments · Fixed by #67501
Assignees
@rafiss
Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL A-sql-pgwire pgwire protocol issues. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@rafiss
Copy link
Collaborator

rafiss commented Oct 4, 2019 •
edited by elinorgarcia
Loading

Postgres docs describe how BackendKeyData is initialized by the DB on startup and sent to the client so it can be used for cancellation requests later.

https://www.postgresql.org/docs/12/protocol-flow.html

BackendKeyData

    This message provides secret-key data that the frontend must save if it wants to be able to issue cancel requests later. The frontend should not respond to this message, but should continue listening for a ReadyForQuery message.

CockroachDB has a separate cancel query syntax. For increased compatibility, we should support the Postgres version too.

There have been other attempts at doing this in the past, but they had some issues.

See #13009 #13191 #17003 #34520

Epic CRDB-7644
@rafiss rafiss added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-sql-pgcompat Semantic compatibility with PostgreSQL labels Oct 4, 2019
@jordanlewis jordanlewis changed the title Support pgwire query cancellation sql: support pgwire query cancellation Apr 2, 2020
@knz knz mentioned this issue Apr 17, 2020
Closed
@rafiss rafiss mentioned this issue Aug 4, 2020
Closed
@awoods187
Copy link
Contributor

@solongordon if this is relatively easy i'd be interested in picking it up because its used in part of the solution for #52240 and seems generally useful behavior for our drivers to posses.

@rafiss
Copy link
Collaborator Author

rafiss commented Aug 5, 2020 •
edited
Loading

See #34520 for the latest attempt at implementing this. Eng has not really reached a consensus on how to do this. But I'm still hopeful!

Knowing that there's a product motivation for working on it could make it easier to drive consensus.

Also, I believe that we collect telemetry on how often a Cancel message is received. Not sure where to find it, but if anyone knows, I'd be curious to see the stats.

@awoods187
Copy link
Contributor

It's the top unimplemented feature: https://cockroachlabs.looker.com/looks/47
image

We should prioritize this particularly if we have some prior art to draw upon.

@jordanlewis jordanlewis mentioned this issue Aug 11, 2020
Closed
@ajwerner ajwerner mentioned this issue Jan 25, 2021
Open
@vy-ton vy-ton added the A-sql-pgwire pgwire protocol issues. label May 10, 2021
@vy-ton vy-ton mentioned this issue May 10, 2021
Closed
@mattcrdb mattcrdb mentioned this issue May 13, 2021
Closed
@rafiss rafiss added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label May 20, 2021
@rafiss rafiss mentioned this issue Jul 12, 2021
Merged
@rafiss rafiss self-assigned this Jul 12, 2021
@kibertoad kibertoad mentioned this issue Oct 11, 2021
Merged
@knz knz mentioned this issue Jan 20, 2022
Closed
@ajwerner
Copy link
Contributor

Idea here on how to translate with the proxy #67501 (comment)

@rafiss rafiss mentioned this issue Feb 2, 2022
Merged
craig bot pushed a commit that referenced this issue Feb 9, 2022
@rafiss @mneverov
@chengxiong-ruan @jbowens
Merge #75870 #76007 #76115 #76262
c7446c5 
75870: rfc: pgwire-compatible query cancellation r=otan,knz a=rafiss

refs #41335

Release note: None

76007: physicalplan: add support for multi-stage execution of regr_avgx, regr_avgy, regr_intercept, regr_r2, and regr_slope aggregate functions. r=yuzefovich a=mneverov

physicalplan: add support for multi-stage execution of regr_avgx, regr_avgy,
regr_intercept, regr_r2, and regr_slope aggregate functions.

See #58347.

Release note (performance improvement): regr_avgx, regr_avgy, regr_intercept,
regr_r2, and regr_slope aggregate functions are now evaluated more efficiently
in a distributed setting

76115: sql: use 16 as default bucket count for hash index r=chengxiong-ruan a=chengxiong-ruan

for some reason I forgot to modify it in my previous pr :(

Release note (sql change): 16 is used as the default bucket count
for hash sharded index.

76262: server: remove DeprecateBaseEncryptionRegistry r=jbowens a=jbowens

Remove the migration server DeprecateBaseEncryptionRegistry RPC. It was
used for the migration of the encryption-at-rest registry format in
21.2 and is no longer relevant.

Missed this in #74314.

Release note: None

Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Max Neverov <[email protected]>
Co-authored-by: Chengxiong Ruan <[email protected]>
Co-authored-by: Jackson Owens <[email protected]>
@craig craig bot closed this as completed in 3ac48db Feb 10, 2022
@vy-ton vy-ton mentioned this issue May 16, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rafiss rafiss

Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL A-sql-pgwire pgwire protocol issues. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Milestone
No milestone
4 participants
@rafiss @ajwerner @vy-ton @awoods187