Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Split purge API into events vs state and add PurgeEventsStorage #6295

Merged
merged 14 commits into from
Nov 8, 2019

Conversation

erikjohnston
Copy link
Member

@erikjohnston erikjohnston commented Oct 30, 2019

This is a bit big as we needed to split out the purge functions to work on events and state in separate transactions.

(Split out from #6245.)

synapse/storage/data_stores/main/events.py Outdated Show resolved Hide resolved
synapse/storage/data_stores/main/events.py Show resolved Hide resolved
synapse/storage/data_stores/main/events.py Outdated Show resolved Hide resolved
synapse/storage/data_stores/main/events.py Outdated Show resolved Hide resolved
synapse/storage/data_stores/main/events.py Outdated Show resolved Hide resolved
synapse/storage/data_stores/main/events.py Outdated Show resolved Hide resolved
synapse/storage/data_stores/main/events.py Outdated Show resolved Hide resolved
synapse/storage/data_stores/main/events.py Outdated Show resolved Hide resolved
@erikjohnston erikjohnston requested a review from richvdh October 31, 2019 16:00
synapse/storage/data_stores/main/events.py Outdated Show resolved Hide resolved
synapse/storage/data_stores/main/events.py Outdated Show resolved Hide resolved
synapse/storage/data_stores/main/events.py Outdated Show resolved Hide resolved
synapse/storage/data_stores/main/state.py Outdated Show resolved Hide resolved
This does mean that we won't clean up orphaned state groups (i.e. state
groups that were persisted but the associated event wasn't).
# Now we fetch all the state groups that should be deleted.
txn.execute(
"""
SELECT DISTINCT state_group FROM events
Copy link
Member

Choose a reason for hiding this comment

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

erm, haven't we just deleted all the relevant rows from events?

Copy link
Member Author

Choose a reason for hiding this comment

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

Gah. And the tests were broken.

synapse/storage/data_stores/main/events.py Outdated Show resolved Hide resolved
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

^

And fix the tests to actually test that things got deleted.
@erikjohnston erikjohnston requested a review from richvdh November 6, 2019 17:03
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

well, it looks plausible now, but your CI is borked

@erikjohnston
Copy link
Member Author

Oh, Collection isn't available on 3.5?

@richvdh
Copy link
Member

richvdh commented Nov 6, 2019

it's in typing_extensions

@erikjohnston
Copy link
Member Author

Hmm, I don't see it in typing_extensions anywhere?

@richvdh
Copy link
Member

richvdh commented Nov 6, 2019

gah. seems you're right. Maybe we could define our own on 3.5. This is starting to get out of scope though.

@erikjohnston erikjohnston merged commit f713c01 into develop Nov 8, 2019
@erikjohnston
Copy link
Member Author

I've just moved it into the docstring for now until we figure out what we're doing :)

@erikjohnston erikjohnston deleted the erikj/split_purge_history branch January 9, 2020 15:47
babolivier pushed a commit that referenced this pull request Sep 1, 2021
* commit 'f713c01e2':
  Move type annotation into docstring
  Fix deleting state groups during room purge.
  Use correct type annotation
  Change to not require a state_groups.room_id index.
  Fix up comment
  Update log line to lie a little less
  Add state_groups.room_id index
  Docstrings
  Fix purge room API
  Newsfile
  Split purge API into events vs state
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