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

Limit cache invalidation replication line length #4748

Merged
merged 4 commits into from Feb 27, 2019

Conversation

Projects
None yet
2 participants
@erikjohnston
Copy link
Member

erikjohnston commented Feb 26, 2019

This fixes a bug where replication completely wedges if the server tries to send a cache invalidation that serialises to a line that is longer than the max line length.

Introduced in #4671

@erikjohnston erikjohnston marked this pull request as ready for review Feb 26, 2019

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

@richvdh
Copy link
Member

richvdh left a comment

Fixes

# We need to be careful that the size of the `members_changed` list
# isn't so large that it causes problems sending over replication, so we
# send them in chunks.
members_changed = list(members_changed)

This comment has been minimized.

@richvdh

richvdh Feb 26, 2019

Member

I think you want synapse.util.batch_iter here.

# isn't so large that it causes problems sending over replication, so we
# send them in chunks.
members_changed = list(members_changed)
for i in range(0, len(members_changed), 100):

This comment has been minimized.

@richvdh

richvdh Feb 26, 2019

Member

given that mxids have a maxlen of 255 chars, we can still easily overflow the max line length of 16K

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

@codecov

This comment has been minimized.

Copy link

codecov bot commented Feb 27, 2019

Codecov Report

Merging #4748 into develop will decrease coverage by <.01%.
The diff coverage is 70%.

@@             Coverage Diff             @@
##           develop    #4748      +/-   ##
===========================================
- Coverage    75.12%   75.11%   -0.01%     
===========================================
  Files          340      340              
  Lines        34860    34867       +7     
  Branches      5711     5713       +2     
===========================================
+ Hits         26188    26190       +2     
- Misses        7058     7062       +4     
- Partials      1614     1615       +1
@richvdh
Copy link
Member

richvdh left a comment

TOTES FINE

@richvdh

This comment has been minimized.

Copy link
Member

richvdh commented Feb 27, 2019

Hopefully fixes #4733

@richvdh richvdh merged commit 6bb1c02 into develop Feb 27, 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 70% of diff hit (target 0%)
Details
codecov/project 75.11% (target 0%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
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.