prosgarz35 opened a new pull request, #3200:
URL: https://github.com/apache/james-project/pull/3200

       ## Motivation & Problem Statement
   
       In the PostgreSQL storage implementation (`postgres-app` / 
`mailbox-postgres`), message bodies and attachments are deduplicated and stored 
in a shared `BlobStore` (e.g., S3, FileBlobStore), while message records are 
maintained
     across two relational tables:
       1. `message_mailbox` (holds mailbox-specific entries, UIDs, flags, and 
references to `message_id`)
       2. `message` (holds deduplicated metadata and the pointer `body_blob_id` 
pointing to the physical payload in `BlobStore`).
   
       When messages are expunged or a mailbox is deleted, the asynchronous 
`DeleteMessageListener` cleans up unreferenced messages. Previously, the 
deletion lifecycle followed this sequence:
   
     1. SELECT 1 FROM message_mailbox WHERE message_id = ? (via isUnreferenced)
     2. If unreferenced:
     a. blobStore.delete(bodyBlobId)   <-- Physical S3/BlobStore deletion FIRST!
     b. attachmentDAO.deleteBlobs(...)
     c. threadDAO.deleteSome(...)
     d. messageDAO.deleteByMessageId(...) <-- Database deletion LAST
   
   
       ### The Data Loss Race Condition
       Because the initial check (`isUnreferenced`) and the final database 
deletion were non-atomic and separated by network I/O to the `BlobStore`:
       1. **Concurrent Message Duplication / Copying**: A message in Folder A 
is expunged while concurrently being copied (`IMAP COPY`, `JMAP message 
move/import`, or concurrent multi-recipient delivery with deduplication) into 
Folder B.
       2. `DeleteMessageListener` checks `isUnreferenced()`: if executed before 
Folder B's record is committed, it returns `true`.
       3. The listener immediately proceeds to **permanently delete the body 
and attachment blobs in `BlobStore`**.
       4. Once the new `message_mailbox` record for Folder B commits, the 
foreign key constraint on `message` prevents `deleteByMessageId` from 
completing (or it fails with a foreign key violation), leaving an active record 
in Folder B.
       5. **Impact**: The user sees the email listed in Folder B, but opening 
or fetching the email body fails with `BlobNotFoundException` / `IOException`. 
**The email content is permanently lost.**
   
       ---
   
       ## Solution: Atomic Database-First Conditional Delete (CAS)
   
       This PR addresses the race condition by enforcing a strict 
**Database-First Atomic Conditional Deletion** policy:
   
       1. **Atomic CAS Query in `PostgresMessageDAO`**:
          Introduced `deleteIfUnreferenced(PostgresMessageId messageId)`:
          ```sql
          DELETE FROM message
          WHERE message_id = ?
            AND NOT EXISTS (
                SELECT 1
                FROM message_mailbox
                WHERE message_mailbox.message_id = message.message_id
            )
          RETURNING body_blob_id;
   
     • This conditional deletion is atomic within PostgreSQL.
     • If concurrent references exist or are added in message_mailbox, the 
DELETE query returns 0 rows and leaves the row untouched.
     • If and only if no references exist, the row is safely removed and 
returns the body_blob_id.
   
     2. Safe BlobStore Teardown in DeleteMessageListener:
         • DeleteMessageListener now relies solely on deleteIfUnreferenced.
         • If PostgreSQL confirms the row was deleted (returning 
Optional<BlobId>), the listener safely proceeds to delete the body and 
attachment blobs from BlobStore.
         • If the database indicates the row is still referenced, the listener 
short-circuits immediately. The physical payload in BlobStore is never touched.
   
     ──────
     ## Changes
   
     • PostgresMessageDAO: Added deleteIfUnreferenced(PostgresMessageId) 
utilizing atomic NOT EXISTS subquery with RETURNING body_blob_id.
     • DeleteMessageListener:
         • Refactored handleMessageDeletion to execute deleteIfUnreferenced 
before touching external storage.
         • Removed separate, non-atomic isUnreferenced query and deprecated 
deleteBodyBlob.
     • DeleteMessageListenerContract: Added test 
deleteIfUnreferencedShouldNotDeleteBlobWhenConcurrentReferenceExists verifying 
that concurrent references prevent both database row deletion and blob 
destruction.
     ──────
     ## Verification & Compatibility
   
     • Verified backward-compatibility and zero schema migration needed (uses 
existing indices on message_mailbox(message_id)).
     • Full Checkstyle compliance (0 violations).
     • Unit & contract test compilation passing with Maven (mvn test-compile 
-pl :apache-james-mailbox-postgres).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to