-
Notifications
You must be signed in to change notification settings - Fork 66
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
Conflict between autocommit and connection pooling in mysql #4
Comments
There's a second problem, actually, which is related. Because the transactions aren't committed, they aren't visible on other connections. This means that when there are multiple connections and autocommit is off, sessions aren't managed at all correctly. Authentication on one connection is not visible to a second, for example, breaking passport.js entirely. |
Thanks for the report! Is this only an issue with MySQL? I am not sure I will get to it since MySQL is low priority for me, but PR are welcome. |
Is this still an issue with the latest version? The connection pool was changed to a new implementation in Knex 0.8. |
I'd be happier if I could test more fully -- just started using knex 8, and I did hit transaction issues in some of my mocking systems, but on the whole it seemed to be an improvement. My impression is that yes: the transaction/connection association is more solid in knex 8, but I haven't tested it systematically. |
When autocommit is turned off (i.e., in transaction mode), connect-session-knex no longer works in MySQL. This is because session update statements might be executed on different connections, and a table lock is acquired for each as soon as the insert statement is executed. If a second connection attempts to do the same, it hangs waiting for the lock. There are no commit statements so the lock isn't released unless/until knex times out the connection.
This could be supported by wrapping the update command, even the MySQL optimised one, in a transaction statement -- or at least allowing that wrapping as an option.
The text was updated successfully, but these errors were encountered: