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 check-then-act race condition #1284

Merged
merged 2 commits into from
Sep 30, 2023
Merged

Conversation

vlad-ivanov-name
Copy link
Collaborator

@vlad-ivanov-name vlad-ivanov-name commented Sep 29, 2023

The semaphore is aquired after the conditions for fetching are already
evaluated. It means that multiple threads/runners can decide to fetch
simultaneously and then wait on the same semaphore, even though only one
fetch is needed. Therefore, fetch would happen more often than what is
specified in fetch times, and requests can be slow under load.

commit-id:a4cd58e6


Stack:

⚠️ Part of a stack created by spr. Do not merge manually using the UI - doing so may have unexpected results.

All crates in workspace are now 2021 Rust

commit-id:e6fac661
The semaphore is aquired after the conditions for fetching are already
evaluated. It means that multiple threads/runners can decide to fetch
simultaneously and then wait on the same semaphore, even though only one
fetch is needed. Therefore, fetch would happen more often than what is
specified in fetch times, and requests can be slow under load.

commit-id:a4cd58e6
@vlad-ivanov-name vlad-ivanov-name changed the base branch from spr/master/e6fac661 to master September 30, 2023 09:50
@vlad-ivanov-name vlad-ivanov-name merged commit 1586eab into master Sep 30, 2023
1 check passed
@vlad-ivanov-name vlad-ivanov-name deleted the spr/master/a4cd58e6 branch September 30, 2023 09:50
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