Added presence update on change of profile information and config flags for selective presence tracking by Michael-Hollister · Pull Request #16992 · element-hq/synapse

Michael-Hollister · 2024-03-11T04:30:35Z

This PR address two issues:

We now send an m.presence update on change of profile information (displayname and avatar_url) per stated in the spec: https://spec.matrix.org/v1.9/client-server-api/#events-on-change-of-profile-information
Since presence updates now has more utility beyond representing a user's online/offline status, I have added config flags to selectively enable/disable server-side tracking of users activity. The following config options have been added and take effect when presence is enabled:
a. sync_presence_tracking (Default enabled): Determines if the server tracks a user's presence activity when syncing. If disabled, the server will not automatically update the user's presence activity when the sync endpoint is called.
b. federation_presence_tracking (Default enabled): Determines if the server will accept presence EDUs that only contain presence activity updates. If disabled, the server will drop processing EDUs that do not contain updates to the status_msg, displayname, and avatar_url fields.

Note that if sync_presence_tracking and/or federation_presence_tracking is disabled, client applications an still call the /presence/{userId}/status endpoints to update the user's presence.

The motivation for adding the config options is to improve server performance when applications are using presence to track user profile updates, but not user activity status. Combining this along with MSC4069 for disabling profile m.room.member event propagation results in more efficient profile update process for client applications to use.

Below are some graphs from loadtests that show performance improvements when sync_presence_tracking and federation_presence_tracking are disabled. The setup is that of 500 users distributed among 2 federated servers in which users only call the /sync endpoint (no room updates or messages being sent during the test).

With current default behavior (presence, sync_presence_tracking, and federation_presence_tracking are enabled)

Server 1: (workers enabled, 1 master worker and 1 presence writer)

Server 2: (workers disabled)

We see that there are CPU usage spikes of up to 50% when processing presence events. When disabling sync_presence_tracking and federation_presence_tracking, we eliminate CPU usage spikes when processing presence updates from user's sync activity:

Server 1: (workers enabled, 1 master worker and 1 presence writer)

Server 2: (workers disabled)

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)

Michael-Hollister · 2024-05-02T21:02:50Z

I get linting errors for some reason. For some reason the linter thinks there are missing fields when constructing UserPresenceState objects? There are 9 values passed in from what I can see...

synapse/storage/databases/main/presence.py:300: error: Need more than 7 values to unpack (9 expected)  [misc]
synapse/storage/databases/main/presence.py:301: error: Cannot determine type of "user_id"  [has-type]
synapse/storage/databases/main/presence.py:302: error: Cannot determine type of "user_id"  [has-type]
synapse/storage/databases/main/presence.py:303: error: Cannot determine type of "state"  [has-type]
synapse/storage/databases/main/presence.py:304: error: Cannot determine type of "last_active_ts"  [has-type]
synapse/storage/databases/main/presence.py:305: error: Cannot determine type of "last_federation_update_ts"  [has-type]
synapse/storage/databases/main/presence.py:306: error: Cannot determine type of "last_user_sync_ts"  [has-type]
synapse/storage/databases/main/presence.py:307: error: Cannot determine type of "status_msg"  [has-type]
synapse/storage/databases/main/presence.py:308: error: Cannot determine type of "currently_active"  [has-type]
synapse/storage/databases/main/presence.py:309: error: Cannot determine type of "displayname"  [has-type]
synapse/storage/databases/main/presence.py:310: error: Cannot determine type of "avatar_url"  [has-type]
synapse/storage/databases/main/presence.py:446: error: Need more than 7 values to unpack (9 expected)  [misc]
synapse/storage/databases/main/presence.py:457: error: Cannot determine type of "user_id"  [has-type]
synapse/storage/databases/main/presence.py:458: error: Cannot determine type of "user_id"  [has-type]
synapse/storage/databases/main/presence.py:459: error: Cannot determine type of "state"  [has-type]
synapse/storage/databases/main/presence.py:460: error: Cannot determine type of "last_active_ts"  [has-type]
synapse/storage/databases/main/presence.py:461: error: Cannot determine type of "last_federation_update_ts"  [has-type]
synapse/storage/databases/main/presence.py:462: error: Cannot determine type of "last_user_sync_ts"  [has-type]
synapse/storage/databases/main/presence.py:463: error: Cannot determine type of "status_msg"  [has-type]
synapse/storage/databases/main/presence.py:464: error: Cannot determine type of "currently_active"  [has-type]
synapse/storage/databases/main/presence.py:465: error: Cannot determine type of "displayname"  [has-type]
synapse/storage/databases/main/presence.py:466: error: Cannot determine type of "avatar_url"  [has-type]
Found 22 errors in 1 file (checked 870 source files)

I fixed the linter issues I was experiencing and also added some documentation for the config flags. Open to feedback to see if these PR changes would be of interest for merging.

anoadragon453

Thanks for the detailed PR description! This looks good on the whole, though I have some quibbles about the wording.

anoadragon453 · 2024-05-30T13:12:04Z

+* `federation_presence_tracking` (Default enabled): Determines if the server will accept
+presence EDUs that only contain presence activity updates. If disabled, the server will drop
+processing EDUs that do not contain updates to the `status_msg`, `displayname`, and


I find this a bit confusing, as the name of this option implies that "presence" equates to only the online/offline/etc. state of the user, and not the transport which carries activity and profile updates. Whereas if you set presence.enabled: false, then both activity and profile updates are ignored.

Could you clarify the difference in your description of these options?

anoadragon453 · 2024-05-30T13:19:42Z

+* `sync_presence_tracking` (Default enabled): Determines if the server tracks a user's presence
+activity when syncing. If disabled, the server will not automatically update the user's presence
+activity when the sync endpoint is called. Note that client applications can still update their
+presence by calling the respective presence endpoints.


Note that Synapse will also count a user as "online" if they call /events:

synapse/synapse/handlers/events.py

Lines 54 to 80 in b548f78

async def get_stream(

self,

requester: Requester,

pagin_config: PaginationConfig,

timeout: int = 0,

as_client_event: bool = True,

affect_presence: bool = True,

room_id: Optional[str] = None,

) -> JsonDict:

"""Fetches the events stream for a given user."""

if room_id:

blocked = await self.store.is_room_blocked(room_id)

if blocked:

raise SynapseError(403, "This room has been blocked on this server")

# send any outstanding server notices to the user.

await self._server_notices_sender.on_user_syncing(requester.user.to_string())

presence_handler = self.hs.get_presence_handler()

context = await presence_handler.user_syncing(

requester.user.to_string(),

requester.device_id,

affect_presence=affect_presence,

presence_state=PresenceState.ONLINE,

)

This option would disable that as well. Perhaps instead of "sync_presence_tracking", you could just call this option "client_presence_tracking"?

I decided to use the terms local_activity_tracking and remote_activity_tracking for these presence config options since the previous paragraph in the docs used similar language:

Presence tracking allows users to see the state (e.g online/offline)
of other local and remote users. ...

I also like client_presence_tracking though, and if you don't prefer the new names I used, I don't mind changing it to client_presence_tracking and federation_client_presence_tracking respectively.

anoadragon453 · 2024-05-30T13:23:40Z

+                # Process only profile presence updates to reduce resource impact
+                if "status_msg" in e or "displayname" in e or "avatar_url" in e:
+                    filtered_edus.append(e)
+            content["push"] = filtered_edus


Could you add a docstring to on_edu mentioning that it potentially modifies the passed content dictionary?

Looking at the calling functions, this isn't a problem. But it's good to call out for future reference.

Michael-Hollister · 2024-05-31T04:02:50Z

Thanks for the feedback! I've applied the suggestions. Feel free to resolve the remaining conversations if the changes are satisfactory.

…17231) This is to address an issue in which `m.presence` results on initial sync are not returning entries of users who are currently offline. The original behaviour was from #1535 This change is useful for applications that use the presence system for tracking user profile information/updates (e.g. #16992 or for profile status messages). This is gated behind a new configuration option to avoid performance impact for applications that don't need this, as a pragmatic solution for now.

