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
credentials/alts: defer ALTS stream creation until handshake time #6077
credentials/alts: defer ALTS stream creation until handshake time #6077
Conversation
cc: @apolcyn |
@arvindbr8 I saw you added the "Status: Requires Report Clarification" tag: please let me know what you'd like me to clarify. :) |
Also, is the title of the PR up-to-date? I don't any see lock being involved here. |
The lock is hidden under the call to acquire(). Updated the title to clarify. :) |
Thanks all for the reviews! @arvindbr8 or @easwars: would one of you be able to merge the PR? I'm afraid I don't have write access. :) |
The existing code creates the stream to the ALTS handshaker service before checking that we are not exceeding the max number of concurrent connections, which defeats the purpose of having the max pending handshakes cap.
While we're here, I also lower themaxPendingHandshakes
cap to 40, to match up with the C-core code and with the current expectations of the ALTS handshaker service. (We will be lowering this cap again in the future.)RELEASE NOTES: none