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 soft delete for statuses for instant deletes through API #11623

Merged
merged 3 commits into from
Aug 22, 2019

Conversation

Gargron
Copy link
Member

@Gargron Gargron commented Aug 19, 2019

  • Fix bug where same status was fetched multiple times in the unreblog API
  • Fix bug where reblogs of sensitive statuses were showing media without spoiler in admin UI

@ClearlyClaire
Copy link
Contributor

What are the performance implications of that?
Also, is it possible that SuspendAccountService may skip deleting discarded statuses?

@Gargron
Copy link
Member Author

Gargron commented Aug 20, 2019

What are the performance implications of that?

Not entirely sure, unfortunately. PG picks different query plans depending on table size so getting real performance estimates in development is hard.

Also, is it possible that SuspendAccountService may skip deleting discarded statuses?

Yes, but because discarding and queueing removal worker happens at the same time, I feel like we don't need to care about those.

@Gargron
Copy link
Member Author

Gargron commented Aug 20, 2019

I have updated the index for the slowest query, getting an account's statuses. I believe the queries for public timelines will not benefit from other changes.

@Gargron Gargron changed the title Add soft delete for statuses to allow them to appear instant Add soft delete for statuses for instant deletes through API Aug 21, 2019
def up
safety_assured { add_index :statuses, [:account_id, :id, :visibility, :updated_at], where: 'deleted_at IS NULL', order: { id: :desc }, algorithm: :concurrently, name: :index_statuses_20190820 }
remove_index :statuses, name: :index_statuses_20180106
end
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't fully understand this, so I can't comment on that, and I don't understand the performance implications, which I am worried about.

Other than that, looks fine to me

Copy link
Member Author

Choose a reason for hiding this comment

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

So, the index is made smaller by limiting it to rows that have a null deleted_at. Because the index is mainly used for queries that ask for rows where the deleted_at column is null (as that is the default scope), it will work like before, and not need to add a further filter for deleted_at in the query planner.

@Gargron Gargron merged commit 282ea17 into master Aug 22, 2019
@Gargron Gargron deleted the feature-soft-delete branch August 22, 2019 19:56
Gargron added a commit that referenced this pull request Sep 27, 2019
Gargron added a commit that referenced this pull request Sep 27, 2019
Gargron added a commit that referenced this pull request Sep 27, 2019
Gargron added a commit that referenced this pull request Sep 27, 2019
hiyuki2578 pushed a commit to ProjectMyosotis/mastodon that referenced this pull request Oct 2, 2019
…n#11623)

* Add soft delete for statuses to allow them to appear instant

* Allow reporting soft-deleted statuses and show them in the admin UI

* Change index for getting an account's statuses
hiyuki2578 pushed a commit to ProjectMyosotis/mastodon that referenced this pull request Oct 2, 2019
hiyuki2578 pushed a commit to ProjectMyosotis/mastodon that referenced this pull request Oct 2, 2019
rtucker pushed a commit to vulpineclub/mastodon that referenced this pull request Jan 7, 2021
rtucker referenced this pull request in vulpineclub/mastodon Jan 7, 2021
… instant deletes through API

* Add soft delete for statuses to allow them to appear instant

* Allow reporting soft-deleted statuses and show them in the admin UI

* Change index for getting an account's statuses
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.

3 participants