mysqljs / mysql

A pure node.js JavaScript Client implementing the MySQL protocol.
MIT License
18.27k stars 2.52k forks source link

Auto-rollback of released connections without transaction commit #1116

Open bionicman21 opened 9 years ago

bionicman21 commented 9 years ago

When using a connection pool, releasing a connection should have a similar effect to ending a connection (aside from simply keeping the connection open). For starters, it would be extremely convenient to know if a connection has started a transaction (i.e. a boolean flag for isInTransaction) which would provide the following benefits:

  1. Passing an existing connection to child functions allows the child functions to operate independently and decide when/whether transactions need to be started (ability to shorten transaction lifecycles to reduce potential deadlocks) vs. managing this in parent code.
  2. If connections are released without making a call to "Connection.commit", they should really auto-rollback the transaction (just like a call to connection.end would do). This ensures that when the connection is re-used, the true effect is that the connection operates like a new connection (rather than having strange side-effects where existing transactions might cause deadlock).
dougwilson commented 9 years ago

This suggestion seems fine to me. PRs implementing this are welcome, but they of course need to work even if a user starts a transaction though a regular query call, so we can make this kind of guarantee.

AdriVanHoudt commented 9 years ago

would you check every query or just set the flag on beginTransaction?

dougwilson commented 9 years ago

Whatever the implementation is, it cannot be confined to requiring the user to only start transactions through beginTransaction

bionicman21 commented 9 years ago

The simpler approach would be to set a flag on begin transaction, but there is an obvious limitation due to the open-ended nature of querying with this lib. In reality, we would check each query and set the flag accordingly, but it may also make sense to only guarantee that flag through begin transaction.

@dougwilson, I know that you mentioned it not being confined to the beginTransaction function, but we may also want to consider performance issues with global query checks.

dougwilson commented 9 years ago

Regardless, it will not be accepted into this lib if it is only confined to begintransaction. This is a low level library, equivalent to a C library. We are not a ORM or high level management. There are libraries that already do that and you cab always write your own that wraps this or mysql2.

bionicman21 commented 9 years ago

Fair enough. It may be worth some thought on how to perform such checks efficiently. Either way, I completely understand the need.

dougwilson commented 9 years ago

I understand the need as well, which is why it is open. Very likely you can do the tracking using the protocol level flags, not by reading the SQL.

Forty-Tw0 commented 5 years ago

This might be of use to check the current session's transaction state, though specific to InnoDB. SELECT * FROM information_schema.innodb_trx WHERE trx_mysql_thread_id=CONNECTION_ID();