fix: Walk auth events once instead of re-queueing parents #2276
No reviewers
Labels
No labels
Abandoned
Blocked
Bug
Changelog
Added
Changelog
Missing
Changelog
None
Cherry-picking
Database
Dependencies
Dependencies/Renovate
Difficulty
Easy
Difficulty
Hard
Difficulty
Medium
Documentation
Enhancement
Good first issue
Help wanted
Inherited
Matrix/Administration
Matrix/Appservices
Matrix/Auth
Matrix/Client
Matrix/Core
Matrix/E2EE
Matrix/Federation
Matrix/Hydra
Matrix/MSC
Matrix/Media
Matrix/T&S
Merge
Merge/Manual
Merge/Squash
Meta
Meta/CI
Meta/Packaging
Priority
Blocking
Priority
High
Priority
Low
Security
Status
Confirmed
Status
Duplicate
Status
Invalid
Status
Needs Investigation
Support
bob the builder
No milestone
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
continuwuation/continuwuity!2276
Loading…
Reference in a new issue
No description provided.
Delete branch "eleboucher/continuwuity:fix/auth-event-walk-exponential"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The auth event walk re-queued an event together with its missing auth events, without checking if they were already queued. Each pass roughly doubles the queue, so a deep auth chain never finishes: it spams debug logs and eats RAM until you restart.
Now it just walks auth_events breadth-first and fetches each event once. Ordering is already done by build_local_dag afterwards.
Pull request checklist:
mainbranch, and the branch is named something other thanmain.myself, if applicable. This includes ensuring code compiles.
@ -640,0 +567,4 @@// Queue this event's auth events. They are discovered by// this same loop, and the whole result is topologically sorted// below, so there's no need to hold this event back until they// arrive.I don't know, the point of pushing it to the back of the queue again is that it can just not be added to the discovered events map if its auth events are unfetchable (e.g. missing or unusable), I feel like this might leave us with a partial auth chain?
I suppose that might not be too bad when
handle_outlier_pdufetches the entire auth chain via the/event_auth/federation endpoint if any of the auth events are missing. The important thing is all of the apex events can be fetched, since the PDU upgrade task uses this function to resolve the state at the incoming event after callingstate_ids, and obviously we need a full state to be able to calculate the changesOnly when an ancestor is malformed: a missing auth event still fails the whole call, and a dependent of a malformed one now fails in handle_outlier_pdu after one extra /event_auth request instead of being dropped early, so it ends up in the same place and apex fetching is unchanged.
Ah yeah that makes sense. There's a bit of a performance hit due to the expensive fallback to event_auth but the server is already in a Messed Up Situation™️ if it gets to that point anyway, so that's probably fine
8d478d8b6a6f305e7a1d6f305e7a1d8578f84af58578f84af5cee69b4c8d