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 idle_in_transaction_session_timeout #5924

Closed
archiecobbs opened this issue Apr 7, 2016 · 30 comments · Fixed by #52938
Closed

sql: implement idle_in_transaction_session_timeout #5924

archiecobbs opened this issue Apr 7, 2016 · 30 comments · Fixed by #52938
Assignees
Labels
A-sql-executor SQL txn logic C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) O-community Originated from the community

Comments

@archiecobbs
Copy link

For any application performing transactions, it's important that the application be able to configure an explicit limit on how long a transaction thread will block trying to perform the transaction.

Enhancement Request: Add support for an SQL variable like this:

SET transaction_timeout = 2500

This will then cause an immediate retry exception (and transaction rollback) if any statement (or the overall transaction?) takes longer than 2500 milliseconds to complete.

It is important that the corresponding SQLError has either existing the "retry" error code ("CR000") or a new, well-documented timeout error code; the application will likely end-up treating these in the same way as retry errors.

Note that this is only a partial solution to the overall problem of controlling timeouts: there also needs to be a timeout configured on the client's PostgreSQL driver itself (for example, there could be a network hang between the local client and the CockroachDB node it's talking to via JDBC). This part of the problem is not within CockroachDB's realm of control, however, it would be nice and helpful for SQL users if CockroachDB also provided a clearly documented way to configure the local PostgreSQL driver timeout. E.g., there are some timeout parameters shown here.

Thanks.

Discussion thread: https://groups.google.com/forum/#!topic/cockroach-db/FpBemFJM4w8

@petermattis petermattis changed the title SQL: allow configuration of transaction timeout sql: allow configuration of transaction timeout Apr 7, 2016
@petermattis
Copy link
Collaborator

@archiecobbs Looks like postgres only supports statement_timeout. Would that be sufficient, or would you want separate control for an overall transaction timeout?

@archiecobbs
Copy link
Author

@petermattis Having statement_timeout would certainly be a big improvement, assuming COMMIT TRANSACTION counts as a statement.

Right now the amount of time you could get stuck is (theoretically) unbounded, and this would fix that.

@petermattis
Copy link
Collaborator

Yes, COMMIT TRANSACTION is a statement. Internally, the corresponding EndTransaction request already contains a deadline so we can ensure that we positively know a transaction was aborted when the deadline was exceeded.

@archiecobbs
Copy link
Author

FWIW, a unit test I'm running in Java against Cockroach hangs forever. This is the situation that motivated creating this issue. Why this test is hanging whereas it doesn't hang with other databases (e.g., MySQL) is a related question.

Here's the stack trace of the thread that's stuck forever waiting for some response:

"pool-1-thread-4" prio=5 tid=0x00007fa033acf000 nid=0x5d03 runnable [0x000000011c011000]
   java.lang.Thread.State: RUNNABLE
    at java.net.SocketInputStream.socketRead0(Native Method)
    at java.net.SocketInputStream.read(SocketInputStream.java:152)
    at java.net.SocketInputStream.read(SocketInputStream.java:122)
    at org.postgresql.core.VisibleBufferedInputStream.readMore(VisibleBufferedInputStream.java:143)
    at org.postgresql.core.VisibleBufferedInputStream.ensureBytes(VisibleBufferedInputStream.java:112)
    at org.postgresql.core.VisibleBufferedInputStream.read(VisibleBufferedInputStream.java:70)
    at org.postgresql.core.PGStream.ReceiveChar(PGStream.java:283)
    at org.postgresql.core.v3.QueryExecutorImpl.processResults(QueryExecutorImpl.java:1799)
    at org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:200)
    - locked <0x00000007ad634380> (a org.postgresql.core.v3.QueryExecutorImpl)
    at org.postgresql.jdbc.PgStatement.execute(PgStatement.java:424)
    at org.postgresql.jdbc.PgPreparedStatement.executeWithFlags(PgPreparedStatement.java:161)
    at org.postgresql.jdbc.PgPreparedStatement.executeUpdate(PgPreparedStatement.java:133)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:606)
    at org.postgresql.ds.PGPooledConnection$StatementHandler.invoke(PGPooledConnection.java:426)
    at com.sun.proxy.$Proxy13.executeUpdate(Unknown Source)
    at org.jsimpledb.kv.sql.SQLKVTransaction.update(SQLKVTransaction.java:296)
    at org.jsimpledb.kv.sql.SQLKVTransaction.put(SQLKVTransaction.java:131)
    - locked <0x00000007ad63f1e0> (a org.jsimpledb.kv.cockroach.CockroachKVTransaction)
    at org.jsimpledb.kv.test.KVDatabaseTest$Writer.run(KVDatabaseTest.java:877)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
    at java.util.concurrent.FutureTask.run(FutureTask.java:262)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
    at java.lang.Thread.run(Thread.java:744)

