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(workflow): Add locks to groupowners to prevent duplicates #29309

Merged
merged 4 commits into from
Oct 19, 2021

Conversation

scttcper
Copy link
Member

FIXES SENTRY-MP8

There's a race condition on creating group owners when there are multiple tasks. This adds a lock and should deduplicate extras?

@scttcper scttcper requested review from a team and wedamija October 14, 2021 02:44
GroupOwner.objects.bulk_create(new_group_owners)
lock = locks.get(f"groupowner-bulk:{group.id}", duration=10)
try:
with lock.acquire():
Copy link
Member

Choose a reason for hiding this comment

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

I'd recommend putting this lock around the entire block that is used to calculate new_group_owners, since that's where the duplication happens as well. Basically the same spot where we start using the timer.

Comment on lines 136 to 137
with lock.acquire():
with metrics.timer("post_process.handle_group_owners"):
Copy link
Member

Choose a reason for hiding this comment

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

Nit: You can combine with statements together like
with lock.acquire(), metrics.timer("post_process.handle_group_owners"):

Copy link
Member Author

Choose a reason for hiding this comment

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

no way

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants