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

Limit cache invalidation replication line length #4748

Merged
merged 4 commits into from
Feb 27, 2019

Conversation

erikjohnston
Copy link
Member

@erikjohnston 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 February 26, 2019 15:00
@erikjohnston erikjohnston requested a review from a team February 26, 2019 15:01
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

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)
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 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):
Copy link
Member

Choose a reason for hiding this comment

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

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

@codecov
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

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

TOTES FINE

@richvdh
Copy link
Member

richvdh commented Feb 27, 2019

Hopefully fixes #4733

@richvdh richvdh merged commit 6bb1c02 into develop Feb 27, 2019
@erikjohnston erikjohnston deleted the erikj/limit_cache_invalidation_size branch January 9, 2020 15:52
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