aiolabs/castle
1
0
Fork 0
Code Issues 5 Pull requests 2 Projects Releases 5 Packages Wiki Activity Actions

Add concurrency protection for Fava/Beancount ledger writes #5

Open
padreug wants to merge 0 commits from fix/concurrency-protection into main
pull from: fix/concurrency-protection
merge into: aiolabs:main
Conversation 0 Commits - Files changed -
padreug commented 2026-01-06 22:57:27 +00:00
Owner
Copy link

Summary

This PR addresses critical race conditions when multiple requests try to write to the Fava/Beancount ledger file simultaneously. Without these protections, concurrent writes can cause data loss, duplicate entries, or file corruption.

Fixes #4

Fix Implemented

The following concurrency protections have been implemented:

1. Global Write Lock in FavaClient

Added asyncio.Lock() to serialize all write operations to the Fava/Beancount ledger:

class FavaClient:
    def __init__(self, ...):
        self._write_lock = asyncio.Lock()
        self._user_locks: Dict[str, asyncio.Lock] = {}

2. Protected Write Methods

The following methods now acquire the global write lock before modifying the ledger:

  • add_entry() - Wrapped with async with self._write_lock:
  • add_account() - Wrapped with lock + retry logic for checksum conflicts
  • update_entry_source() - Wrapped with lock
  • delete_entry() - Wrapped with lock

3. Retry Logic with Exponential Backoff

add_account() now implements optimistic concurrency control:

  • Retries up to 3 times on checksum conflict (HTTP 409/412)
  • Exponential backoff: 0.1s, 0.2s, 0.4s between retries
  • Re-checks if account was created by concurrent request before retrying
  • Raises ChecksumConflictError if all retries fail

4. Idempotent Entry Creation

New add_entry_idempotent() method:

result = await fava.add_entry_idempotent(entry, idempotency_key="ln-abc123")
if result.get("existing"):
    # Entry already existed, no duplicate created
  • Checks if entry with idempotency key (as Beancount link) already exists
  • Prevents duplicate entries even when same operation is retried
  • Returns existing entry data if found

5. Per-User Locking

Added get_user_lock(user_id) method for user-specific operations:

user_lock = fava.get_user_lock(user_id)
async with user_lock:
    # User-specific operations serialized

6. Protected Invoice Payment Processing

Updated on_invoice_paid() in tasks.py:

  • Uses per-user lock to serialize payment processing per user
  • Uses add_entry_idempotent() with payment hash as idempotency key
  • Prevents duplicate settlement entries even if same payment processed twice

Files Changed

  • fava_client.py - Added locks, retry logic, idempotent method
  • tasks.py - Updated on_invoice_paid() to use new protections

What This Fixes

Scenario Before After
Two users submit expenses simultaneously Data loss possible Serialized, both recorded
Same invoice processed twice Duplicate entries Single entry (idempotent)
Concurrent account creation 409 error Retry succeeds
User A and B simultaneous requests Can corrupt each other Per-user isolation

Test Plan

  • Verify concurrent expense submissions are serialized (no data loss)
  • Verify same payment hash processed twice creates only one entry
  • Verify account creation conflicts retry and succeed
  • Verify users don't block each other (per-user locks)
  • Run existing tests to ensure no regressions

