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

Commit

Permalink
Fix logging context warnings when losing replication connection (#10984)
Browse files Browse the repository at this point in the history
Instead of triggering `__exit__` manually on the replication handler's
logging context, use it as a context manager so that there is an
`__enter__` call to balance the `__exit__`.
  • Loading branch information
squahtx authored Oct 15, 2021
1 parent 013e0f9 commit 6a67f37
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 10 deletions.
1 change: 1 addition & 0 deletions changelog.d/10984.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix spurious warnings about losing the logging context on the `ReplicationCommandHandler` when losing the replication connection.
18 changes: 13 additions & 5 deletions synapse/replication/tcp/protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,13 @@ def __init__(self, clock: Clock, handler: "ReplicationCommandHandler"):

# a logcontext which we use for processing incoming commands. We declare it as a
# background process so that the CPU stats get reported to prometheus.
self._logging_context = BackgroundProcessLoggingContext(
"replication-conn", self.conn_id
)
with PreserveLoggingContext():
# thanks to `PreserveLoggingContext()`, the new logcontext is guaranteed to
# capture the sentinel context as its containing context and won't prevent
# GC of / unintentionally reactivate what would be the current context.
self._logging_context = BackgroundProcessLoggingContext(
"replication-conn", self.conn_id
)

def connectionMade(self):
logger.info("[%s] Connection established", self.id())
Expand Down Expand Up @@ -434,8 +438,12 @@ def on_connection_closed(self):
if self.transport:
self.transport.unregisterProducer()

# mark the logging context as finished
self._logging_context.__exit__(None, None, None)
# mark the logging context as finished by triggering `__exit__()`
with PreserveLoggingContext():
with self._logging_context:
pass
# the sentinel context is now active, which may not be correct.
# PreserveLoggingContext() will restore the correct logging context.

def __str__(self):
addr = None
Expand Down
18 changes: 13 additions & 5 deletions synapse/replication/tcp/redis.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,13 @@ def __init__(self, *args, **kwargs):

# a logcontext which we use for processing incoming commands. We declare it as a
# background process so that the CPU stats get reported to prometheus.
self._logging_context = BackgroundProcessLoggingContext(
"replication_command_handler"
)
with PreserveLoggingContext():
# thanks to `PreserveLoggingContext()`, the new logcontext is guaranteed to
# capture the sentinel context as its containing context and won't prevent
# GC of / unintentionally reactivate what would be the current context.
self._logging_context = BackgroundProcessLoggingContext(
"replication_command_handler"
)

def connectionMade(self):
logger.info("Connected to redis")
Expand Down Expand Up @@ -182,8 +186,12 @@ def connectionLost(self, reason):
super().connectionLost(reason)
self.synapse_handler.lost_connection(self)

# mark the logging context as finished
self._logging_context.__exit__(None, None, None)
# mark the logging context as finished by triggering `__exit__()`
with PreserveLoggingContext():
with self._logging_context:
pass
# the sentinel context is now active, which may not be correct.
# PreserveLoggingContext() will restore the correct logging context.

def send_command(self, cmd: Command):
"""Send a command if connection has been established.
Expand Down

0 comments on commit 6a67f37

Please sign in to comment.