OpenCloud
/
MatrixSynapse
mirror of https://github.com/matrix-org/synapse
1
0
Fork 0
Code Issues Releases Wiki Activity

Fix overcounting of pushers when they are replaced (#13296) 

Signed-off-by: Sean Quah <seanq@matrix.org>
pull/13326/head
Sean Quah 2022-07-18 17:39:39 +01:00 committed by GitHub
parent 8c60c572f0
commit 5526f9fc4f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 17 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
changelog.d/13296.bugfix Normal file
View File

@ -0,0 +1 @@
Fix a bug introduced in v1.18.0 where the `synapse_pushers` metric would overcount pushers when they are replaced.

27
synapse/push/pusherpool.py
View File

@ -328,7 +328,7 @@ class PusherPool:
return None return None
try: try:
p = self.pusher_factory.create_pusher(pusher_config) pusher = self.pusher_factory.create_pusher(pusher_config)
except PusherConfigException as e: except PusherConfigException as e:
logger.warning( logger.warning(
"Pusher incorrectly configured id=%i, user=%s, appid=%s, pushkey=%s: %s", "Pusher incorrectly configured id=%i, user=%s, appid=%s, pushkey=%s: %s",
@ -346,23 +346,28 @@ class PusherPool:
) )
return None return None
if not p: if not pusher:
return None return None
appid_pushkey = "%s:%s" % (pusher_config.app_id, pusher_config.pushkey) appid_pushkey = "%s:%s" % (pusher.app_id, pusher.pushkey)
byuser = self.pushers.setdefault(pusher_config.user_name, {}) byuser = self.pushers.setdefault(pusher.user_id, {})
if appid_pushkey in byuser: if appid_pushkey in byuser:
byuser[appid_pushkey].on_stop() previous_pusher = byuser[appid_pushkey]
byuser[appid_pushkey] = p previous_pusher.on_stop()
synapse_pushers.labels(type(p).__name__, p.app_id).inc() synapse_pushers.labels(
type(previous_pusher).__name__, previous_pusher.app_id
).dec()
byuser[appid_pushkey] = pusher
synapse_pushers.labels(type(pusher).__name__, pusher.app_id).inc()
# Check if there *may* be push to process. We do this as this check is a # Check if there *may* be push to process. We do this as this check is a
# lot cheaper to do than actually fetching the exact rows we need to # lot cheaper to do than actually fetching the exact rows we need to
# push. # push.
user_id = pusher_config.user_name user_id = pusher.user_id
last_stream_ordering = pusher_config.last_stream_ordering last_stream_ordering = pusher.last_stream_ordering
if last_stream_ordering: if last_stream_ordering:
have_notifs = await self.store.get_if_maybe_push_in_range_for_user( have_notifs = await self.store.get_if_maybe_push_in_range_for_user(
user_id, last_stream_ordering user_id, last_stream_ordering
@ -372,9 +377,9 @@ class PusherPool:
# risk missing push. # risk missing push.
have_notifs = True have_notifs = True
p.on_started(have_notifs) pusher.on_started(have_notifs)
return p return pusher
async def remove_pusher(self, app_id: str, pushkey: str, user_id: str) -> None: async def remove_pusher(self, app_id: str, pushkey: str, user_id: str) -> None:
appid_pushkey = "%s:%s" % (app_id, pushkey) appid_pushkey = "%s:%s" % (app_id, pushkey)