Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

Commit a2f1833

Browse files
committed
Fix concurrent modification errors in pusher metrics (#7106)
* commit 'e913823a2': Fix concurrent modification errors in pusher metrics (#7106)
2 parents fd4dc99 + e913823 commit a2f1833

File tree

2 files changed

+19
-9
lines changed

2 files changed

+19
-9
lines changed

changelog.d/7106.feature

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Add prometheus metrics for the number of active pushers.

synapse/push/pusherpool.py

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import logging
1818
from collections import defaultdict
19+
from threading import Lock
1920
from typing import Dict, Tuple, Union
2021

2122
from twisted.internet import defer
@@ -56,12 +57,17 @@ def __init__(self, _hs):
5657
# map from user id to app_id:pushkey to pusher
5758
self.pushers = {} # type: Dict[str, Dict[str, Union[HttpPusher, EmailPusher]]]
5859

60+
# a lock for the pushers dict, since `count_pushers` is called from an different
61+
# and we otherwise get concurrent modification errors
62+
self._pushers_lock = Lock()
63+
5964
def count_pushers():
6065
results = defaultdict(int) # type: Dict[Tuple[str, str], int]
61-
for pushers in self.pushers.values():
62-
for pusher in pushers.values():
63-
k = (type(pusher).__name__, pusher.app_id)
64-
results[k] += 1
66+
with self._pushers_lock:
67+
for pushers in self.pushers.values():
68+
for pusher in pushers.values():
69+
k = (type(pusher).__name__, pusher.app_id)
70+
results[k] += 1
6571
return results
6672

6773
LaterGauge(
@@ -293,11 +299,12 @@ def _start_pusher(self, pusherdict):
293299
return
294300

295301
appid_pushkey = "%s:%s" % (pusherdict["app_id"], pusherdict["pushkey"])
296-
byuser = self.pushers.setdefault(pusherdict["user_name"], {})
297302

298-
if appid_pushkey in byuser:
299-
byuser[appid_pushkey].on_stop()
300-
byuser[appid_pushkey] = p
303+
with self._pushers_lock:
304+
byuser = self.pushers.setdefault(pusherdict["user_name"], {})
305+
if appid_pushkey in byuser:
306+
byuser[appid_pushkey].on_stop()
307+
byuser[appid_pushkey] = p
301308

302309
# Check if there *may* be push to process. We do this as this check is a
303310
# lot cheaper to do than actually fetching the exact rows we need to
@@ -326,7 +333,9 @@ def remove_pusher(self, app_id, pushkey, user_id):
326333
if appid_pushkey in byuser:
327334
logger.info("Stopping pusher %s / %s", user_id, appid_pushkey)
328335
byuser[appid_pushkey].on_stop()
329-
del byuser[appid_pushkey]
336+
with self._pushers_lock:
337+
del byuser[appid_pushkey]
338+
330339
yield self.store.delete_pusher_by_app_id_pushkey_user_id(
331340
app_id, pushkey, user_id
332341
)

0 commit comments

Comments
 (0)