Expose transaction_depth through get_transaction_depth() method

[mpyw]

Issue

• Resolves Feature Request: Expose transaction_depth with a Read-Only Getter Method #3426

Description

This PR introduces the get_transaction_depth() method for exposing the current transaction depth in various database connections.

• MySqlConnection and PgConnection now implement the TransactionDepth::get_transaction_depth() method to synchronously retrieve the transaction depth.
• SqliteConnection implements the AsyncTransactionDepth::get_transaction_depth() method to retrieve the transaction depth asynchronously by acquiring a lock (as the existing implementation requires a lock to access the transaction level).
• AnyConnection uses the AsyncTransactionDepth::get_transaction_depth() method to handle SqliteConnection, and in the dyn AnyConnectionBackend implementation, it dispatches to the respective connection's implementation.

Concerns

• Since the method get_transaction_depth() is used for both traits, there is a potential for import collisions between the two traits. However, it is unlikely that both traits would be imported simultaneously, so this should not pose a significant issue.
• Regarding the SqliteConnection implementation, I'm unsure if this approach is optimal. Specifically, is it acceptable to acquire a lock just to retrieve the transaction nesting level? Additionally, my understanding is that the lock is automatically released when it goes out of scope—is this correct?

Additional Context

I am relatively new to using Rust professionally, having only recently started working with it. If there are any issues with my approach or if I am violating any best practices, please feel free to point them out.

[abonander]

I'm not a fan of the trait split. The SQLite driver can be refactored a bit to track the transaction depth in the connection struct itself (or the ConnectionWorker struct), we've just got to be more careful of keeping the value in-sync.

---

Instead of a bespoke trait, since you're only implementing it directly for the connection types anyway, the method can just go on the Connection trait.

It can be a provided method that forwards to a method on TransactionManager. It's not a breaking change to add a required method to the latter because it's #[doc(hidden)] and thus exempt from our semver guarantees.

---

Since most users would likely just do conn.get_transaction_depth() != 0 to check if the connection is in a transaction, I would also add a provided method like

#[inline]
fn is_in_transaction(&self) -> bool {
    self.get_transaction_depth() != 0
}

---

No emoji in the commit messages, please. It's not necessary, and who knows what it might break.

[mpyw]

I now understand what you mean by provided method. It means "default implementation" in other languages.

[mpyw]

@abonander Strictly speaking, it seems more correct to use RwLock rather than AtomicUsize and to keep it locked during operations. However, since RwLock requires handling errors, I chose this approach for now. If you prefer an implementation using RwLock, please let me know your thoughts.

---

[Edit]

I am considering adopting RwLock from parking_lot crate.

[mpyw]

@abonander If we were to replace the implementation with parking_lot::RwLock, it would look like this:

diff --git a/sqlx-sqlite/src/connection/worker.rs b/sqlx-sqlite/src/connection/worker.rs
index 6dda814a..d36b1df2 100644
--- a/sqlx-sqlite/src/connection/worker.rs
+++ b/sqlx-sqlite/src/connection/worker.rs
@@ -34,14 +34,14 @@ pub(crate) struct ConnectionWorker {
 }
 
 pub(crate) struct WorkerSharedState {
-    transaction_depth: AtomicUsize,
+    transaction_depth: parking_lot::RwLock<usize>,
     cached_statements_size: AtomicUsize,
     pub(crate) conn: Mutex<ConnectionState>,
 }
 
 impl WorkerSharedState {
     pub(crate) fn get_transaction_depth(&self) -> usize {
-        self.transaction_depth.load(Ordering::Acquire)
+        *self.transaction_depth.read()
     }
 
     pub(crate) fn get_cached_statements_size(&self) -> usize {
@@ -104,7 +104,7 @@ impl ConnectionWorker {
                 };
 
                 let shared = Arc::new(WorkerSharedState {
-                    transaction_depth: AtomicUsize::new(0),
+                    transaction_depth: parking_lot::RwLock::new(0),
                     cached_statements_size: AtomicUsize::new(0),
                     // note: must be fair because in `Command::UnlockDb` we unlock the mutex
                     // and then immediately try to relock it; an unfair mutex would immediately
@@ -193,12 +193,12 @@ impl ConnectionWorker {
                             update_cached_statements_size(&conn, &shared.cached_statements_size);
                         }
                         Command::Begin { tx } => {
-                            let depth = shared.transaction_depth.load(Ordering::Acquire);
+                            let mut depth = shared.transaction_depth.write();
                             let res =
                                 conn.handle
-                                    .exec(begin_ansi_transaction_sql(depth))
+                                    .exec(begin_ansi_transaction_sql(*depth))
                                     .map(|_| {
-                                        shared.transaction_depth.fetch_add(1, Ordering::Release);
+                                        *depth += 1;
                                     });
                             let res_ok = res.is_ok();
 
@@ -209,9 +209,9 @@ impl ConnectionWorker {
                                 // immediately otherwise it would remain started forever.
                                 if let Err(error) = conn
                                     .handle
-                                    .exec(rollback_ansi_transaction_sql(depth + 1))
+                                    .exec(rollback_ansi_transaction_sql(*depth + 1))
                                     .map(|_| {
-                                        shared.transaction_depth.fetch_sub(1, Ordering::Release);
+                                        *depth -= 1;
                                     })
                                 {
                                     // The rollback failed. To prevent leaving the connection
@@ -223,13 +223,13 @@ impl ConnectionWorker {
                             }
                         }
                         Command::Commit { tx } => {
-                            let depth = shared.transaction_depth.load(Ordering::Acquire);
+                            let mut depth = shared.transaction_depth.write();
 
-                            let res = if depth > 0 {
+                            let res = if *depth > 0 {
                                 conn.handle
-                                    .exec(commit_ansi_transaction_sql(depth))
+                                    .exec(commit_ansi_transaction_sql(*depth))
                                     .map(|_| {
-                                        shared.transaction_depth.fetch_sub(1, Ordering::Release);
+                                        *depth -= 1;
                                     })
                             } else {
                                 Ok(())
@@ -249,13 +249,13 @@ impl ConnectionWorker {
                                 continue;
                             }
 
-                            let depth = shared.transaction_depth.load(Ordering::Acquire);
+                            let mut depth = shared.transaction_depth.write();
 
-                            let res = if depth > 0 {
+                            let res = if *depth > 0 {
                                 conn.handle
-                                    .exec(rollback_ansi_transaction_sql(depth))
+                                    .exec(rollback_ansi_transaction_sql(*depth))
                                     .map(|_| {
-                                        shared.transaction_depth.fetch_sub(1, Ordering::Release);
+                                        *depth -= 1;
                                     })
                             } else {
                                 Ok(())

This approach would ensure that operations on transaction_depth are handled atomically as a single unit, avoiding potential race conditions that could occur with separate AtomicUsize operations. If you believe this implementation with parking_lot::RwLock is preferable to the current AtomicUsize approach, we can consider adopting it. What are your thoughts?

[abonander]

Merged in #3765