Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Batch cache invalidation over replication #4671

Merged
merged 5 commits into from Feb 19, 2019

Conversation

Projects
None yet
3 participants
@erikjohnston
Copy link
Member

erikjohnston commented Feb 18, 2019

Currently whenever the current state changes in a room invalidate a lot
of caches, which cause a lot of traffic over replication. Instead,
lets batch up all those invalidations and send a single poke down
the replication streams.

Hopefully this will reduce load on the master process by substantially
reducing traffic.

erikjohnston added some commits Feb 18, 2019

Batch cache invalidation over replication
Currently whenever the current state changes in a room invalidate a lot
of caches, which cause *a lot* of traffic over replication. Instead,
lets batch up all those invalidations and send a single poke down
the replication streams.

Hopefully this will reduce load on the master process by substantially
reducing traffic.

@erikjohnston erikjohnston requested a review from matrix-org/synapse-core Feb 18, 2019

@erikjohnston

This comment has been minimized.

Copy link
Member Author

erikjohnston commented Feb 18, 2019

This passes the sytests worker config locally (and breaks when I break the cache invalidation)

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Feb 18, 2019

Codecov Report

Merging #4671 into develop will increase coverage by <.01%.
The diff coverage is 82.14%.

@@             Coverage Diff             @@
##           develop    #4671      +/-   ##
===========================================
+ Coverage    75.17%   75.18%   +<.01%     
===========================================
  Files          340      340              
  Lines        34712    34736      +24     
  Branches      5677     5680       +3     
===========================================
+ Hits         26096    26116      +20     
- Misses        7013     7015       +2     
- Partials      1603     1605       +2
@richvdh
Copy link
Member

richvdh left a comment

lgtm apart from some quibbling

# We probably haven't pulled in the cache in this worker,
# which is fine.
pass
if row.cache_func == _CURRENT_STATE_CACHE_NAME:

This comment has been minimized.

@richvdh

richvdh Feb 19, 2019

Member

I'd like to see some words about this (and in general what the 'caches' stream means) documented in docs/tcp_replication.tcp.

This comment has been minimized.

@erikjohnston

erikjohnston Feb 19, 2019

Author Member

I've added some words. I haven't specified exactly what _CURRENT_STATE_CACHE_NAME means, as I feel like that's just going to quickly get out of date with the code and is probably unnecessary.

Show resolved Hide resolved synapse/storage/_base.py Outdated
Show resolved Hide resolved synapse/storage/_base.py Outdated
Show resolved Hide resolved synapse/storage/_base.py Outdated
Show resolved Hide resolved synapse/storage/_base.py Outdated
Show resolved Hide resolved synapse/storage/_base.py Outdated

@erikjohnston erikjohnston requested a review from richvdh Feb 19, 2019

@richvdh
Copy link
Member

richvdh left a comment

lgtm otherwise


However, there are times when a number of caches need to be invalidated at the
same time with the same key. To reduce traffic we batch those invalidations into
a single poke by defining a special cache name that workers understand to mean

This comment has been minimized.

@richvdh

richvdh Feb 19, 2019

Member

I don't mind not listing the exact caches, but please can you define the special cache name?

Show resolved Hide resolved synapse/storage/_base.py Outdated
Show resolved Hide resolved synapse/storage/_base.py Outdated

@erikjohnston erikjohnston merged commit c003450 into develop Feb 19, 2019

7 checks passed

ci/circleci: sytestpy2merged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy2postgresmerged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy3merged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy3postgresmerged Your tests passed on CircleCI!
Details
codecov/patch 82.14% of diff hit (target 0%)
Details
codecov/project 75.18% (target 0%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@erikjohnston erikjohnston deleted the erikj/state_cache_invalidation branch Mar 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.