caisin/sea-orm
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Prevent returning connections to pool with a positive transaction depth (#1283)

Browse Source 
Mark transaction as closed *only* after commit/rollback succeeds.

Previously, `open` on the transaction would be set to `false` prior to attempting
to commit or rollback the transaction. When the operation failed, for example, due
to a serialization failure with a serializable isolation level, this would leave
the transaction in an inconsistent state, where it thought it was closed but really
it was still open. The connection would then be returned to the connection pool with
a transaction depth of 1, causing a savepoint to be erroneously created the next time
a transaction was created for the connection.

By waiting to set `open` to `false` until the commit/rollback succeeds, a failure
to do either will result in us correctly rolling back the transaction when dropping
it, ensuring that the connection is returned to the pool with a transaction depth
of 0. Note that this is consistent with how `sqlx` handles transactions.

We attempted to write a test, but had a very difficult time forcing postgres to fail
to commit a transaction. We found that it would block our requests instead when creating
conflicting updates, and we couldn't find any information about when it blocks vs when
transaction commits fail.

Co-Authored-By: Nathan Sobo <nathan@zed.dev>

Co-authored-by: Nathan Sobo <nathan@zed.dev>
This commit is contained in:
Antonio Scandurra 2022-12-16 14:11:51 +01:00 committed by GitHub
parent 69eb4b8ea8
commit df2dcdabac
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 2 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/database/transaction.rs
View File

@ -176,7 +176,6 @@ impl DatabaseTransaction {
#[instrument(level = "trace")] #[instrument(level = "trace")]
#[allow(unreachable_code)] #[allow(unreachable_code)]
pub async fn commit(mut self) -> Result<(), DbErr> { pub async fn commit(mut self) -> Result<(), DbErr> {
self.open = false;
match *self.conn.lock().await { match *self.conn.lock().await {
#[cfg(feature = "sqlx-mysql")] #[cfg(feature = "sqlx-mysql")]
InnerConnection::MySql(ref mut c) => { InnerConnection::MySql(ref mut c) => {
@ -201,6 +200,7 @@ impl DatabaseTransaction {
c.commit(); c.commit();
} }
} }
self.open = false;
Ok(()) Ok(())
} }
@ -208,7 +208,6 @@ impl DatabaseTransaction {
#[instrument(level = "trace")] #[instrument(level = "trace")]
#[allow(unreachable_code)] #[allow(unreachable_code)]
pub async fn rollback(mut self) -> Result<(), DbErr> { pub async fn rollback(mut self) -> Result<(), DbErr> {
self.open = false;
match *self.conn.lock().await { match *self.conn.lock().await {
#[cfg(feature = "sqlx-mysql")] #[cfg(feature = "sqlx-mysql")]
InnerConnection::MySql(ref mut c) => { InnerConnection::MySql(ref mut c) => {
@ -233,6 +232,7 @@ impl DatabaseTransaction {
c.rollback(); c.rollback();
} }
} }
self.open = false;
Ok(()) Ok(())
} }