-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix(tools): Cache tools in BaseToolset to prevent duplicate MCP connections #3898
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?
Conversation
…ctions When get_tools_with_prefix() is called multiple times during agent preprocessing, it now caches the result per readonly_context to avoid redundant get_tools() calls. This fixes issue google#3763 where AgentTool with MCP sub-agents created multiple duplicate MCP connections, causing 3-9x API call duplication. Changes: - Add _tools_cache and _cache_context_id to BaseToolset - Cache tools in get_tools_with_prefix() based on context identity - Add tests for caching behavior
Summary of ChangesHello @dannovikov, 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 optimizes the ADK framework by introducing a caching layer to the Highlights
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
The pull request introduces a caching mechanism to BaseToolset.get_tools_with_prefix() to prevent redundant calls to get_tools() and reduce duplicate MCP connections. This is a valuable improvement addressing a significant efficiency issue. The changes correctly initialize cache variables and update the docstring to reflect the new behavior. The added unit tests verify the basic caching functionality for single and different contexts.
|
Hi @dannovikov, Thank you for your contribution! We appreciate you taking the time to submit this pull request. Your PR has been received by the team and is currently under review. We will provide feedback as soon as we have an update to share. |
|
Hi @seanzhou1023 , can you please review this. |
Link to an existing issue (if applicable):
Problem:
When using
AgentToolto call a sub-agent that uses MCP tools, ADK creates multiple duplicate MCP connections that all execute the same tool call, resulting in massive API call duplication (3-9x observed). This causes 80% of API calls to be unnecessary duplicates, wasting resources and hitting rate limits faster.The root cause is that
get_tools_with_prefix()is called multiple times during agent preprocessing (inbase_llm_flow._preprocess_async), and each call triggersget_tools()which creates new MCP connections and fetches tools from the server.Solution:
Add caching to
BaseToolset.get_tools_with_prefix()to prevent redundantget_tools()calls. The cache stores tools perreadonly_contextidentity, so:get_tools())This solution is placed at
get_tools_with_prefix()because it's the single public API entry point that the ADK framework uses to load tools from toolsets (called from_convert_tool_union_to_toolsin llm_agent.py).Testing Plan
Unit Tests:
Added tests:
test_get_tools_caching- verifiesget_tools()is called only once whenget_tools_with_prefix()is called multiple timestest_get_tools_caching_with_different_contexts- verifies cache correctly invalidates for different contextsManual End-to-End (E2E) Tests:
Run the following reproduction script to verify the fix:
Before fix:
After fix:
Checklist