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

Fix get_pdu asking every remote destination even after it finds an event #13346

Merged
merged 5 commits into from
Jul 27, 2022
Merged

Fix get_pdu asking every remote destination even after it finds an event #13346

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
58317e6
Fix `get_pdu` asking every destination even after it finds an event
MadLittleMods Jul 20, 2022
0929b16
Add changelog
MadLittleMods Jul 20, 2022
0881f2a
Describe as long-standing bug
MadLittleMods Jul 25, 2022
1b0cf9e
Update changelog.d/13346.misc
MadLittleMods Jul 25, 2022
97c0469
Explain that the bug was never hit
MadLittleMods Jul 26, 2022
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
1 change: 1 addition & 0 deletions changelog.d/13346.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix long-standing bugged logic which was never hit in `get_pdu` asking every remote destination even after it finds an event.
6 changes: 3 additions & 3 deletions synapse/federation/federation_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,9 +403,9 @@ async def get_pdu(
# Prime the cache
self._get_pdu_cache[event.event_id] = event

# FIXME: We should add a `break` here to avoid calling every
# destination after we already found a PDU (will follow-up
# in a separate PR)
# Now that we have an event, we can break out of this
# loop and stop asking other destinations.
break
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would it be easy to add a regression to test to this, or no?

I'm not sure and I don't want to invest in looking into it. It's a bug that was never expressed because we never call get_pdu with more than one destination.


except SynapseError as e:
logger.info(
Expand Down