From 6c1ec5a1bdfaed2d13a501fd2441d99c73711adb Mon Sep 17 00:00:00 2001 From: Oleg Girko Date: Tue, 10 Jul 2018 00:11:39 +0100 Subject: Use more portable syntax using attrs package. Newer syntax attr.ib(factory=dict) is just a syntactic sugar for attr.ib(default=attr.Factory(dict)) It was introduced in newest version of attrs package (18.1.0) and doesn't work with older versions. We should either require minimum version of attrs to be 18.1.0, or use older (slightly more verbose) syntax. Requiring newest version is not a good solution because Linux distributions may have older version of attrs (17.4.0 in Fedora 28), and requiring to build (and package) newer version just to use newer syntactic sugar in only one test is just too much. It's much better to fix that test to use older syntax. Signed-off-by: Oleg Girko --- tests/server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/server.py b/tests/server.py index 46223ccf05..e93f5a7f94 100644 --- a/tests/server.py +++ b/tests/server.py @@ -22,7 +22,7 @@ class FakeChannel(object): wire). """ - result = attr.ib(factory=dict) + result = attr.ib(default=attr.Factory(dict)) @property def json_body(self): -- cgit 1.5.1 From 05f5dabc10f9d7a4403c9571c12371b2b6dd93f7 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 10 Jul 2018 17:21:17 +0100 Subject: Use stream cache in get_linearized_receipts_for_room This avoids us from uncessarily hitting the database when there has been no change for the room --- synapse/replication/slave/storage/receipts.py | 2 +- synapse/storage/receipts.py | 17 +++++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/synapse/replication/slave/storage/receipts.py b/synapse/replication/slave/storage/receipts.py index 7ab12b850f..ed12342f40 100644 --- a/synapse/replication/slave/storage/receipts.py +++ b/synapse/replication/slave/storage/receipts.py @@ -49,7 +49,7 @@ class SlavedReceiptsStore(ReceiptsWorkerStore, BaseSlavedStore): def invalidate_caches_for_receipt(self, room_id, receipt_type, user_id): self.get_receipts_for_user.invalidate((user_id, receipt_type)) - self.get_linearized_receipts_for_room.invalidate_many((room_id,)) + self._get_linearized_receipts_for_room.invalidate_many((room_id,)) self.get_last_receipt_event_id_for_user.invalidate( (user_id, room_id, receipt_type) ) diff --git a/synapse/storage/receipts.py b/synapse/storage/receipts.py index 3738901ea4..401400d927 100644 --- a/synapse/storage/receipts.py +++ b/synapse/storage/receipts.py @@ -151,7 +151,6 @@ class ReceiptsWorkerStore(SQLBaseStore): defer.returnValue([ev for res in results.values() for ev in res]) - @cachedInlineCallbacks(num_args=3, tree=True) def get_linearized_receipts_for_room(self, room_id, to_key, from_key=None): """Get receipts for a single room for sending to clients. @@ -164,6 +163,16 @@ class ReceiptsWorkerStore(SQLBaseStore): Returns: list: A list of receipts. """ + if from_key: + if not self._receipts_stream_cache.has_entity_changed(room_id, from_key): + defer.succeed([]) + + return self._get_linearized_receipts_for_room(room_id, to_key, from_key) + + @cachedInlineCallbacks(num_args=3, tree=True) + def _get_linearized_receipts_for_room(self, room_id, to_key, from_key=None): + """See get_linearized_receipts_for_room + """ def f(txn): if from_key: sql = ( @@ -211,7 +220,7 @@ class ReceiptsWorkerStore(SQLBaseStore): "content": content, }]) - @cachedList(cached_method_name="get_linearized_receipts_for_room", + @cachedList(cached_method_name="_get_linearized_receipts_for_room", list_name="room_ids", num_args=3, inlineCallbacks=True) def _get_linearized_receipts_for_rooms(self, room_ids, to_key, from_key=None): if not room_ids: @@ -373,7 +382,7 @@ class ReceiptsStore(ReceiptsWorkerStore): self.get_receipts_for_user.invalidate, (user_id, receipt_type) ) # FIXME: This shouldn't invalidate the whole cache - txn.call_after(self.get_linearized_receipts_for_room.invalidate_many, (room_id,)) + txn.call_after(self._get_linearized_receipts_for_room.invalidate_many, (room_id,)) txn.call_after( self._receipts_stream_cache.entity_has_changed, @@ -493,7 +502,7 @@ class ReceiptsStore(ReceiptsWorkerStore): self.get_receipts_for_user.invalidate, (user_id, receipt_type) ) # FIXME: This shouldn't invalidate the whole cache - txn.call_after(self.get_linearized_receipts_for_room.invalidate_many, (room_id,)) + txn.call_after(self._get_linearized_receipts_for_room.invalidate_many, (room_id,)) self._simple_delete_txn( txn, -- cgit 1.5.1 From bb3d5360878f10deb89f27e6332279540b8888e2 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 10 Jul 2018 17:28:31 +0100 Subject: Newsfile --- changelog.d/3505.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/3505.feature diff --git a/changelog.d/3505.feature b/changelog.d/3505.feature new file mode 100644 index 0000000000..ca1867f529 --- /dev/null +++ b/changelog.d/3505.feature @@ -0,0 +1 @@ +Reduce database consumption when processing large numbers of receipts -- cgit 1.5.1 From 6ccefef07a0eb366d64076bf6052b94409262981 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 10 Jul 2018 18:12:39 +0100 Subject: Use 'is not None' and add comments --- synapse/storage/receipts.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/synapse/storage/receipts.py b/synapse/storage/receipts.py index 401400d927..33fc2b9b31 100644 --- a/synapse/storage/receipts.py +++ b/synapse/storage/receipts.py @@ -140,7 +140,9 @@ class ReceiptsWorkerStore(SQLBaseStore): """ room_ids = set(room_ids) - if from_key: + if from_key is not None: + # Only ask the database about rooms where there have been new + # receipts added since `from_key` room_ids = yield self._receipts_stream_cache.get_entities_changed( room_ids, from_key ) @@ -163,7 +165,9 @@ class ReceiptsWorkerStore(SQLBaseStore): Returns: list: A list of receipts. """ - if from_key: + if from_key is not None: + # Check the cache first to see if any new receipts have been added + # since`from_key`. If not we can no-op. if not self._receipts_stream_cache.has_entity_changed(room_id, from_key): defer.succeed([]) -- cgit 1.5.1 From 85354bb18e100f086828a83c269c8902f33cecee Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Wed, 11 Jul 2018 03:27:03 +1000 Subject: changelog entry --- changelog.d/3498.misc | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 changelog.d/3498.misc diff --git a/changelog.d/3498.misc b/changelog.d/3498.misc new file mode 100644 index 0000000000..e69de29bb2 -- cgit 1.5.1 From aff1dfdf3decdd3ed60d7d8de8d2b07904f39d2b Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 12 Jul 2018 09:45:37 +0100 Subject: Update return value docstring --- synapse/storage/receipts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/receipts.py b/synapse/storage/receipts.py index 33fc2b9b31..0ac665e967 100644 --- a/synapse/storage/receipts.py +++ b/synapse/storage/receipts.py @@ -163,7 +163,7 @@ class ReceiptsWorkerStore(SQLBaseStore): from the start. Returns: - list: A list of receipts. + Deferred[list]: A list of receipts. """ if from_key is not None: # Check the cache first to see if any new receipts have been added -- cgit 1.5.1