Skip to content

fix: remove exclusive locks from read-only storage operations#4854

Merged
greysonlalonde merged 3 commits into
mainfrom
fix/remove-read-locks-reduce-contention
Mar 14, 2026
Merged

fix: remove exclusive locks from read-only storage operations#4854
greysonlalonde merged 3 commits into
mainfrom
fix/remove-read-locks-reduce-contention

Conversation

@greysonlalonde

@greysonlalonde greysonlalonde commented Mar 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • remove store_lock from read-only storage operations across LanceDB, ChromaDB, and RAG adapters that were causing exclusive lock contention under multi-process prefork workers
  • wrap lock_store BaseLockException with an actionable error message pointing users to set REDIS_URL
  • add exception handling in encoding_flow/recall_flow ThreadPoolExecutor calls so a failed storage search degrades gracefully instead of crashing the flow
  • fix flow.py double-logging where ancestor listeners re-logged downstream errors

Context

under prefork workers with 8 concurrency, read operations like search, list_scopes, get_scope_info, count were all holding exclusive Redis locks via portalocker.RedisLock. with 8 workers x N threads all fighting for the same lock channel, contention triggered a portalocker bug where AlreadyLocked is raised with the exceptions module object as its argument, producing the uninformative <module 'portalocker.exceptions'> error.

LanceDB uses MVCC and ChromaDB is thread-safe since v0.4 — neither needs exclusive locks for reads.

Test plan

  • 108 existing memory/chromadb/lock tests pass
  • 8 concurrent crew kickoffs via celery prefork + redis locks: 0 errors
  • 50 concurrent crew kickoffs via celery prefork + redis locks: 0 errors, 104k log lines, zero portalocker/lock/trace_id errors
  • LanceDB concurrent read safety verified empirically: 1400 reads + 40 writes across 16 threads, 0 errors

Note

Medium Risk
Touches concurrency/locking around storage reads and modifies error handling in memory flows; behavior under multi-process load could change (e.g., races or previously-serialized reads now concurrent) and should be validated in real deployments.

Overview
Reduces lock contention by removing exclusive store_lock usage from read-only operations across LanceDB/ChromaDB integrations (e.g., LanceDB adapter query, RAG query, ChromaDB client search/asearch, and multiple LanceDBStorage read paths like search, get_record, list_scopes, get_scope_info, count, etc.).

Improves resilience and diagnostics under concurrency by (1) wrapping portalocker lock acquisition failures with a clearer LockException message in lock_store.py, (2) making EncodingFlow and RecallFlow degrade gracefully when parallel storage searches/listing fail (log warning + skip/fallback instead of crashing), and (3) preventing duplicate listener error logs in Flow._execute_single_listener by marking exceptions as already logged.

Written by Cursor Bugbot for commit 91bad3b. This will update automatically on new commits. Configure here.

…inate lock contention

read operations like search, list_scopes, get_scope_info, count across
LanceDB, ChromaDB, and RAG adapters were holding exclusive locks unnecessarily.
under multi-process prefork workers this caused RedisLock contention triggering
a portalocker bug where AlreadyLocked is raised with the exceptions module as its arg.

- remove store_lock from 7 LanceDB read methods since MVCC handles concurrent reads
- remove store_lock from ChromaDB search/asearch which are thread-safe since v0.4
- remove store_lock from RAG core query and LanceDB adapter query
- wrap lock_store BaseLockException with actionable error message
- add exception handling in encoding_flow/recall_flow ThreadPoolExecutor calls
- fix flow.py double-logging of ancestor listener errors

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment thread lib/crewai/src/crewai/memory/recall_flow.py Outdated
Comment thread lib/crewai/src/crewai/utilities/lock_store.py
both branches of the if/else and the except all produced the same
candidates = [scope_prefix] result, making the get_scope_info call
and conditional pointless
the try/except wrapped the yield inside the contextmanager, which meant
any BaseLockException raised by the caller's code inside the with block
would be caught and re-raised with a misleading "Failed to acquire lock"
message. split into acquire-then-yield so only actual acquisition
failures get the actionable error message.
@greysonlalonde greysonlalonde merged commit b8d7942 into main Mar 14, 2026
46 checks passed
@greysonlalonde greysonlalonde deleted the fix/remove-read-locks-reduce-contention branch March 14, 2026 04:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants