diff options
| author | Sean Quah <8349537+squahtx@users.noreply.github.com> | 2022-07-18 17:39:39 +0100 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2022-07-18 17:39:39 +0100 |
| commit | 5526f9fc4f53c9e0eac6aa610bf0a76906b772dc (patch) | |
| tree | 63cc2cc12ed0d6ba886ce984eae50769481ebf37 | |
| parent | Up the dependency on canonicaljson to ^1.5.0 (#13172) (diff) | |
| download | synapse-5526f9fc4f53c9e0eac6aa610bf0a76906b772dc.tar.xz | |
Fix overcounting of pushers when they are replaced (#13296)
Signed-off-by: Sean Quah <seanq@matrix.org>
| -rw-r--r-- | changelog.d/13296.bugfix | 1 | ||||
| -rw-r--r-- | synapse/push/pusherpool.py | 27 |
2 files changed, 17 insertions, 11 deletions
diff --git a/changelog.d/13296.bugfix b/changelog.d/13296.bugfix new file mode 100644 index 0000000000..ff0eb2b4a1 --- /dev/null +++ b/changelog.d/13296.bugfix @@ -0,0 +1 @@ +Fix a bug introduced in v1.18.0 where the `synapse_pushers` metric would overcount pushers when they are replaced. diff --git a/synapse/push/pusherpool.py b/synapse/push/pusherpool.py index d0cc657b44..1e0ef44fc7 100644 --- a/synapse/push/pusherpool.py +++ b/synapse/push/pusherpool.py @@ -328,7 +328,7 @@ class PusherPool: return None try: - p = self.pusher_factory.create_pusher(pusher_config) + pusher = self.pusher_factory.create_pusher(pusher_config) except PusherConfigException as e: logger.warning( "Pusher incorrectly configured id=%i, user=%s, appid=%s, pushkey=%s: %s", @@ -346,23 +346,28 @@ class PusherPool: ) return None - if not p: + if not pusher: 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: - byuser[appid_pushkey].on_stop() - byuser[appid_pushkey] = p + previous_pusher = byuser[appid_pushkey] + 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 # lot cheaper to do than actually fetching the exact rows we need to # push. - user_id = pusher_config.user_name - last_stream_ordering = pusher_config.last_stream_ordering + user_id = pusher.user_id + last_stream_ordering = pusher.last_stream_ordering if last_stream_ordering: have_notifs = await self.store.get_if_maybe_push_in_range_for_user( user_id, last_stream_ordering @@ -372,9 +377,9 @@ class PusherPool: # risk missing push. 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: appid_pushkey = "%s:%s" % (app_id, pushkey) |