Sorry I don't have more debug info (I'm go-ignorant). Let me know if you want instructions on how to reproduce.

@bdarnell
Copy link
Contributor

Which version of cockroachdb are you running? We had a deadlock in last week's beta that will be fixed in the new version being released today. If that doesn't fix it, please file a separate issue with more information about what exactly you're trying to do, and any relevant bits of the server logs (by default in cockroach-data/logs/)

@archiecobbs
Copy link
Author

Version I have is:

$ cockroach version
Build Tag:   beta-20160505
Build Time:  2016/05/12 00:16:46
Platform:    darwin amd64
Go Version:  go1.6
C Compiler:  4.2.1 Compatible Apple LLVM 7.0.2 (clang-700.1.81)

I'll upgrade tomorrow, try again, and then update this issue.

Thanks.

@archiecobbs
Copy link
Author

archiecobbs commented May 13, 2016

I'm getting the same behavior with this version:

Build Tag:   beta-20160512
Build Time:  2016/05/13 01:21:09
Platform:    darwin amd64
Go Version:  go1.6
C Compiler:  4.2.1 Compatible Apple LLVM 7.0.2 (clang-700.1.81)

Two different tests are hanging. The first one hangs sometimes, the second one hangs always.

Test setup:

CREATE TABLE IF NOT EXISTS "KV" (
  "kv_key" BYTES PRIMARY KEY NOT NULL,
  "kv_value" BYTES NOT NULL
);
DELETE FROM "KV";

Test 1: testConflictingTransactions():

Open two transactions T1 and T2

T1: SELECT "kv_value" FROM "KV" WHERE "kv_key" = ? where ? = 0x10
T2: SELECT "kv_value" FROM "KV" WHERE "kv_key" = ? where ? = 0x10
T1: UPSERT INTO "KV" ("kv_key", "kv_value") VALUES (?, ?) where ?1 = 0x10, ?2 = 0x01
T2: UPSERT INTO "KV" ("kv_key", "kv_value") VALUES (?, ?) where ?1 = 0x10, ?2 = 0x02

At this point T2 hangs - sometimes:

"pool-1-thread-4" prio=5 tid=0x00007fd25a994000 nid=0x5d03 runnable [0x0000000116f98000]
   java.lang.Thread.State: RUNNABLE
    at java.net.SocketInputStream.socketRead0(Native Method)
    at java.net.SocketInputStream.read(SocketInputStream.java:152)
    at java.net.SocketInputStream.read(SocketInputStream.java:122)
    at org.postgresql.core.VisibleBufferedInputStream.readMore(VisibleBufferedInputStream.java:143)
    at org.postgresql.core.VisibleBufferedInputStream.ensureBytes(VisibleBufferedInputStream.java:112)
    at org.postgresql.core.VisibleBufferedInputStream.read(VisibleBufferedInputStream.java:70)
    at org.postgresql.core.PGStream.ReceiveChar(PGStream.java:283)
    at org.postgresql.core.v3.QueryExecutorImpl.processResults(QueryExecutorImpl.java:1799)
    at org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:200)
    - locked <0x00000007ad6381c0> (a org.postgresql.core.v3.QueryExecutorImpl)
    at org.postgresql.jdbc.PgStatement.execute(PgStatement.java:424)
    at org.postgresql.jdbc.PgPreparedStatement.executeWithFlags(PgPreparedStatement.java:161)
    at org.postgresql.jdbc.PgPreparedStatement.executeUpdate(PgPreparedStatement.java:133)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:606)
    at org.postgresql.ds.PGPooledConnection$StatementHandler.invoke(PGPooledConnection.java:426)
    at com.sun.proxy.$Proxy13.executeUpdate(Unknown Source)
    at org.jsimpledb.kv.sql.SQLKVTransaction.update(SQLKVTransaction.java:296)
    at org.jsimpledb.kv.sql.SQLKVTransaction.put(SQLKVTransaction.java:131)
    - locked <0x00000007ad643020> (a org.jsimpledb.kv.cockroach.CockroachKVTransaction)
    at org.jsimpledb.kv.test.KVDatabaseTest$Writer.run(KVDatabaseTest.java:890)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
    at java.util.concurrent.FutureTask.run(FutureTask.java:262)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
    at java.lang.Thread.run(Thread.java:744)

