From 6a67f3786a73f72739ebe8e5aca372c39626d768 Mon Sep 17 00:00:00 2001
From: Sean Quah <8349537+squahtx@users.noreply.github.com>
Date: Fri, 15 Oct 2021 13:10:58 +0100
Subject: [PATCH] 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__`.
---
changelog.d/10984.misc | 1 +
synapse/replication/tcp/protocol.py | 18 +++++++++++++-----
synapse/replication/tcp/redis.py | 18 +++++++++++++-----
3 files changed, 27 insertions(+), 10 deletions(-)
create mode 100644 changelog.d/10984.misc
diff --git a/changelog.d/10984.misc b/changelog.d/10984.misc
new file mode 100644
index 0000000000..86c4081cc4
--- /dev/null
+++ b/changelog.d/10984.misc
@@ -0,0 +1 @@
+Fix spurious warnings about losing the logging context on the `ReplicationCommandHandler` when losing the replication connection.
diff --git a/synapse/replication/tcp/protocol.py b/synapse/replication/tcp/protocol.py
index 8c80153ab6..7bae36db16 100644
--- a/synapse/replication/tcp/protocol.py
+++ b/synapse/replication/tcp/protocol.py
@@ -182,9 +182,13 @@ class BaseReplicationStreamProtocol(LineOnlyReceiver):
# 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())
@@ -434,8 +438,12 @@ class BaseReplicationStreamProtocol(LineOnlyReceiver):
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
diff --git a/synapse/replication/tcp/redis.py b/synapse/replication/tcp/redis.py
index 062fe2f33e..8d28bd3f3f 100644
--- a/synapse/replication/tcp/redis.py
+++ b/synapse/replication/tcp/redis.py
@@ -100,9 +100,13 @@ class RedisSubscriber(txredisapi.SubscriberProtocol):
# 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")
@@ -182,8 +186,12 @@ class RedisSubscriber(txredisapi.SubscriberProtocol):
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.
--
GitLab