docs: Document thread-safe RateLimiter for multi-agent usage#255
docs: Document thread-safe RateLimiter for multi-agent usage#255MervinPraison merged 2 commits intomainfrom
Conversation
) - Rewrite docs/features/rate-limiter.mdx with comprehensive thread safety content - Add hero diagram, 3-step Quick Start, sequence + mode-selection diagrams - Include Thread Safety & Multi-Agent Use section with code patterns - Add lock types table entry and cross-link in docs/features/thread-safety.mdx - Follow AGENTS.md structure with Steps, AccordionGroup, CardGroup components - Use standard Mermaid color scheme (#8B0000, #F59E0B, #10B981, #6366F1) - All code examples use correct imports and are copy-paste runnable 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Claude <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughDocumentation updates to rate limiter features defining shared RPM/TPM budget architecture across agents and threads. Includes expanded configuration guidance, thread-safety guarantees, code examples for concurrent usage, and architectural diagrams. Parallel update to thread-safety documentation adds RateLimiter lock-type mappings. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly enhances the Rate Limiter documentation with new diagrams, usage examples, and configuration details, while also updating the thread-safety guide. Feedback indicates that the documentation makes incorrect claims regarding the thread-safety of synchronous operations, as the underlying implementation reportedly lacks the necessary locking mechanisms. These factual errors in the documentation should be corrected to accurately reflect the code's behavior.
| | Step | What happens | | ||
| |------|--------------| | ||
| | Refill | Tokens regenerate based on elapsed time and `requests_per_minute` / `tokens_per_minute`. | | ||
| | Acquire | A thread reserves a token; under contention, only one thread mutates state at a time. | |
There was a problem hiding this comment.
The description for the 'Acquire' step states that 'under contention, only one thread mutates state at a time'. This is currently inaccurate for synchronous calls. The implementation of RateLimiter.acquire() in praisonaiagents/llm/rate_limiter.py does not use any threading.Lock, meaning concurrent calls from multiple threads will result in race conditions when updating internal state like _tokens and _last_update.
|
|
||
| <Note> | ||
| The standalone `rate_limiter=` parameter is deprecated. Use `execution=ExecutionConfig(rate_limiter=obj)` instead. | ||
| Every method on `RateLimiter` — both sync (`acquire`, `acquire_tokens`, `try_acquire`, `reset`) and async (`acquire_async`, `acquire_tokens_async`) — is safe to call concurrently. You can share a single `RateLimiter` across threads, `AgentTeam` members, `PraisonAIAgents`, and `ParallelToolCallExecutor` workers without exceeding the configured budget. |
There was a problem hiding this comment.
This section claims that synchronous methods such as acquire and reset are safe to call concurrently. However, the source code for RateLimiter lacks threading.Lock protection for these methods. Sharing a single instance across threads (e.g., using ThreadPoolExecutor as shown in the example below) is not thread-safe for synchronous operations in the current implementation.
| ``` | ||
|
|
||
| <Note> | ||
| `available_tokens` and `available_api_tokens` are safe to read from any thread — they acquire the same locks as `acquire()` internally. |
There was a problem hiding this comment.
The assertion that available_tokens and available_api_tokens acquire locks internally is incorrect. Neither these properties nor the synchronous acquire() method implement locking in the provided source code. This makes monitoring the budget from multiple threads unsafe as it triggers state mutations via _refill() without synchronization.
|
|
||
| ### Rate Limiter | ||
|
|
||
| `RateLimiter` can be shared across threads and agents. Both the sync and async method families are fully locked — see [Rate Limiter → Thread Safety & Multi-Agent Use](/docs/features/rate-limiter#thread-safety--multi-agent-use) for patterns. |
There was a problem hiding this comment.
The documentation states that synchronous method families of RateLimiter are 'fully locked'. However, the implementation in praisonaiagents/llm/rate_limiter.py does not use locks for synchronous methods, only for the lazy initialization of async locks. This statement is misleading for users expecting thread safety in multi-threaded environments.
| |-----------|-----------|--------| | ||
| | chat_history | `Lock` | Simple mutual exclusion | | ||
| | caches | `RLock` | Allows reentrant access | | ||
| | `RateLimiter` (sync) | `threading.Lock` | Protects `_tokens`, `_api_tokens`, and refill state from races in multi-threaded acquire calls | |
There was a problem hiding this comment.
The table entry for RateLimiter (sync) incorrectly identifies threading.Lock as the protection mechanism for internal state. In reality, the implementation does not use this lock during acquisition calls, leaving the state vulnerable to races in multi-threaded contexts. The documentation should be corrected to reflect the actual implementation or the implementation should be updated to match these claims.
…0424-1016 # Conflicts: # docs/features/rate-limiter.mdx
Fixes #252
This PR implements the documentation requirements from issue #252 to document the thread-safe RateLimiter functionality for multi-agent and parallel usage scenarios.
Changes Made
Major Update: docs/features/rate-limiter.mdx
Minor Update: docs/features/thread-safety.mdx
Quality Verification
Generated with Claude Code
Summary by CodeRabbit