Test 2: testNonconflictingTransactions():

Open 10 transactions T1, T2, ..., T10

T1: SELECT "kv_key", "kv_value" FROM "KV" WHERE "kv_key" >= ? ORDER BY "kv_key" ASC LIMIT 1 where ? = 0x00
T1: UPSERT INTO "KV" ("kv_key", "kv_value") VALUES (?, ?) where ?1 = 0x80, ?2 = 0x02
T2: SELECT "kv_key", "kv_value" FROM "KV" WHERE "kv_key" >= ? ORDER BY "kv_key" ASC LIMIT 1 where ? = 0x01

Test hangs at this point:

"pool-1-thread-8" prio=5 tid=0x00007fd1fda8c800 nid=0x6503 runnable [0x000000011e378000]
   java.lang.Thread.State: RUNNABLE
    at java.net.SocketInputStream.socketRead0(Native Method)
    at java.net.SocketInputStream.read(SocketInputStream.java:152)
    at java.net.SocketInputStream.read(SocketInputStream.java:122)
    at org.postgresql.core.VisibleBufferedInputStream.readMore(VisibleBufferedInputStream.java:143)
    at org.postgresql.core.VisibleBufferedInputStream.ensureBytes(VisibleBufferedInputStream.java:112)
    at org.postgresql.core.VisibleBufferedInputStream.read(VisibleBufferedInputStream.java:70)
    at org.postgresql.core.PGStream.ReceiveChar(PGStream.java:283)
    at org.postgresql.core.v3.QueryExecutorImpl.processResults(QueryExecutorImpl.java:1799)
    at org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:200)
    - locked <0x00000007ad638468> (a org.postgresql.core.v3.QueryExecutorImpl)
    at org.postgresql.jdbc.PgStatement.execute(PgStatement.java:424)
    at org.postgresql.jdbc.PgPreparedStatement.executeWithFlags(PgPreparedStatement.java:161)
    at org.postgresql.jdbc.PgPreparedStatement.executeQuery(PgPreparedStatement.java:114)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:606)
    at org.postgresql.ds.PGPooledConnection$StatementHandler.invoke(PGPooledConnection.java:426)
    at com.sun.proxy.$Proxy13.executeQuery(Unknown Source)
    at org.jsimpledb.kv.sql.SQLKVTransaction.query(SQLKVTransaction.java:276)
    at org.jsimpledb.kv.sql.SQLKVTransaction.queryKVPair(SQLKVTransaction.java:250)
    at org.jsimpledb.kv.sql.SQLKVTransaction.getAtLeast(SQLKVTransaction.java:99)
    - locked <0x00000007adee7da8> (a org.jsimpledb.kv.cockroach.CockroachKVTransaction)
    at org.jsimpledb.kv.test.KVDatabaseTest$Reader.call(KVDatabaseTest.java:861)
    at org.jsimpledb.kv.test.KVDatabaseTest$Reader.call(KVDatabaseTest.java:842)
    at java.util.concurrent.FutureTask.run(FutureTask.java:262)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
    at java.lang.Thread.run(Thread.java:744)

