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

Support any process writing to cache invalidation stream. - #7436

Merged
erikjohnston merged 9 commits into
developfrom
erikj/split_cache_stream
May 7, 2020
Merged

erikjohnston merged 9 commits into
developfrom
erikj/split_cache_stream

Conversation

@erikjohnston

@erikjohnston erikjohnston commented May 6, 2020 •

Copy link
Copy Markdown
Member

This is done via two big changes:

  1. Change the cache stream to use the new MultiWriterIdGenerator
  2. Change the definition of Stream.current_token to accept an instance name parameter, which allows us to support the idea of there being multiple writers and so multiple positions to keep track of.

Its probably worth going through commit by commit, as there is a bit of boiler plate refactoring going on.

@erikjohnston erikjohnston changed the title Erikj/split cache stream Support any process writing to cache invalidation stream. May 6, 2020
@erikjohnston
erikjohnston force-pushed the erikj/split_cache_stream branch 2 times, most recently from af43047 to 9eef604 Compare May 6, 2020 11:55
@erikjohnston
erikjohnston force-pushed the erikj/split_cache_stream branch from 9eef604 to 5779503 Compare May 6, 2020 12:24
@erikjohnston
erikjohnston requested a review from a team May 6, 2020 13:49
@clokep

clokep commented May 6, 2020

Copy link
Copy Markdown
Member

@erikjohnston Looks like this has a conflict!

@erikjohnston

Copy link
Copy Markdown
Member Author

@erikjohnston Looks like this has a conflict!

But it didn't just now 😭

Comment thread synapse/storage/data_stores/main/schema/delta/58/cache_instance.sql.postgres Outdated

DROP TABLE IF EXISTS cache_invalidation_stream;

CREATE TABLE cache_invalidation_stream (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't know if we care, but re-using this table name means it will be hard to downgrade.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point, I've renamed it and truncated the old table.

@clokep clokep left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this looks OK, modulo the couple of comments I left.

@erikjohnston
erikjohnston force-pushed the erikj/split_cache_stream branch from 493ef66 to f3e5d26 Compare May 7, 2020 10:55
@erikjohnston
erikjohnston merged commit d7983b6 into develop May 7, 2020
@erikjohnston
erikjohnston deleted the erikj/split_cache_stream branch May 7, 2020 12:51
phil-flex pushed a commit to phil-flex/synapse that referenced this pull request Jun 16, 2020
DMRobertson pushed a commit that referenced this pull request Nov 8, 2023
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