-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix event compaction to nonblocking #4326
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix event compaction to nonblocking #4326
Conversation
Summary of ChangesHello @lwangverizon, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the performance and stability of the application by transforming event compaction from a blocking operation into an asynchronous background process. Previously, compaction, which involves LLM API calls and database writes, caused noticeable delays for users. The changes introduce concurrency control to manage resource usage under high load and robust error handling to prevent service disruptions, ultimately leading to a more responsive and resilient user experience. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request is a great improvement, making event compaction non-blocking and adding concurrency controls. The changes significantly enhance performance and robustness by moving the synchronous compaction process to a background task, managed by a semaphore to prevent resource exhaustion. The implementation is well-thought-out, with good error handling and comprehensive tests. I've included a few suggestions to further improve robustness, such as ensuring background tasks are not prematurely garbage collected and enhancing thread safety. Overall, this is an excellent contribution.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
2. Or, if no issue exists, describe the change:
Problem:
Event compaction was running synchronously and blocking
runner.run_async()exit, causing significant delays on the frontend. When compaction was enabled, the async generator would not complete until compaction finished, which could take several seconds because compaction involves:maybe_summarize_events) - typically taking 1-3 seconds per compactionappend_event) - adding additional latencyImpact:
Even though all agent events had already been yielded to the frontend, the generator would not complete until compaction finished. This meant:
Additional Issue:
Under high concurrency scenarios, there was no mechanism to limit concurrent compaction tasks, which could lead to:
Solution:
This PR introduces a solution to make event compaction truly non-blocking, improving application performance by eliminating the blocking delay before
runner.run_async()exits.Made compaction non-blocking: Changed compaction from synchronous
awaitto an asynchronous background task usingasyncio.create_task(). This allows:Added concurrency control: Introduced a configurable
max_concurrent_compactionsparameter (default: 10) to theRunnerclass that uses a semaphore to limit concurrent compaction tasks. This prevents:Improved error handling: Wrapped compaction in comprehensive error handling so failures:
Updated documentation: Updated docstrings to accurately reflect that compaction runs asynchronously and no longer blocks generator completion.
Key Improvement:
The solution transforms compaction from a blocking operation (that delayed frontend responses) into a truly asynchronous background task, significantly improving application responsiveness while maintaining all compaction functionality.
The solution maintains backward compatibility (default behavior works for most scenarios) while providing fine-grained control for production environments with different resource constraints.
Testing Plan
Unit Tests:
Test Results:
Test Coverage:
Manual End-to-End (E2E) Tests:
Setup:
Manual Testing Steps:
Non-blocking behavior: Run multiple invocations and observe that the generator completes immediately (within milliseconds) while compaction runs in the background. Verify frontend receives completion signal without delay.
Concurrency limiting: Under high load (multiple concurrent requests), verify that compaction tasks are limited by the semaphore. Monitor resource usage (LLM API calls, DB connections) to ensure they don't exceed limits.
Error handling: Simulate compaction failures (e.g., network errors) and verify that:
Configuration validation: Test invalid
max_concurrent_compactionsvalues (0, -1) and verifyValueErroris raised.Expected Results:
Sample Code:
See
contributing/samples/compaction_config_example/for complete examples demonstrating the new features.Checklist
Additional context
Files Changed:
src/google/adk/runners.py: Added non-blocking compaction, semaphore-based concurrency control, andmax_concurrent_compactionsparametertests/unittests/test_runners.py: Added comprehensive test suite (TestRunnerCompactionclass with 8 tests)contributing/samples/compaction_config_example/: Added sample code demonstrating the new featuresKey Implementation Details:
asyncio.create_task()with error handlingPerformance Impact:
Backward Compatibility:
max_concurrent_compactionsis optional with sensible defaultProduction Recommendations: