element-hq / element-hq/synapse
our storage classes are hella confusing
- Dominant language
- Python
- Stars
- 4.6k
- Forks
- 600
- Avg merge
- 5d 22h
- Merged PRs (30d)
- 51
Description
This issue has been migrated from [#11733](https://github.com/matrix-org/synapse/issues/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 `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 `DatabasePool`s and `Stores`, and holds references to them.
3. a `Storage` is a higher-level thing which spans multiple `Stores`. Ideally it does not do any SQL itself, but delegates it all to the `Store`s. We have three of these (`PurgeEventsStorage`, `StateGroupStorage`, `EventsPersistenceStorage`). Note that a lot of non-storage code skips this layer and goes straight to the `Store`s.
`Storage` itself instantiates and holds references to the `Storage`s.
Edit 2022/06/06: These are now known as `StorageController`s, 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.
* [x] Likewise, `Storage` -> `StorageManager`. (superceded by #12913)
* [ ] the various implementations of the main `Store` could do with being named as such, for consistency with `StateGroupDataStore` - eg `MasterMainDataStore`, `WorkerMainDataStore`.
* [x] `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`. #12031
* [ ] All the million places that have a `self.store` should instead have a `self._main_store`.
Contributor guide
Assessment
This issue has not been assessed yet.