Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Rewrite presence for performance. #582

Merged
merged 17 commits into from Feb 19, 2016

Conversation

Projects
None yet
2 participants
Owner

erikjohnston commented Feb 17, 2016

  • Removes profile information from m.presence
  • The server idles out users.
  • Sending read receipts and typing bumps last active time.
  • While a user is online and their last_active_ago time remains below 1 minute, don't send updates to client. This is done by including a currently_active boolean field that when true indicates that clients should assume the user is only and active until further notice (i.e., their "last active" should be "now"). Clients will receive a new presence event when users stop being active.
  • Timeout non-offline states for remote users after 30mins
  • Persist the presence state across restarts

TODO:

  • Upload sytest branch
  • Fix unit tests.

@erikjohnston erikjohnston referenced this pull request in matrix-org/sytest Feb 18, 2016

Merged

Fix tests for new presence implementation. #187

@NegativeMjark NegativeMjark commented on an outdated diff Feb 18, 2016

synapse/util/wheel_timer.py
+ self.entries = []
+ self.current_tick = 0
+
+ def insert(self, now, obj, then):
+ """Inserts object into timer.
+
+ Args:
+ now (int): Current time in msec
+ obj (object): Object to be inserted
+ then (int): When to return the object strictly after.
+ """
+ then_key = int(then / self.bucket_size) + 1
+ for entry in self.entries:
+ # Add to first bucket we find. This should gracefully handle inserts
+ # for times in the past.
+ if entry.end_key >= then_key:
@NegativeMjark

NegativeMjark Feb 18, 2016

Contributor

If there are no "gaps" in the in the entries list, then why do we need to scan to find the appropriate entry?
Could we simply index into the entry list instead?

@NegativeMjark NegativeMjark commented on an outdated diff Feb 18, 2016

synapse/storage/util/id_generators.py
"""Returns the maximum stream id such that all stream ids less than or
equal to it have been successfully persisted.
+
+ Used to take a DataStore param, which is no longer needed.
@NegativeMjark

NegativeMjark Feb 18, 2016

Contributor

Could we fix up the call sites rather than adding a *args?

@NegativeMjark NegativeMjark commented on the diff Feb 18, 2016

synapse/storage/__init__.py
@@ -174,6 +197,27 @@ def _get_cache_dict(self, db_conn, table, entity_column, stream_column, max_valu
return cache, min_val
+ def _get_active_presence(self, db_conn):
+ """Fetch non-offline presence from the database so that we can register
+ the appropriate time outs.
+ """
+
+ sql = (
+ "SELECT user_id, state, last_active_ts, last_federation_update_ts,"
+ " last_user_sync_ts, status_msg, currently_active FROM presence_stream"
+ " WHERE state != ?"
+ )
+ sql = self.database_engine.convert_param_style(sql)
+
+ txn = db_conn.cursor()
@NegativeMjark

NegativeMjark Feb 18, 2016

Contributor

Do you need to close the cursor?

@NegativeMjark NegativeMjark commented on an outdated diff Feb 18, 2016

synapse/rest/client/v1/presence.py
@@ -35,8 +35,15 @@ def on_GET(self, request, user_id):
requester = yield self.auth.get_user_by_req(request)
user = UserID.from_string(user_id)
- state = yield self.handlers.presence_handler.get_state(
- target_user=user, auth_user=requester.user)
+ if requester.user != user:
+ allowed = yield self.handlers.presence_handler.is_visible(
+ observed_user=user, observer_user=requester.user,
+ )
+
+ if not allowed:
+ raise AuthError(403, "You are allowed to see their presence.")
@NegativeMjark

NegativeMjark Feb 18, 2016

Contributor

"You are not..."

Contributor

NegativeMjark commented Feb 18, 2016

LGTM

erikjohnston added a commit that referenced this pull request Feb 19, 2016

Merge pull request #582 from matrix-org/erikj/presence
Rewrite presence for performance.

@erikjohnston erikjohnston merged commit e5ad2e5 into develop Feb 19, 2016

1 check passed

default Build finished. 323 tests run, 0 skipped, 0 failed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment