Sliding sync: various fixups to the sliding sync joined room background job #17673
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
erikjohnston
changed the title
This has two advantages: the first is we only read rooms that the server is joined to. The second is that we can end up locked for ages waiting for various delete room jobs that are running (which lock the rooms rows).
We somehow have a room where the only non-outlier event has a negative stream ordering. It is unclear how this happened.
erikjohnston
force-pushed
the
erikj/ss_fix_bg_update_joined
branch
from
34d201f
to
d60e55b
Compare
Comment on lines
-1725
to
-1728
| assert most_recent_event_stream_ordering > 0, ( | ||
| "We should have at-least one event in the room (our own join membership event for example) " | ||
| + "that isn't backfilled (negative `stream_ordering`) if we are joined to the room." | ||
| ) |
There was a problem hiding this comment.
I guess there is a room where this is possible? We should add a comment in this asserts place that ideally we could assert this but there is actually some case where it happens.
# If we've just joined a remote room, then the last bump event may
# have been backfilled (and so have a negative stream ordering).
# These negative stream orderings can't sensibly be compared, but TODO
We need some value since the event_stream_ordering is a NOT NULL column. Negative works but we avoid doing it in other places since it would send the room to the bottom of the list on the client.
Comment on lines
+1602
to
+1603
| SELECT DISTINCT room_id FROM local_current_membership | ||
| WHERE membership = 'join' |
There was a problem hiding this comment.
I assume you've tested that this is fast enough?
There was a problem hiding this comment.
It's not ideal, but it completes in a few minutes on matrix.org, so should be fast enough everywhere else.
MadLittleMods
approved these changes
Sep 9, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow-up to #17652, #17641, #17634, #17631 and #17632 to fix-up #17512