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

getting rid of the replication "stream" tables #13456

Open
richvdh opened this issue Aug 4, 2022 · 2 comments
Open

getting rid of the replication "stream" tables #13456

richvdh opened this issue Aug 4, 2022 · 2 comments
Labels
A-Database DB stuff like queries, migrations, new/remove columns, indexes, unexpected entries in the db T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. Z-Cleanup Things we want to get rid of, but aren't actively causing pain

Comments

@richvdh
Copy link
Member

richvdh commented Aug 4, 2022

Currently we have a number of tables in the database that exist only to record data for the replication streams. These include:

  • cache_invalidation_stream_by_instance
  • current_state_delta_stream
  • ex_outlier_stream
  • presence_stream
  • push_rules_stream

(and there may well be others).

Essentially, whenever we record a change to a table that needs to be replicated to workers, we add a row to one of these tables; the rows are then used in one of two ways:

  • ReplicationStreamer._run_notifier_loop regularly polls them (via the *Stream._update_function methods) and sends out a NOTIFY over Redis pubsub with the data from the table.
  • If a worker gets disconnected from Redis (so misses notifications), it can catch up with any missed notification by reading the relevant table itself.

The reason we use this arrangement is twofold:

  1. It allows workers which miss the memo (because they were disconnected from Redis) to catch up with anything they missed.
  2. Since the Redis notifications are sent out asynchronously by _run_notifier_loop, it is possible for the "writing" process to abort between updating the database and sending the notification to Redis. Persisting the data in postgres ensures that we can replay anything that wasn't sent when we restart.

However, I assert that these extra tables are a source of complexity, as well as increased database I/O and storage (not least because we never clear them out (#5888)). Worse, whenever we need to add a new type of replication stream, we have to add a load of extra paraphenalia in the shape of a new stream table. It would be good to consider how to get rid of them.

@reivilibre reivilibre added the T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. label Aug 4, 2022
@richvdh
Copy link
Member Author

richvdh commented Aug 4, 2022

We had some thoughts. Let's deal with the two usecases separately:

"Workers that miss the memo" could be dealt with by using Redis streams instead of pub/sub. (We'd need to kill off TCP replication (#11728).) Conceptually this moves the task of maintaining the "stream" table into Redis.

"Writing worker dies before notifying" could be dealt with by sending out the redis notification before committing the postgres transaction. This brings further problems, though - in particular, the receiving worker needs to know when it is actually safe to read the new data from the database.

We have two proposals for dealing with this new problem:

  • (h/t @squahtx): We could include the postgres txid_current() in the redis notification, and have the receiving process poll txid_status() until it completes. Interestingly this might also allow the worker to safely read from an asynchronous postgres replica.
  • Use Postgres locking. For example, have the writing worker take out a FOR UPDATE row lock, and the reading worker take a FOR SHARE lock. (That will work for updates, but not inserts. For inserts, provided you have an incrementing primary key, you can safely use the current mechanism of just polling the table asynchronously. Or you could use pg advisory locks)

@ArtObr
Copy link

ArtObr commented Sep 6, 2022

Hello there, @richvdh
So if my synapse app is running in monolith mode, I don't need any of the above tables?
Would it be safe to just delete all of the content in that case? (e.g. DELETE * FROM cache_invalidation_stream_by_instance)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Database DB stuff like queries, migrations, new/remove columns, indexes, unexpected entries in the db T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. Z-Cleanup Things we want to get rid of, but aren't actively causing pain
Projects
None yet
Development

No branches or pull requests

4 participants