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

Split out state storage into separate data store. #6245

Closed
wants to merge 4 commits into from

Conversation

@erikjohnston
Copy link
Member

erikjohnston commented Oct 24, 2019

This also includes splitting purge storage APIs into two, one that works on events and the other on state.

Based on #6240.

Commits independently reviewable.

@erikjohnston erikjohnston force-pushed the erikj/split_out_state_store branch from f5c309b to a03c7f9 Oct 24, 2019
@erikjohnston erikjohnston requested a review from matrix-org/synapse-core Oct 24, 2019
@erikjohnston erikjohnston added this to In progress in Homeserver Task Board via automation Oct 24, 2019
@erikjohnston erikjohnston moved this from In progress to Review in Homeserver Task Board Oct 25, 2019
Copy link
Member

richvdh left a comment

these multi-thousand-line diffs are a bit unreviewable. Is there any way to split them up so that code moves are separate from changes?

@@ -39,12 +40,15 @@ class Storage(object):
"""

def __init__(self, hs, stores: DataStores):
self.stores = stores

This comment has been minimized.

Copy link
@richvdh

richvdh Oct 29, 2019

Member

is this necessary?

# We include the main data store here mainly so that we don't have to
# rewrite all the existing code to split it into high vs low level
# interfaces.
self.main = stores.main

self.persistence = EventsPersistenceStore(hs, stores)
self.state = StateGroupStorage(hs, stores)

This comment has been minimized.

Copy link
@richvdh

richvdh Oct 29, 2019

Member

As per #6240 (comment): the inconsistency in naming between Storage and Store is... inconsistent.

@erikjohnston

This comment has been minimized.

Copy link
Member Author

erikjohnston commented Oct 30, 2019

Fair, I've tried to keep the massive diffs in separate commits, but let me see if I can rejig these a bit further

Currently we always point it to the same physical database.
@erikjohnston erikjohnston force-pushed the erikj/split_out_state_store branch from a03c7f9 to 2ddefcd Oct 30, 2019
@erikjohnston

This comment has been minimized.

Copy link
Member Author

erikjohnston commented Oct 30, 2019

Ok, I'm splitting this up some more into separate PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
2 participants
You can’t perform that action at this time.