8000 Prevent race condition for Store write operations by viren-nadkarni · Pull Request #6689 · localstack/localstack · GitHub
[go: up one dir, main page]
Skip to content

Prevent race condition for Store write operations #6689

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 20, 2022
Merged

Prevent race condition for Store write operations #6689

merged 2 commits into from
Aug 20, 2022

Conversation

viren-nadkarni
Copy link
Member

Summary

Adds re-entrant lock for write operations to Stores to prevent races.

Related

Should resolve #6682

@viren-nadkarni viren-nadkarni self-assigned this Aug 17, 2022
@viren-nadkarni viren-nadkarni temporarily deployed to localstack-ext-tests August 17, 2022 12:05 Inactive
@coveralls
Copy link
coveralls commented Aug 17, 2022 •
edited
Loading

Coverage Status

Coverage decreased (-0.02%) to 91.55% when pulling 77002c4 on stores-rlock into 6591436 on master.

@github-actions
Copy link
github-actions bot commented Aug 17, 2022 •
edited
Loading

LocalStack integration with Pro

       3 files         3 suites   1h 5m 21s ⏱️
1 190 tests 1 149 ✔️ 41 💤 0 ❌
1 557 runs  1 485 ✔️ 72 💤 0 ❌

Results for commit 77002c4.

♻️ This comment has been updated with latest results.

giograno
giograno approved these changes Aug 17, 2022
View reviewed changes
Copy link
Member
@giograno giograno left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thanks for quicky jumping on this :)

thrau
thrau reviewed Aug 17, 2022
View reviewed changes
Copy link
Member
@thrau thrau left a comment

Choose a reason for hiding this comment

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

hey! just stumbled across this and have a comment regarding concurrency/performance. we should be mindful of introducing global locks for things that happen per-rquest.

@viren-nadkarni viren-nadkarni temporarily deployed to localstack-ext-tests August 18, 2022 06:51 Inactive
@thrau thrau merged commit ae4a2ad into master Aug 20, 2022
@thrau thrau deleted the stores-rlock branch August 20, 2022 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thrau thrau thrau left review comments

@giograno giograno giograno approved these changes

Assignees

@viren-nadkarni viren-nadkarni

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

bug: 1.0.x breaks concurrent BatchWriteItem to dynamodb tables
4 participants
@viren-nadkarni @coveralls @thrau @giograno
0