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.

Sign up for GitHub

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

Jump to bottom

Never return negative bump stamp #17748

Merged
merged 2 commits into from
Sep 24, 2024
+17 −0
Merged

Never return negative bump stamp #17748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
52c9b81
Never return negative bump stamp
erikjohnston Sep 24, 2024
7e92fec
Newsfile
erikjohnston Sep 24, 2024
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/17748.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix bug in sliding sync where the server would incorrectly return a negative bump stamp, which caused Element X apps to stop syncing.
16 changes: 16 additions & 0 deletions synapse/handlers/sliding_sync/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1063,6 +1063,22 @@ async def get_room_sync_data(
if new_bump_stamp is not None:
bump_stamp = new_bump_stamp

if bump_stamp < 0:
# We never want to send down negative stream orderings, as you can't
# sensibly compare positive and negative stream orderings (they have
# different meanings).
#
# A negative bump stamp here can only happen if the stream ordering
# of the membership event is negative (and there are no further bump
# stamps), which can happen if the server leaves and deletes a room,
# and then rejoins it.
#
# To deal with this, we just set the bump stamp to zero, which will
# shove this room to the bottom of the list. This is OK as the
# moment a new message happens in the room it will get put into a
# sensible order again.
bump_stamp = 0
Comment on lines +1076 to +1080
Copy link
Member

Choose a reason for hiding this comment

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

"new message" or "new event"? Because if it's new events, the join event will anyway generate a new positive stream ordering? (I basically know nothing about all this, I'm trying to understand :D)

Copy link
Member Author

Choose a reason for hiding this comment

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

Technically its a new "bump event", which are things like messages, stickers, etc. The idea is that we only want to "bump" the room to the top of the room list (on the client) for events of interest, and not e.g. display name changes.

synapse/synapse/types/handlers/__init__.py

Lines 28 to 36 in 5173741

SLIDING_SYNC_DEFAULT_BUMP_EVENT_TYPES = {
EventTypes.Create,
EventTypes.Message,
EventTypes.Encrypted,
EventTypes.Sticker,
EventTypes.CallInvite,
EventTypes.PollStart,
EventTypes.LiveLocationShareStart,
}

the join event will anyway generate a new positive stream ordering?

Yeah, the membership events should always be created with a positive stream ordering. It's only if the room gets locally deleted and then rejoins the room that the local membership events would get persisted with a negative stream ordering.

Copy link

Choose a reason for hiding this comment

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

Given the nature of #community-moderation-effort-bl:neko.dev, it basically never receives any of the above events, so it is plausible that there is a huge backfill without any bump event.


unstable_expanded_timeline = False
prev_room_sync_config = previous_connection_state.room_configs.get(room_id)
# Record the `room_sync_config` if we're `ignore_timeline_bound` (which means
Expand Down
Loading