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

Fix background update for sliding sync (find previous membership) (v1) #17631

Merged
merged 2 commits into from
Aug 29, 2024

Conversation

erikjohnston
Copy link
Member

This reverts commit ab414f2.

Introduced in #17599

@erikjohnston erikjohnston marked this pull request as ready for review August 29, 2024 15:51
@erikjohnston erikjohnston requested a review from a team as a code owner August 29, 2024 15:51
txn.execute(
"""
SELECT m.event_id, m.membership
FROM event_auth AS a
INNER JOIN room_memberships AS m ON (a.auth_id = m.event_id)
Copy link
Contributor

@MadLittleMods MadLittleMods Aug 29, 2024

Choose a reason for hiding this comment

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

For context on why this doesn't work, @erikjohnston ran this on matrix.org and found a leave membership where the event_auth only has two events that we don't have at all. The low number of event_auth is suspicious and really strange that we don't have the events at all. Even checking the PDU event JSON of the leave event shows it having two auth events.

According to the room_memberships table, they were previously invite

Another strange thing is that the room_memberships.event_id of the invite event doesn't match one of the event_auth of the leave event (the leave event doesn't point back to the invite).

@erikjohnston erikjohnston merged commit bb80894 into develop Aug 29, 2024
37 checks passed
@erikjohnston erikjohnston deleted the erikj/ss_fix_bg_update branch August 29, 2024 15:58
WHERE
room_id = ?
AND user_id = ?
AND event_stream_ordering < ?
Copy link
Contributor

@MadLittleMods MadLittleMods Aug 29, 2024

Choose a reason for hiding this comment

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

@erikjohnston found another snag down the line:

Turns out room_memberships.event_stream_ordering can be null here.

We need to rely on the events.stream_ordering and do a INNER JOIN events AS e USING (room_id, event_id)

Even though the events.stream_ordering also doesn't have a NOT NULL constraint, it doesn't have any rows where this is the case (SELECT event_id FROM events WHERE stream_ordering = null;). The fact it is a nullable column is a holdover from a rename of the column.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed in #17632

@MadLittleMods MadLittleMods changed the title Fix background update for sliding sync Fix background update for sliding sync (find previous membership) Aug 29, 2024
@MadLittleMods MadLittleMods changed the title Fix background update for sliding sync (find previous membership) Fix background update for sliding sync (find previous membership) (v1) Aug 29, 2024
erikjohnston added a commit that referenced this pull request Aug 30, 2024
Follow-up to #17631 and
#17632 to fix-up
#17599

---------

Co-authored-by: Eric Eastwood <eric.eastwood@beta.gouv.fr>
erikjohnston added a commit that referenced this pull request Sep 5, 2024
erikjohnston added a commit that referenced this pull request Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants