From b073ad647267c243ac6154fdfe6d6979de14edf3 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 9 Jun 2021 10:46:19 +0100 Subject: [PATCH 1/4] Fix bug when running presence off master Hopefully fixes #10027. --- synapse/storage/databases/main/presence.py | 2 +- synapse/storage/util/id_generators.py | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/presence.py b/synapse/storage/databases/main/presence.py index 6a2baa7841e0..1388771c40e8 100644 --- a/synapse/storage/databases/main/presence.py +++ b/synapse/storage/databases/main/presence.py @@ -50,7 +50,7 @@ def __init__( instance_name=self._instance_name, tables=[("presence_stream", "instance_name", "stream_id")], sequence_name="presence_stream_sequence", - writers=hs.config.worker.writers.to_device, + writers=hs.config.worker.writers.presence, ) else: self._presence_id_gen = StreamIdGenerator( diff --git a/synapse/storage/util/id_generators.py b/synapse/storage/util/id_generators.py index b1bd3a52d98f..d768b1841f67 100644 --- a/synapse/storage/util/id_generators.py +++ b/synapse/storage/util/id_generators.py @@ -397,6 +397,10 @@ def get_next(self): # ... persist event ... """ + # If we have a list of instance that are allowed to write to this + # stream, make sure we're in it. + assert not self._writers or self._instance_name in self._writers + return _MultiWriterCtxManager(self) def get_next_mult(self, n: int): @@ -406,6 +410,10 @@ def get_next_mult(self, n: int): # ... persist events ... """ + # If we have a list of instance that are allowed to write to this + # stream, make sure we're in it. + assert not self._writers or self._instance_name in self._writers + return _MultiWriterCtxManager(self, n) def get_next_txn(self, txn: LoggingTransaction): @@ -416,6 +424,10 @@ def get_next_txn(self, txn: LoggingTransaction): # ... persist event ... """ + # If we have a list of instance that are allowed to write to this + # stream, make sure we're in it. + assert not self._writers or self._instance_name in self._writers + next_id = self._load_next_id_txn(txn) with self._lock: From 77ad77b24b8bfa4b645b48fc9819cc5a3005f0f5 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 9 Jun 2021 10:47:48 +0100 Subject: [PATCH 2/4] Newsfile --- changelog.d/10149.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/10149.bugfix diff --git a/changelog.d/10149.bugfix b/changelog.d/10149.bugfix new file mode 100644 index 000000000000..5ef0fa4826fb --- /dev/null +++ b/changelog.d/10149.bugfix @@ -0,0 +1 @@ +Fix bug where presence updates would stop working when using a presence writer worker. From 458aef87e31ef985d7a2d988f146904a9fdd1ff9 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 10 Jun 2021 16:48:08 +0100 Subject: [PATCH 3/4] Use exceptions --- synapse/storage/util/id_generators.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/synapse/storage/util/id_generators.py b/synapse/storage/util/id_generators.py index d768b1841f67..d2c2ee1f1776 100644 --- a/synapse/storage/util/id_generators.py +++ b/synapse/storage/util/id_generators.py @@ -399,7 +399,8 @@ def get_next(self): # If we have a list of instance that are allowed to write to this # stream, make sure we're in it. - assert not self._writers or self._instance_name in self._writers + if self._writers and self._instance_name not in self._writers: + raise Exception("Tried to allocate stream ID on non-writer") return _MultiWriterCtxManager(self) @@ -412,7 +413,8 @@ def get_next_mult(self, n: int): # If we have a list of instance that are allowed to write to this # stream, make sure we're in it. - assert not self._writers or self._instance_name in self._writers + if self._writers and self._instance_name not in self._writers: + raise Exception("Tried to allocate stream ID on non-writer") return _MultiWriterCtxManager(self, n) @@ -426,7 +428,8 @@ def get_next_txn(self, txn: LoggingTransaction): # If we have a list of instance that are allowed to write to this # stream, make sure we're in it. - assert not self._writers or self._instance_name in self._writers + if self._writers and self._instance_name not in self._writers: + raise Exception("Tried to allocate stream ID on non-writer") next_id = self._load_next_id_txn(txn) From 13d7200b5afcd5505e9fc74736acd0e2fa9d29f7 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 11 Jun 2021 09:33:10 +0100 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- changelog.d/10149.bugfix | 2 +- synapse/storage/util/id_generators.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/changelog.d/10149.bugfix b/changelog.d/10149.bugfix index 5ef0fa4826fb..cb2d2eedb389 100644 --- a/changelog.d/10149.bugfix +++ b/changelog.d/10149.bugfix @@ -1 +1 @@ -Fix bug where presence updates would stop working when using a presence writer worker. +Fix a bug which caused presence updates to stop working some time after restart, when using a presence writer worker. diff --git a/synapse/storage/util/id_generators.py b/synapse/storage/util/id_generators.py index d2c2ee1f1776..f1e62f9e855d 100644 --- a/synapse/storage/util/id_generators.py +++ b/synapse/storage/util/id_generators.py @@ -397,7 +397,7 @@ def get_next(self): # ... persist event ... """ - # If we have a list of instance that are allowed to write to this + # If we have a list of instances that are allowed to write to this # stream, make sure we're in it. if self._writers and self._instance_name not in self._writers: raise Exception("Tried to allocate stream ID on non-writer") @@ -411,7 +411,7 @@ def get_next_mult(self, n: int): # ... persist events ... """ - # If we have a list of instance that are allowed to write to this + # If we have a list of instances that are allowed to write to this # stream, make sure we're in it. if self._writers and self._instance_name not in self._writers: raise Exception("Tried to allocate stream ID on non-writer") @@ -426,7 +426,7 @@ def get_next_txn(self, txn: LoggingTransaction): # ... persist event ... """ - # If we have a list of instance that are allowed to write to this + # If we have a list of instances that are allowed to write to this # stream, make sure we're in it. if self._writers and self._instance_name not in self._writers: raise Exception("Tried to allocate stream ID on non-writer")