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 race condition in check_auth #1331

Merged
merged 3 commits into from
May 24, 2024
Merged

Conversation

vlad-ivanov-name
Copy link
Collaborator

When a lot of requests arrive roughly at the same time, several requests can enter the critical section where an HTTP request to upstream is made to check the auth provided by the client. This means that potentially thousands of requests can get through to the remote, leading to rate limits and network errors with some remotes.

  • Fix the issue by extending the lock region
  • Switch mutex to async to avoid blocking runtime
  • Improve tracing

commit-id:d44447ef

@vlad-ivanov-name
Copy link
Collaborator Author

vlad-ivanov-name commented May 21, 2024

I will follow up with graphql fix in a different PR

@vlad-ivanov-name vlad-ivanov-name enabled auto-merge (squash) May 24, 2024 14:45
When a lot of requests arrive roughly at the same time, several requests
can enter the critical section where an HTTP request to upstream is made
to check the auth provided by the client. This means that potentially
thousands of requests can get through to the remote, leading to rate
limits and network errors with some remotes.

* Fix the issue by extending the lock region
* Switch mutex to async to avoid blocking runtime
* Improve tracing

commit-id:d44447ef
commit-id:b2e11ff1
commit-id:7d950008
@vlad-ivanov-name vlad-ivanov-name merged commit 3043aeb into master May 24, 2024
1 check passed
@vlad-ivanov-name vlad-ivanov-name deleted the fix-auth-race-condition branch May 24, 2024 14:51
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.

None yet

2 participants