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

Stop backpaginating when events not visible #4699

Merged
merged 9 commits into from
Mar 5, 2019

Conversation

erikjohnston
Copy link
Member

No description provided.

@erikjohnston erikjohnston requested a review from a team February 20, 2019 18:17
@codecov-io
Copy link

codecov-io commented Feb 20, 2019

Codecov Report

Merging #4699 into develop will decrease coverage by 0.68%.
The diff coverage is 86.48%.

@@            Coverage Diff             @@
##           develop   #4699      +/-   ##
==========================================
- Coverage    75.09%   74.4%   -0.69%     
==========================================
  Files          340     340              
  Lines        34923   35795     +872     
  Branches      5723    5982     +259     
==========================================
+ Hits         26225   26635     +410     
- Misses        7088    7518     +430     
- Partials      1610    1642      +32

synapse/handlers/federation.py Outdated Show resolved Hide resolved
synapse/handlers/federation.py Outdated Show resolved Hide resolved
synapse/handlers/federation.py Outdated Show resolved Hide resolved
@erikjohnston erikjohnston requested a review from richvdh February 27, 2019 13:06
synapse/storage/event_federation.py Outdated Show resolved Hide resolved
synapse/handlers/federation.py Show resolved Hide resolved
@erikjohnston erikjohnston force-pushed the erikj/stop_fed_not_in_room branch from 8da569d to 7356052 Compare March 4, 2019 14:43
When filtering events to send to server we check more than just history
visibility. However when deciding whether to backfill or not we only
care about the history visibility.
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.

are we not likely to end up doing the filter_events_for server thing (which is quite expensive) repeatedly, every time anyone asks for some backfill? Should we cache the results of this somehow? (possibly in a separate PR?)

generally seems sane to me.

synapse/visibility.py Outdated Show resolved Hide resolved
changelog.d/4699.bugfix Outdated Show resolved Hide resolved
synapse/handlers/federation.py Outdated Show resolved Hide resolved
synapse/handlers/federation.py Outdated Show resolved Hide resolved
# TODO: If we do do a backfill the we should filter the extremities to
# only include those that point to visible portions of history.
#
# TODO: Correctly handle the case where we are allowed to see the
Copy link
Member

Choose a reason for hiding this comment

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

how would we do this? is it a thing we are ever likely to be able to do? if not, a TODO is inappropriate

Copy link
Member Author

Choose a reason for hiding this comment

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

It should be possible, I've updated the comment with roughly what's needed.

synapse/handlers/federation.py Show resolved Hide resolved
@erikjohnston erikjohnston merged commit b050a10 into develop Mar 5, 2019
@erikjohnston erikjohnston deleted the erikj/stop_fed_not_in_room 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.

3 participants