Fix/concurrent summarization loop#455
Merged
Merged
Conversation
With dramatiq-gevent, multiple concurrent greenlets share a single OS thread, so threading.get_ident() returns the same ID for all of them. This caused _get_thread_event_loop() to return a shared cached loop to concurrent greenlets. When 4-5 task_summarize_conversation actors fired simultaneously, each called nest_asyncio-patched run_until_complete() on the same loop. The asyncio.Future objects created by run_in_executor() inside those coroutines cross-contaminated each other, raising: "got Future attached to a different loop" Fix: create a fresh asyncio.new_event_loop() per run_async_in_new_loop() call and close it in finally. Both callers (summarize_conversation, get_conversation_content) use only stateless async operations — all I/O is synchronous requests-based via run_in_thread_pool — so fresh loops per call is safe. Exposed by Stage 3 load test (5 concurrent conversations finalizing simultaneously). Pre-existing bug, not introduced by webhook changes. Co-authored-by: Cursor <cursoragent@cursor.com>
Adds tests that reproduce the exact failure condition: - Concurrent threads calling run_async_in_new_loop simultaneously - Same-thread-ID scenario (simulates gevent greenlets sharing one OS thread) Both pass with the fresh-loop-per-call fix and would have failed with the previous cached thread-loop implementation. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.