Added presence update on change of profile information and config flags for selective presence tracking #16992
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
Michael-Hollister
force-pushed
the
michael/presence-enhancements
branch
from
28e1f2f
to
c893e5a
Compare
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. |
| * `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 |
There was a problem hiding this comment.
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?
| * `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. |
There was a problem hiding this comment.
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
This option would disable that as well. Perhaps instead of "sync_presence_tracking", you could just call this option "client_presence_tracking"?
There was a problem hiding this comment.
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.
| # 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 |
There was a problem hiding this comment.
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.
|
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>
Michael-Hollister
force-pushed
the
michael/presence-enhancements
branch
from
9716d53
to
2428eb8
Compare
|
|
This PR address two issues:
m.presenceupdate on change of profile information (displaynameandavatar_url) per stated in the spec: https://spec.matrix.org/v1.9/client-server-api/#events-on-change-of-profile-informationpresenceis 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 thestatus_msg,displayname, andavatar_urlfields.Note that if
sync_presence_trackingand/orfederation_presence_trackingis disabled, client applications an still call the/presence/{userId}/statusendpoints 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.memberevent 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_trackingandfederation_presence_trackingare disabled. The setup is that of 500 users distributed among 2 federated servers in which users only call the/syncendpoint (no room updates or messages being sent during the test).With current default behavior (
presence,sync_presence_tracking, andfederation_presence_trackingare 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_trackingandfederation_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
EventStoretoEventWorkerStore.".code blocks.(run the linters)