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.

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

sql: implement server-side transaction timeouts #61102

Closed
lancel66 opened this issue Feb 25, 2021 · 10 comments · Fixed by #89033
Closed

sql: implement server-side transaction timeouts #61102

lancel66 opened this issue Feb 25, 2021 · 10 comments · Fixed by #89033
Assignees
Labels
A-sql-executor SQL txn logic 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

@lancel66
Copy link

lancel66 commented Feb 25, 2021

Is your feature request related to a problem? Please describe.
Even with a statement_timeout setting of 50 milliseconds, there is no cap to how long the transaction can run (and hold locks and increase contention, etc)

Describe the solution you'd like
A transaction_timeout setting which would handle multi-statement transactions which statement_timeout setting cannot address.

Describe alternatives you've considered
Low statement_timeout setting

gz#7680

gz#8786

gz#9005

Epic CRDB-17785
Jira issue: CRDB-3068

@lancel66 lancel66 added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Feb 25, 2021
@blathers-crl
Copy link

blathers-crl bot commented Feb 25, 2021

Hello, I am Blathers. I am here to help you get the issue triaged.

I was unable to automatically find someone to ping.

If we have not gotten back to your issue within a few business days, you can try the following:

  • Join our community slack channel and ask on #cockroachdb.
  • Try find someone from here if you know they worked closely on the area and CC them.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@blathers-crl blathers-crl bot added O-community Originated from the community X-blathers-untriaged blathers was unable to find an owner labels Feb 25, 2021
@lancel66
Copy link
Author

@ajwerner this is the issue we discussed in Slack.

@rafiss rafiss added A-sql-executor SQL txn logic and removed X-blathers-untriaged blathers was unable to find an owner labels Feb 25, 2021
@rafiss
Copy link
Collaborator

rafiss commented Feb 25, 2021

See #5924 for a previous discussion about this feature. I think we landed on only implementing idle_in_transaction_session_timeout, which times out if the application has a transaction open, but no statements running in it for long enough.

Using that combined with statement_timeout for long running statements seems to get most of the utility.

What is the remaining use case? It would have to be a transaction that has lots of statements in it that all execute under the statement_timeout, but there's so many of them that the transaction takes longer than the desired transaction time.

Not that we have to stick to what PostgreSQL does, but just noting that PG also has a lock_timeout setting which we don't.

cc @awoods187

@lancel66
Copy link
Author

From customer who initiated this issue in response to @rafiss

“ I've seen the idle_in_transaction_session_timeout setting. If I understand that correctly it kills connections that hold an idle transaction open past some timeout.

Two things that don't seem to align with my original request:

  • I don't want the connection itself to be closed, I want the transaction to be aborted
  • I want the transaction to be aborted after it has executed for X seconds, regardless of whether it was idle or not. “

@andreimatei andreimatei removed the O-community Originated from the community label Mar 5, 2021
@jlinder jlinder added the T-sql-queries SQL Queries Team label Jun 16, 2021
@lancel66
Copy link
Author

lancel66 commented Jun 24, 2021

Customer who initiated this request had an outage due to a badly written, long-running query transaction. In addition to having a server-side setting to completely kill a query, they would also like to be able to do this manually via an SQL command. This request may need to be put into a separate issue, thoughts @rafiss? cc @shermanCRL @erikgrinaker

@rafiss
Copy link
Collaborator

rafiss commented Jun 24, 2021

To clarify, is the ask for command that lets you kill a query, or a command that lets you kill a transaction?

There already is a command that lets you kill a query. See https://www.cockroachlabs.com/docs/stable/cancel-query.html

Also see https://www.cockroachlabs.com/docs/v21.1/manage-long-running-queries

If the ask is for a command to kill a transaction, then I would like to hear what additional functionality is needed apart from the CANCEL QUERY command linked above. Without knowing anything more about the requirements, my recommendation is to use CANCEL QUERY to kill a query, then any other queries that come later in the long-running transaction should fail immediately until a ROLLBACK is received.

@lancel66
Copy link
Author

@rafiss I meant transactions, thanks.

See (internal) support tickets #8786, #8791 and #8792 for information on the P0 outage, and the associated Jira issues CRDB-8207, CRDB-8208, CRDB-3808, CRDB-7475, CRDB-2878, CRDB-8220 and CRDB-5416 for more information on additional functionality needed.

@rafiss
Copy link
Collaborator

rafiss commented Jun 25, 2021

Thank you Lance!

I need a little more help here. The Jira issues you linked all seem to be intended to get at the problem from the other end -- that is, those issues are performance improvements so that transactions don't get stuck. They don't seem related to how we would implement a CANCEL TRANSACTION command. (At least not from my non-KV understanding; @nvanbenschoten or @andreimatei could you correct me if I'm wrong?)

I would like to understand what we expect a CANCEL TRANSACTION command is expected to give us that the CANCEL QUERY command does not. It would help to have a user story for the CANCEL TRANSACTION command.


Also, the clarifying info you gave on the earlier request for server-side transaction timeouts helped a lot! I do think a server-side transaction timeout setting sounds like a good idea.

@ajwerner
Copy link
Contributor

I'll tack on that CANCEL SESSION exists and transactions can be joined to sessions via crdb_internal.cluster_transactions and crdb_internal.cluster_sessions show transaction IDs.

@vy-ton
Copy link
Contributor

vy-ton commented Jun 28, 2021

To summarize the responses above that will help us consolidate use cases (feel free to edit directly):

For multi-statement transactions:

I want a timeout setting that accounts for total transaction time not just individual statement time
Customer who initiated this request had an outage due to a badly written, long-running transaction.

@lancel66 Does the user have an obvious threshold for badly written vs legitimate long-running transactions? Was the badly written txn ever idle?

Is it OK for the transaction timeout to be applied at a statement boundary (in between two statements in the transaction)? Or does the timeout need to be more precise and cancel an actively running query?

For single statement transactions:

  • Is the user satisfied with the existing levers that we offer, e.g. statement_timeout and CANCEL QUERY?

@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Jul 12, 2021
@exalate-issue-sync exalate-issue-sync bot removed the T-sql-queries SQL Queries Team label Jul 14, 2021
@jordanlewis jordanlewis changed the title Implement server-side transaction timeouts sql: implement server-side transaction timeouts Dec 14, 2021
@craig craig bot closed this as completed in ea43f89 Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-executor SQL txn logic 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
Development

Successfully merging a pull request may close this issue.

7 participants