Procházet zdrojové kódy

Wrap check-then-write account and book creation in transactions

create_account, append_account_version, and create_book each ran a SELECT
existence/version check and then an INSERT as two separate statements on the
pool. Two concurrent calls for the same id could both pass the check, or two
appends could read the same version and race to insert current + 1, yielding
a torn read or a raw primary-key error instead of a clean AlreadyExists /
VersionConflict.

Wrap each check and insert in a single transaction (begin, check, insert,
commit) so the pair is atomic. The in-memory store already holds its write
lock across the check and insert, so only the SQL backend needed this.

Claude-Session: https://claude.ai/code/session_01SJFJen8Ethv9Q6Ysb1xmz4
Cesar Rodas před 1 dnem
rodič
revize
ac0c7df5cd
1 změnil soubory, kde provedl 44 přidání a 6 odebrání
  1. 44 6
      crates/kuatia-storage-sql/src/lib.rs

+ 44 - 6
crates/kuatia-storage-sql/src/lib.rs

@@ -267,11 +267,20 @@ impl AccountStore for SqlStore {
     }
 
     async fn create_account(&self, account: Account) -> Result<(), StoreError> {
+        // Wrap the existence check and the insert in one transaction so two
+        // concurrent creates of the same (id, subaccount) cannot both pass the
+        // check; the loser sees the conflict rather than a torn read.
+        let mut tx = self
+            .pool
+            .begin()
+            .await
+            .map_err(|e| StoreError::Internal(e.to_string()))?;
+
         let exists =
             sqlx::query("SELECT 1 FROM accounts WHERE id = $1 AND subaccount = $2 LIMIT 1")
                 .bind(account.id.id)
                 .bind(account.id.sub as i64)
-                .fetch_optional(&self.pool)
+                .fetch_optional(&mut *tx)
                 .await
                 .map_err(|e| StoreError::Internal(e.to_string()))?;
         if exists.is_some() {
@@ -292,19 +301,32 @@ impl AccountStore for SqlStore {
             .bind(account.book.0)
             .bind(serialize_json(&account.user_data)?)
             .bind(serialize_json(&account.metadata)?)
-            .execute(&self.pool)
+            .execute(&mut *tx)
+            .await
+            .map_err(|e| StoreError::Internal(e.to_string()))?;
+
+        tx.commit()
             .await
             .map_err(|e| StoreError::Internal(e.to_string()))?;
         Ok(())
     }
 
     async fn append_account_version(&self, account: Account) -> Result<(), StoreError> {
+        // Read the current version and insert the next one atomically, so two
+        // concurrent appends cannot both read the same version and race to insert
+        // `current + 1`.
+        let mut tx = self
+            .pool
+            .begin()
+            .await
+            .map_err(|e| StoreError::Internal(e.to_string()))?;
+
         let current = sqlx::query(
             "SELECT version FROM accounts WHERE id = $1 AND subaccount = $2 ORDER BY version DESC LIMIT 1",
         )
         .bind(account.id.id)
         .bind(account.id.sub as i64)
-        .fetch_optional(&self.pool)
+        .fetch_optional(&mut *tx)
         .await
         .map_err(|e| StoreError::Internal(e.to_string()))?
         .ok_or_else(|| StoreError::NotFound(format!("account {:?}", account.id)))?;
@@ -335,7 +357,11 @@ impl AccountStore for SqlStore {
             .bind(account.book.0)
             .bind(serialize_json(&account.user_data)?)
             .bind(serialize_json(&account.metadata)?)
-            .execute(&self.pool)
+            .execute(&mut *tx)
+            .await
+            .map_err(|e| StoreError::Internal(e.to_string()))?;
+
+        tx.commit()
             .await
             .map_err(|e| StoreError::Internal(e.to_string()))?;
         Ok(())
@@ -964,9 +990,17 @@ impl EventStore for SqlStore {
 #[async_trait]
 impl BookStore for SqlStore {
     async fn create_book(&self, book: Book) -> Result<(), StoreError> {
+        // Existence check and insert in one transaction so two concurrent creates
+        // of the same book id cannot both pass the check.
+        let mut tx = self
+            .pool
+            .begin()
+            .await
+            .map_err(|e| StoreError::Internal(e.to_string()))?;
+
         let exists = sqlx::query("SELECT 1 FROM books WHERE id = $1 LIMIT 1")
             .bind(book.id.0)
-            .fetch_optional(&self.pool)
+            .fetch_optional(&mut *tx)
             .await
             .map_err(|e| StoreError::Internal(e.to_string()))?;
         if exists.is_some() {
@@ -978,7 +1012,11 @@ impl BookStore for SqlStore {
             .bind(book.id.0)
             .bind(&book.name)
             .bind(&data)
-            .execute(&self.pool)
+            .execute(&mut *tx)
+            .await
+            .map_err(|e| StoreError::Internal(e.to_string()))?;
+
+        tx.commit()
             .await
             .map_err(|e| StoreError::Internal(e.to_string()))?;
         Ok(())