Skip to content

our storage classes are hella confusing #11733

Description

@matrixbot

This issue has been migrated from #11733.


Store, StateGroupDataStore, DataStore, Storage, DatabasePool, Databases... 🤯

#8033 made some attempts to clean it up, but there's a lot more to do here.

There are basically three layers:

  1. a DatabasePool represents a connection to a physical database. Normally you have exactly one of these, but it's possible to set Synapse up to talk to two separate postgres instances (one of them gets used for state storage (state_groups_state, mainly), and the other for everything else), in which case you have two.

  2. a <foo>Store is a low-level thing containing SQL, and is linked to a single DatabasePool. We basically have two of these (state and main), again with the state/everything else split. state is always a StateGroupDataStore, but main can be any of (DataStore, AdminCmdSlavedStore, GenericWorkerSlavedStore) depending on the synapse app.

    PersistEventsStore seems to be a special case, and is a layer over the main store.

    Databases is a singleton which instantiates the DatabasePools and <foo>Stores, and holds references to them.

  3. a <foo>Storage is a higher-level thing which spans multiple <foo>Stores. Ideally it does not do any SQL itself, but delegates it all to the Stores. We have three of these (PurgeEventsStorage, StateGroupStorage, EventsPersistenceStorage). Note that a lot of non-storage code skips this layer and goes straight to the <foo>Stores.

    Storage itself instantiates and holds references to the <foo>Storages.

    Edit 2022/06/06: These are now known as <foo>StorageControllers, and Storage itself is now known as StorageControllers.


This is all very well, but I seem to be incapable of holding it in my head at once. This is really not helped by there being a lot of inconsistent naming. Examples:

  • Databases should surely be called DataStoreManager or similar. And HomeServer.get_datastores, which returns the Databases, should be renamed.
  • Likewise, Storage -> StorageManager. (superceded by Dummy issue #12913)
  • the various implementations of the main Store could do with being named as such, for consistency with StateGroupDataStore - eg MasterMainDataStore, WorkerMainDataStore.
  • HomeServer.get_datastore (which returns the main Store) exists only for backwards compatibility. We should replace it with calls to get_datastores().main for consistency with the StateGroupDataStore. Dummy issue #12031
  • All the million places that have a self.store should instead have a self._main_store.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions