Confusion about transaction code:

[smhk]

My transaction code:

try {
    const cn = await pool.getConnection()
    await cn.beginTransaction()

    await cn.execute(' ...
    await cn.execute(' ...

    await cn.commit()
}
catch {
    await cn.rollback()
}

But am I wrong? Should it be:

const cn = await pool.getConnection()
try {
    await cn.beginTransaction()

    await cn.execute(' ...
    await cn.execute(' ...

    await cn.commit()
}
catch {
    await cn.rollback()
}

But wait! Should it be?

const cn = await pool.getConnection()
await cn.beginTransaction()
try {
    await cn.execute(' ...
    await cn.execute(' ...

    await cn.commit()
}
catch {
    await cn.rollback()
}

I'm afraid I don't know the answer. Thanks in advance anyone who can advise. (It may help others also?)

[sidorares]

beginTransaction / commit / rollback are just simple helpers to perform START TRANSACTION / COMMIT / ROLLBACK queries

To me 3rd example is the most correct but I'm not an SQL / mysql expert

Possible reasons why pool.getConnection() can fail: network error in the process of getting a new connection, too many connections limit reached (server side), client side limit on number of open connections

Not sure when START TRANSACTION can fail ( reasons other than connection is killed because of network error or other client KILL thread_id ) but I guess rollback only makes sense after successful START TRANSACTION

[sidorares]

there was a discussion somewhere here about possible better helper api that would handle this pattern, for example something similar to https://github.com/porsager/postgres#transactions ( I've started some initial work here )

[maoweifan]

I think the first one is wrong, because cn is in try, catch can't get the reference

And in the experience of some other languages (such as java), the opening of the transaction will also be put outside the try, because the failure of the transaction to open does not actually need to be rolled back, only if there is an error during the execution process, it will be rolled back. capture

[smhk]

thanks for the fantastic info @sidorares . I'm sure this will be helpful to others also. Additionally it's one of those cases where on the internet, there is some bad/incorrect example code, which gets repeatedly copied. Thanks again, mysql2 is totally incredible

(Just FYI there was a delay with ghub notifying these comments, thanks again.)

[trasherdk]

Isn't the point of transactions, to isolate a group of query/execute commands?

If, for some reason, beginTransaction() fail, the following group of commands should never run.

Some back-off/retry on beginTransaction() could be useful :)

[sidorares]

@trasherdk in the example above if beginTransaction fails the control goes to catch block

[trasherdk]

@sidorares If by example above, you mean:

const cn = await pool.getConnection()
try {
    await cn.beginTransaction()

    await cn.execute(' ...
    await cn.execute(' ...

    await cn.commit()
}
catch {
    await cn.rollback()
}

Then I'd expect you to be right.
But with beginTransaction() outside the try {} catch {} block, how would control go to catch {}?

[sidorares]

Yes, currently you are expected to handle this manually. There is definitely a room for improvement, feel free to suggest your api

[trasherdk]

Well, the snippet I quoted, is how I would expect the flow to go.

I'd probable throw in a few SELECT ... LOCK IN SHARE MODE and SELECT ... FOR UPDATE before any execute() :)

[sidorares]

we could have a helper like this as a replacement for beginTransaction / commit / rollback:

const result = await connection.transaction(async connection => {
   connection.execute(/*...*/);
   return connection.query(/*...*/);
}) 

// also pool - a shortcut for pool.getConnection + connection.transaction
await pool.transaction(async connection => {
   connection.execute(/*...*/);
   connection.query(/*...*/);
}) 

wdyt @trasherdk ?

[trasherdk]

If pool.transaction( conn => {}) includes some kind of retry, then cool.
Otherwise, it's better handled by whatever flow it's part of.

// also pool - a shortcut for pool.getConnection + connection.transaction
await pool.transaction(async connection => {
   await connection.execute(/*...*/);
   const [ cols, rows ] = await connection.query(/*...*/);
})

But how is commit/rollback handled in this construction?

// also pool - a shortcut for pool.getConnection + connection.transaction
await pool.transaction(async connection => {
   await connection.execute(/*...*/);
   const [ cols, rows ] = await connection.query(/*...*/);

  connection.commit()
}).catch(error) {
  connection.rollback()
}

It might be better to bake the retry/back-off into the existing beginTransaction()

conn.beginTransaction(retry=5, retryTime=500, backoff=100)
or
conn.beginTransaction({ retry: 5, retryTime: 500, backoff: 100 })