gx0r / connect-session-knex

A knex.js session store for express-session, supporting PostgreSQL, MySQL, MariaDB, Oracle and SQLite.
https://www.npmjs.com/package/connect-session-knex
ISC License
90 stars 67 forks source link

Conflict between autocommit and connection pooling in mysql #4

Open morungos opened 9 years ago

morungos commented 9 years ago

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.

morungos commented 9 years ago

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.

gx0r commented 9 years ago

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.

gx0r commented 9 years ago

Is this still an issue with the latest version? The connection pool was changed to a new implementation in Knex 0.8.

morungos commented 9 years ago

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.