To run these tests yourself:

  1. git clone [email protected]:archiecobbs/jsimpledb
  2. cd jsimpledb
  3. mvn install
  4. Edit pom.xml, and uncomment the <cockroachURL> line (edit to suit)
  5. cd jsimpledb-kv-cockroach
  6. cockroach sql -e 'create database if not exists jsimpledb'
  7. mvn test

@archiecobbs
Copy link
Author

FYI, test produces same hanging behavior with beta-20160519.

@archiecobbs
Copy link
Author

Created separate issue #7556 for the deadlock problem.

@RaduBerinde
Copy link
Member

Some more related comments in #7556 (comment)

@spencerkimball
Copy link
Member

@petermattis should this be 1.0? Could you assign if so, remove the 1.0 milestone otherwise.

@spencerkimball spencerkimball added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Mar 30, 2017
@petermattis
Copy link
Collaborator

Given our plumbing of context everywhere, this seems relatively straightforward to do on the surface.

@cuongdo cuongdo assigned justinj and unassigned cuongdo Apr 18, 2017
@vivekmenezes vivekmenezes modified the milestones: 1.1, 1.0 Apr 18, 2017
@dianasaur323 dianasaur323 added O-community Originated from the community and removed community-questions labels Apr 23, 2017
@cuongdo
Copy link
Contributor

cuongdo commented May 24, 2017

Because this feature is not already on our 1.1 roadmap, I'm moving this to "Later."

@cuongdo cuongdo modified the milestones: Later, 1.1 May 24, 2017
@petermattis
Copy link
Collaborator

This looks like a good starter project.

abhimadan pushed a commit to abhimadan/cockroach that referenced this issue Mar 7, 2018
This new session variable has identical semantics to the
`statement_timeout` variable in Postgres, but also allows you to pass an
interval as an argument. Passing an integer is also allowed in order to
be compatible with Postgres.

Fixes cockroachdb#20789.
Touches cockroachdb#5924.

Release note (sql change): Add `statement_timeout` session variable.
@abhimadan abhimadan removed their assignment Apr 11, 2018
@knz knz added the A-sql-executor SQL txn logic label May 15, 2018
@nstewart
Copy link
Contributor

What is the incremental work left on this ticket after implementing statement_timeout, documented here: https://www.cockroachlabs.com/docs/dev/set-vars.html#supported-variables? Is this done?

@vivekmenezes
Copy link
Contributor

@andreimatei to comment and close

@bdarnell
Copy link
Contributor

Statement timeouts and transaction timeouts are different; adding statement_timeout doesn't address the need for a transaction timeout. This transaction is primarily about transaction timeouts (perhaps using the postgres variable idle_in_transaction_session_timeout)

@knz knz modified the milestones: 2.1, 2.2 Aug 30, 2018
@petermattis petermattis removed this from the 2.2 milestone Oct 5, 2018
@jordanlewis jordanlewis changed the title sql: allow configuration of transaction timeout sql: implement idle_in_transaction_session_timeout Apr 2, 2020
@awoods187
Copy link
Contributor

awoods187 commented Jul 31, 2020

We now have a statement_timeout and a idle_in_session_timeout (introduced in 20.2) we should round out the set with this idle_in_transaction_session_timeout.

Confusingly, this variable is only implemented now as a no-op:

root@127.0.0.1:59509/movr> SET idle_in_transaction_session_timeout = 10;
ERROR: invalid value for parameter "idle_in_transaction_session_timeout": "10"
SQLSTATE: 22023
DETAIL: this parameter is currently recognized only for compatibility and has no effect in CockroachDB.
HINT: Available values: 0

Despite showing up in SHOW ALL:

                    variable                    |                                                          value
------------------------------------------------+--------------------------------------------------------------------------------------------------------------------------
  idle_in_session_timeout                       | 0
  idle_in_transaction_session_timeout           | 0

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) O-community Originated from the community
Projects
None yet
Development

Successfully merging a pull request may close this issue.