Transactions: Is the current implementation bulletproof?

[SebastianApel]

Hi Jeremy, I was thinking about the transaction implementation. My test scenario now works fine (with the workaround, see issue #34) in the "single client test case" scenario.

But I am not sure what would happen under load with a lot of parallel transactions. What happens when a parallel execution also uses the connection - how can you be 100% sure that the "other" execution is not issuing SQL statements over the connection DURING the transaction (i.e. after the START TRANSACTION, but before the COMMIT / ROLLBACK)?

For the COMMIT case, that would not be critical, but for the ROLLBACK case, it would result in data loss (because it would appear to the "other" thread that the statement executed just fine - but later it gets rolled back, so the data will not be in the database... Ouch).

Have you thought about that case? Are you sure your current approach is bulletproof?

An idea for an improved approach could be:

• get a NEW, PRIVATE connection (i.e. one that is not shared)
• use only that for the transaction
• quit the transaction after the COMMIT / ROLLBACK

Happy to discuss...

[jeremydaly]

Hi @SebastianApel,

Thanks for reporting this. If you are using AWS Lambda, there should only be one connection per invocation (i.e. concurrent user), so you should never get the race condition you're referring to. I'm not sure how this would work across other providers exactly, so input from others would be helpful.

Let me look at #34 in more detail.

• Jeremy

[SebastianApel]

Hi @jeremydaly, thanks for your reply. I am not familiar enough with Lambda to jugde that case.

I am currently using the library in an non-Lambda usecase (because it promissifies mysqljs nicely, it handles connection timeouts "well" out of the box, and I liked the transaction idea). My current setup is based on express, so my rationale in that case is valid - right? (I'm pretty new to node, so I might be wrong here - feedback is highly welcome).

If my rationale holds, then at least a warning in the README would be very helpful for the users.

Sebastian

[bluedusk]

I think this lib is for lambda based mysql connection management, if you don't use lambda, I don't see necessity to use this.

[douglasgsouza]

I have also been thinking about this issue, especially regarding the reuse of the connection.

The way that the transaction () method was implemented seems to be safe, as it would execute all queries at the same time, calling the rollback in case of error. Sufficient for most use cases.

But I will post here my reflection on this issue:

It can be a problem when trying to use it procedurally. In the example below I have other complex non-sql operations that depend on executing queries in stages, to check for unique constraints.

Example:

const transaction = mysql.transaction ();
try {
  transaction.query ("INSERT INTO table (name) VALUES (?);
  --- do a non-sql operation only if the above query is successful.
  transaction.query ("INSERT INTO table (name) VALUES (?);
  await transaction.commit (); // transaction is only initiated here and querys are executed only here.
} catch (e) {
  await transaction.rollback ();
}

In this case, queries are only executed on commit and if the first query causes an error, the non-sql operation could not be performed.

So I thought of a second approach, starting the transaction manually with individual queries (START TRANSACTION). But then I don't know if I would have a problem because I was reusing connections with simultaneous lambda executions.

My suggestion is:

• create a separate beginTransaction () method that returns a transaction instance that uses a different connection exclusively for each transaction.
• maintain an array with these transaction connections, which can be released through the manager.

I would even like to help with the implementation, but I don't even know where to start.

Thanks Jeremy, this package and your blog articles are fantastic.