Skip to content

Conversation

@mattrobenolt
Copy link
Contributor

@mattrobenolt mattrobenolt commented Aug 30, 2016

Related to 3735627

As a side effect of this previous commit, it's possible that the current
task now continues along before all of it's expected objects have
actually been removed. This leads to issues when it tries to delete the
Team/Organization which has dependent objects still remaining to be
deleted.

This just adds a check and a try again later to the task until all of
the previous tasks are actually finished.

@getsentry/infrastructure


This change is Reviewable

@mattrobenolt
Copy link
Contributor Author

I'm pretty confident that these issues would have eventually resolved themselves since the task would have kept retrying naturally until the objects were actually removed, but this is a bit more sane way to handle it.


# Wait until all Teams have actually been deleted before continuing
if Team.objects.filter(organization=o).exists():
current.retry(exc=Exception('waiting a bit'), countdown=15)
Copy link
Member

Choose a reason for hiding this comment

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

Don't use retry as I'm pretty it has side effects if max retries is enabled

@dcramer
Copy link
Member

dcramer commented Aug 30, 2016

It's unclear why this should be needed and I'm not convinced we should make the change as it's likely masking something else. Deletions should just delete with the assumption that any children are working correctly

@mattrobenolt
Copy link
Contributor Author

I could explain better IRL probably. Since this is definitely possible to hit after I fixed a previous issue that was masking this.

mattrobenolt added a commit that referenced this pull request Aug 31, 2016
mattrobenolt added a commit that referenced this pull request Aug 31, 2016
Related to 3735627

As a side effect of this previous commit, it's possible that the current
task now continues along before all of it's expected objects have
actually been removed. This leads to issues when it tries to delete the
Team/Organization which has dependent objects still remaining to be
deleted.

This just adds a check and a try again later to the task until all of
the previous tasks are actually finished.
@mattrobenolt
Copy link
Contributor Author

Updated to not use retry, but to enqueue a new task.

@mattrobenolt
Copy link
Contributor Author

It's unclear why this should be needed and I'm not convinced we should make the change as it's likely masking something else. Deletions should just delete with the assumption that any children are working correctly

The assumption here is a bit wrong. So the problem is that say, inside delete_organization, it in turn calls delete_team to delete it's children. But delete_team doesn't always finish synchronously. It's common and very possible that it defers more tasks to continue the deletion attempts. This leaves teams in DELETION_IN_PROGRESS state, so we need to wait until they're all actually gone and all the subtasks have finished.

@tkaemming
Copy link
Contributor

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@dcramer
Copy link
Member

dcramer commented Sep 2, 2016

Can we just add return values to delete team / etc


Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from Reviewable

@mattrobenolt mattrobenolt deleted the delete-wait branch September 2, 2016 01:51
@github-actions github-actions bot locked and limited conversation to collaborators Dec 23, 2020
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.

4 participants