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]