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

Fix "Starting metrics collection from sentinel context" errors #9053

Merged
merged 3 commits into from
Jan 8, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/9053.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Various clean-ups to the structured logging and logging context code.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kinda wonder if this is a different bugfix. It should mean that we correctly track some CPU usage that was being lost before.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, though tbh the measure block looks pretty pointless, its not doing very much in there at all

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't it go off and wake up all the syncs and things?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It just needs to look up each user and room in the dicts and call .notify(). I've updated the changelog anyway

39 changes: 19 additions & 20 deletions synapse/notifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -396,31 +396,30 @@ def on_new_event(

Will wake up all listeners for the given users and rooms.
"""
with PreserveLoggingContext():
with Measure(self.clock, "on_new_event"):
user_streams = set()
with Measure(self.clock, "on_new_event"):
user_streams = set()

for user in users:
user_stream = self.user_to_user_stream.get(str(user))
if user_stream is not None:
user_streams.add(user_stream)
for user in users:
user_stream = self.user_to_user_stream.get(str(user))
if user_stream is not None:
user_streams.add(user_stream)

for room in rooms:
user_streams |= self.room_to_user_streams.get(room, set())
for room in rooms:
user_streams |= self.room_to_user_streams.get(room, set())

time_now_ms = self.clock.time_msec()
for user_stream in user_streams:
try:
user_stream.notify(stream_key, new_token, time_now_ms)
except Exception:
logger.exception("Failed to notify listener")
time_now_ms = self.clock.time_msec()
for user_stream in user_streams:
try:
user_stream.notify(stream_key, new_token, time_now_ms)
except Exception:
logger.exception("Failed to notify listener")

self.notify_replication()
self.notify_replication()

# Notify appservices
self._notify_app_services_ephemeral(
stream_key, new_token, users,
)
# Notify appservices
self._notify_app_services_ephemeral(
stream_key, new_token, users,
)

def on_new_replication_data(self) -> None:
"""Used to inform replication listeners that something has happened
Expand Down
3 changes: 2 additions & 1 deletion synapse/util/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ def __init__(self, clock, name):
curr_context = current_context()
if not curr_context:
logger.warning(
"Starting metrics collection from sentinel context: metrics will be lost"
"Starting metrics collection %r from sentinel context: metrics will be lost",
name,
)
parent_context = None
else:
Expand Down