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

Commit 6a67f37

Browse files
authored
Fix logging context warnings when losing replication connection (#10984)
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__`.
1 parent 013e0f9 commit 6a67f37

File tree

3 files changed

+27
-10
lines changed

3 files changed

+27
-10
lines changed

changelog.d/10984.misc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix spurious warnings about losing the logging context on the `ReplicationCommandHandler` when losing the replication connection.

synapse/replication/tcp/protocol.py

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -182,9 +182,13 @@ def __init__(self, clock: Clock, handler: "ReplicationCommandHandler"):
182182

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

189193
def connectionMade(self):
190194
logger.info("[%s] Connection established", self.id())
@@ -434,8 +438,12 @@ def on_connection_closed(self):
434438
if self.transport:
435439
self.transport.unregisterProducer()
436440

437-
# mark the logging context as finished
438-
self._logging_context.__exit__(None, None, None)
441+
# mark the logging context as finished by triggering `__exit__()`
442+
with PreserveLoggingContext():
443+
with self._logging_context:
444+
pass
445+
# the sentinel context is now active, which may not be correct.
446+
# PreserveLoggingContext() will restore the correct logging context.
439447

440448
def __str__(self):
441449
addr = None

synapse/replication/tcp/redis.py

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -100,9 +100,13 @@ def __init__(self, *args, **kwargs):
100100

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

107111
def connectionMade(self):
108112
logger.info("Connected to redis")
@@ -182,8 +186,12 @@ def connectionLost(self, reason):
182186
super().connectionLost(reason)
183187
self.synapse_handler.lost_connection(self)
184188

185-
# mark the logging context as finished
186-
self._logging_context.__exit__(None, None, None)
189+
# mark the logging context as finished by triggering `__exit__()`
190+
with PreserveLoggingContext():
191+
with self._logging_context:
192+
pass
193+
# the sentinel context is now active, which may not be correct.
194+
# PreserveLoggingContext() will restore the correct logging context.
187195

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

0 commit comments

Comments
 (0)