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

Bulk-invalidate e2e cached queries after claiming keys #16613

Merged
merged 15 commits into from
Nov 9, 2023
Merged

Bulk-invalidate e2e cached queries after claiming keys #16613

Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
190b50a
Remove stale comment
Nov 8, 2023
b1ca34f
simple_insert_many's keys should be a sequence
Nov 8, 2023
171d705
Impl bulk-invalidating caches over replication
Nov 8, 2023
c6f2a8f
Add get_next_mult_txn.
clokep Nov 8, 2023
307c77a
Mark multiple IDs as finished.
clokep Nov 8, 2023
d9b7d08
Helpful implementation note!!
Nov 8, 2023
f6ee91c
dummy test case
Nov 8, 2023
9e72bf4
Commit to them being tuples
Nov 8, 2023
be349f2
Changelog
Nov 8, 2023
91a48af
lint
Nov 8, 2023
35938df
Fixup 307c77abead5210518622100c5e9a4c24a9a11e5
Nov 8, 2023
69b539e
Remove guff from the non-worker test
Nov 8, 2023
4899623
Fix var name
Nov 8, 2023
e3a6358
Use set.update instead of a loop over set.add
Nov 8, 2023
b95d361
Tweak changelog
Nov 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Remove stale comment 
Missed in #7436
  • Loading branch information
David Robertson committed Nov 8, 2023
commit 190b50ad6a7ab04b3d566ae24a78ae7070acdd9d
4 changes: 0 additions & 4 deletions synapse/storage/databases/main/cache.py
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

Should we update the non-bulk versions of these to actually require tuples?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was wondering if we should actually be requiring List[str], since the keys column is []text. It'd save a conversion tuple->list here and there? 🤷

Copy link
Member

Choose a reason for hiding this comment

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

I think tuples are used though because they're immutable so is what is used in the cache keys?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahh, that makes sense!

I'd be in favour of specifying Tuple[str, ...], but in another PR.

Copy link
Member

Choose a reason for hiding this comment

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

Do they have to be strings? I'd be kind of surprised if we don't have other types in there?

Anyway, yes sounds like a different PR.

Original file line number Diff line number Diff line change
Expand Up @@ -564,10 +564,6 @@ def _send_invalidation_to_replication(
if isinstance(self.database_engine, PostgresEngine):
assert self._cache_id_gen is not None

# get_next() returns a context manager which is designed to wrap
# the transaction. However, we want to only get an ID when we want
# to use it, here, so we need to call __enter__ manually, and have
# __exit__ called after the transaction finishes.
stream_id = self._cache_id_gen.get_next_txn(txn)
txn.call_after(self.hs.get_notifier().on_new_replication_data)

Expand Down