Limitations

  • Global write lock serializes ALL writes (potential bottleneck at scale)
  • For horizontal scaling, distributed locking (Redis) would be needed
  • Per-user locks are in-memory only (don't persist across restarts)

🤖 Generated with Claude Code

## Summary This PR addresses **critical race conditions** when multiple requests try to write to the Fava/Beancount ledger file simultaneously. Without these protections, concurrent writes can cause data loss, duplicate entries, or file corruption. Fixes #4 --- ## Fix Implemented The following concurrency protections have been implemented: ### 1. Global Write Lock in FavaClient Added `asyncio.Lock()` to serialize all write operations to the Fava/Beancount ledger: ```python class FavaClient: def __init__(self, ...): self._write_lock = asyncio.Lock() self._user_locks: Dict[str, asyncio.Lock] = {} ``` ### 2. Protected Write Methods The following methods now acquire the global write lock before modifying the ledger: - `add_entry()` - Wrapped with `async with self._write_lock:` - `add_account()` - Wrapped with lock + retry logic for checksum conflicts - `update_entry_source()` - Wrapped with lock - `delete_entry()` - Wrapped with lock ### 3. Retry Logic with Exponential Backoff `add_account()` now implements optimistic concurrency control: - Retries up to 3 times on checksum conflict (HTTP 409/412) - Exponential backoff: 0.1s, 0.2s, 0.4s between retries - Re-checks if account was created by concurrent request before retrying - Raises `ChecksumConflictError` if all retries fail ### 4. Idempotent Entry Creation New `add_entry_idempotent()` method: ```python result = await fava.add_entry_idempotent(entry, idempotency_key="ln-abc123") if result.get("existing"): # Entry already existed, no duplicate created ``` - Checks if entry with idempotency key (as Beancount link) already exists - Prevents duplicate entries even when same operation is retried - Returns existing entry data if found ### 5. Per-User Locking Added `get_user_lock(user_id)` method for user-specific operations: ```python user_lock = fava.get_user_lock(user_id) async with user_lock: # User-specific operations serialized ``` ### 6. Protected Invoice Payment Processing Updated `on_invoice_paid()` in tasks.py: - Uses per-user lock to serialize payment processing per user - Uses `add_entry_idempotent()` with payment hash as idempotency key - Prevents duplicate settlement entries even if same payment processed twice --- ## Files Changed - `fava_client.py` - Added locks, retry logic, idempotent method - `tasks.py` - Updated `on_invoice_paid()` to use new protections ## What This Fixes | Scenario | Before | After | |----------|--------|-------| | Two users submit expenses simultaneously | Data loss possible | Serialized, both recorded | | Same invoice processed twice | Duplicate entries | Single entry (idempotent) | | Concurrent account creation | 409 error | Retry succeeds | | User A and B simultaneous requests | Can corrupt each other | Per-user isolation | ## Test Plan - [ ] Verify concurrent expense submissions are serialized (no data loss) - [ ] Verify same payment hash processed twice creates only one entry - [ ] Verify account creation conflicts retry and succeed - [ ] Verify users don't block each other (per-user locks) - [ ] Run existing tests to ensure no regressions ## Limitations - Global write lock serializes ALL writes (potential bottleneck at scale) - For horizontal scaling, distributed locking (Redis) would be needed - Per-user locks are in-memory only (don't persist across restarts) 🤖 Generated with [Claude Code](https://claude.com/claude-code)
padreug added 1 commit 2026-01-06 22:57:28 +00:00 
This commit addresses critical race conditions when multiple requests
try to write to the ledger file simultaneously.

Changes:
- Add global asyncio.Lock to FavaClient to serialize all write operations
- Add per-user locks for finer-grained concurrency control
- Wrap add_entry(), update_entry_source(), delete_entry() with write lock
- Add retry logic with exponential backoff to add_account() for checksum conflicts
- Add new add_entry_idempotent() method to prevent duplicate entries
- Add ChecksumConflictError exception for conflict handling
- Update on_invoice_paid() to use per-user locking and idempotent entry creation

Fixes #4

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This branch is already included in the target branch. There is nothing to merge.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix/concurrency-protection:fix/concurrency-protection
git checkout fix/concurrency-protection

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git checkout main
git merge --no-ff fix/concurrency-protection
git checkout fix/concurrency-protection
git rebase main
git checkout main
git merge --ff-only fix/concurrency-protection
git checkout fix/concurrency-protection
git rebase main
git checkout main
git merge --no-ff fix/concurrency-protection
git checkout main
git merge --squash fix/concurrency-protection
git checkout main
git merge --ff-only fix/concurrency-protection
git checkout main
git merge fix/concurrency-protection
git push origin main
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: aiolabs/castle#5
No description provided.
Delete branch "fix/concurrency-protection"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?