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

Sliding Sync: Lazy-loading room members on incremental sync (remember memberships) #17809

Conversation

MadLittleMods
Copy link
Contributor

@MadLittleMods MadLittleMods commented Oct 9, 2024

Lazy-loading room members on incremental sync and remember which memberships we've sent down the connection before (up-to 100)

Fix #17804

Follow-up/alternative to #17806

Depends on #17785

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Code style is correct
    (run the linters)

# sent it before and send the new state. (if we were tracking
# that we sent any other state, we should still keep track
# that).
{},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's possible that we could remember the specific state_keys that we have sent down before but this currently just acts the same as if a whole type was removed (same as simple_remove_type)

This is just a performance optimization though and the result would look like following:

Suggested change
{},
{
EventTypes.Member: {
"@user3:test",
}
},

Perhaps it's good that we "garbage collect" and forget what we've sent before for a given type when the client stops caring about a certain type 🤷.

@MadLittleMods MadLittleMods marked this pull request as ready for review October 9, 2024 22:07
@MadLittleMods MadLittleMods requested a review from a team as a code owner October 9, 2024 22:07
Copy link
Member

@erikjohnston erikjohnston left a comment

Choose a reason for hiding this comment

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

I think this makes sense! I am slightly worried about the increase in the size of the tables if we keep adding more and more members in there, but I'll have a little think about how we might make that more efficient.

synapse/handlers/sliding_sync/__init__.py Show resolved Hide resolved
…c-lazy-load-members-on-incrental-sync3

Conflicts:
	synapse/handlers/sliding_sync/__init__.py
Base automatically changed from erikj/ss_required_state to develop October 14, 2024 12:31
…ers-on-incrental-sync3

Conflicts:
	synapse/handlers/sliding_sync/__init__.py
	tests/handlers/test_sliding_sync.py
Copy link
Member

@erikjohnston erikjohnston left a comment

Choose a reason for hiding this comment

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

Thanks!

@MadLittleMods MadLittleMods merged commit 0932c77 into develop Nov 4, 2024
39 checks passed
@MadLittleMods MadLittleMods deleted the madlittlemods/sliding-sync-lazy-load-members-on-incrental-sync3 branch November 4, 2024 16:18
@MadLittleMods
Copy link
Contributor Author

Woot! Thanks for the review @erikjohnston 🦎

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.

Sliding sync does not correctly do lazy loaded members for incremental sync
2 participants