Skip to content
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

fix: Make race condition between channel open and RPC less likely to occur #5514

Merged
merged 5 commits into from
Feb 12, 2025

Conversation

lokitoth
Copy link
Member

Right now we rely on opening the channel to associate a ClientId with an entry on the gateway side. This causes a race when the channel is being opened in the background while an RPC (e.g. MyAgent.register()) is invoked.

If the RPC is processed first, the gateway rejects it due to "invalid" clientId.

This fix makes this condition less likely to trigger, but there is still a piece of the puzzle that needs to be solved on the Gateway side.

Right now we rely on opening the channel to associate a ClientId with an entry on the gateway side. This causes a race when the channel is being opened in the background while an RPC (e.g. MyAgent.register()) is invoked.

If the RPC is processed first, the gateway rejects it due to "invalid" clientId.
@lokitoth lokitoth force-pushed the dev/fix_grpcagentruntime_startup2 branch from 62e9bea to b324983 Compare February 12, 2025 17:59
Copy link

codecov bot commented Feb 12, 2025

Codecov Report

Attention: Patch coverage is 88.23529% with 2 lines in your changes missing coverage. Please review.

Project coverage is 78.35%. Comparing base (f49f159) to head (f3bb234).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...t/src/autogen_ext/runtimes/grpc/_worker_runtime.py 88.23% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5514   +/-   ##
=======================================
  Coverage   78.35%   78.35%           
=======================================
  Files         167      167           
  Lines        9923     9926    +3     
=======================================
+ Hits         7775     7778    +3     
  Misses       2148     2148           
Flag Coverage Δ
unittests 78.35% <88.23%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lokitoth lokitoth force-pushed the dev/fix_grpcagentruntime_startup2 branch from b324983 to 5e0e0fb Compare February 12, 2025 18:05
@lokitoth lokitoth force-pushed the dev/fix_grpcagentruntime_startup2 branch from 8654e71 to f3bb234 Compare February 12, 2025 21:31
@lokitoth lokitoth merged commit 676b611 into main Feb 12, 2025
53 checks passed
@lokitoth lokitoth deleted the dev/fix_grpcagentruntime_startup2 branch February 12, 2025 21:40
lokitoth added a commit that referenced this pull request Feb 13, 2025
…registration (#5521)

This finishes the fix for the race condition between opening a
GrpcWorkerConnection and registering agent types on that worker. Now,
instead of failing to register, we return from the call (with the
expectation that we will finish registration as we set up the
connection)

Part 1: #5494 
Part 2: #5514

---------

Co-authored-by: Ryan Sweet <[email protected]>
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