…gs for selective presence tracking Signed-off-by: Michael Hollister <michael@futo.org>

Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>

CLAassistant · 2024-10-14T10:36:17Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

Michael-Hollister requested a review from a team as a code owner March 11, 2024 04:30

This comment was marked as resolved.

Sign in to view

Michael-Hollister force-pushed the michael/presence-enhancements branch from 28e1f2f to c893e5a Compare May 1, 2024 18:38

github-actions Bot deployed to PR Documentation Preview May 3, 2024 08:24 Active

Michael-Hollister mentioned this pull request May 24, 2024

Fixed presence results not returning offline users on initial sync #17231

Merged

3 tasks

anoadragon453 requested changes May 30, 2024

View reviewed changes

Michael-Hollister and others added 8 commits July 29, 2024 12:06

Added presence update on change of profile information and config fla…

418b936

…gs for selective presence tracking Signed-off-by: Michael Hollister <michael@futo.org>

Added missing type hints to DB queries

0ff515d

Added documentation for configuring new presence tracking options

f1e6483

Removed duplicate license comment block

7b73476

Improved wording of presence tracking documentation

e5aac2a

Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>

Improved wording of documentation and config option naming

d6d113e

Updated changelog description

e06395e

Docs grammar fix

2428eb8

Michael-Hollister force-pushed the michael/presence-enhancements branch from 9716d53 to 2428eb8 Compare July 29, 2024 17:33

github-actions Bot deployed to PR Documentation Preview July 29, 2024 17:34 Active

Michael-Hollister mentioned this pull request Jul 29, 2024

Merge upstream 7-29-24 circles-project/synapse#17

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added presence update on change of profile information and config flags for selective presence tracking#16992

Added presence update on change of profile information and config flags for selective presence tracking#16992
Michael-Hollister wants to merge 8 commits intoelement-hq:developfrom
Michael-Hollister:michael/presence-enhancements

Michael-Hollister commented Mar 11, 2024 •

edited

Loading

Uh oh!

This comment was marked as resolved.

Michael-Hollister commented May 2, 2024

Uh oh!

anoadragon453 left a comment

Uh oh!

anoadragon453 May 30, 2024

Uh oh!

Uh oh!

Uh oh!

anoadragon453 May 30, 2024

Uh oh!

Michael-Hollister May 31, 2024

Uh oh!

anoadragon453 May 30, 2024

Uh oh!

Uh oh!

Uh oh!

Michael-Hollister commented May 31, 2024

Uh oh!

CLAassistant commented Oct 14, 2024 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	async def get_stream(
	self,
	requester: Requester,
	pagin_config: PaginationConfig,
	timeout: int = 0,
	as_client_event: bool = True,
	affect_presence: bool = True,
	room_id: Optional[str] = None,
	) -> JsonDict:
	"""Fetches the events stream for a given user."""

	if room_id:
	blocked = await self.store.is_room_blocked(room_id)
	if blocked:
	raise SynapseError(403, "This room has been blocked on this server")

	# send any outstanding server notices to the user.
	await self._server_notices_sender.on_user_syncing(requester.user.to_string())

	presence_handler = self.hs.get_presence_handler()

	context = await presence_handler.user_syncing(
	requester.user.to_string(),
	requester.device_id,
	affect_presence=affect_presence,
	presence_state=PresenceState.ONLINE,
	)

Conversation

Michael-Hollister commented Mar 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Checklist

Uh oh!

This comment was marked as resolved.

Michael-Hollister commented May 2, 2024

Uh oh!

anoadragon453 left a comment

Choose a reason for hiding this comment

Uh oh!

anoadragon453 May 30, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

anoadragon453 May 30, 2024

Choose a reason for hiding this comment

Uh oh!

Michael-Hollister May 31, 2024

Choose a reason for hiding this comment

Uh oh!

anoadragon453 May 30, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Michael-Hollister commented May 31, 2024

Uh oh!

CLAassistant commented Oct 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Michael-Hollister commented Mar 11, 2024 •

edited

Loading

CLAassistant commented Oct 14, 2024 •

edited

Loading