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

Add mutex locks to installers #7053

Merged
merged 3 commits into from Oct 21, 2022
Merged

Add mutex locks to installers #7053

merged 3 commits into from Oct 21, 2022

Conversation

viren-nadkarni
Copy link
Member

@viren-nadkarni viren-nadkarni commented Oct 20, 2022

This PR moves the installation steps behind a mutex lock to avoid race conditions if the installer is invoked multiple times.

We rely on the LRU-cached get_installer() method to return the same installer instance for a given version to ensure the same lock is utilised.

@viren-nadkarni viren-nadkarni temporarily deployed to localstack-ext-tests October 20, 2022 12:28 Inactive
@github-actions
Copy link

github-actions bot commented Oct 20, 2022

LocalStack integration with Pro

       3 files  ±0         3 suites  ±0   1h 22m 0s ⏱️ - 2m 45s
1 410 tests ±0  1 227 ✔️ ±0  183 💤 ±0  0 ±0 
2 008 runs  ±0  1 593 ✔️ ±0  415 💤 ±0  0 ±0 

Results for commit 726b062. ± Comparison against base commit 1122926.

♻️ This comment has been updated with latest results.

@viren-nadkarni viren-nadkarni temporarily deployed to localstack-ext-tests October 21, 2022 06:06 Inactive
@viren-nadkarni viren-nadkarni marked this pull request as ready for review October 21, 2022 06:08
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! The implementation looks good! I just added a small comment to add a docstring.

localstack/packages/api.py Show resolved Hide resolved
@viren-nadkarni viren-nadkarni temporarily deployed to localstack-ext-tests October 21, 2022 06:30 Inactive
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 79.335% when pulling 726b062 on installer-mutex into 1122926 on master.

@viren-nadkarni viren-nadkarni merged commit 93e2bbe into master Oct 21, 2022
@viren-nadkarni viren-nadkarni deleted the installer-mutex branch October 21, 2022 08:03
@alexrashed
Copy link
Member

FYI: This PR is based on #6783

alexrashed pushed a commit that referenced this pull request Oct 24, 2022
cmoralesmx pushed a commit to cmoralesmx/localstack that referenced this pull request Oct 24, 2022
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

3 participants