diff --git a/CHANGELOG.md b/CHANGELOG.md index 876b0e43..bcc118da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ List of the most important changes for each release. +## 0.8.15 +- Improves buffer serialization performance by performing bulk counter lookup + ## 0.8.14 - Adds utility for addressing immediate FK constraints caused by Django upgrade, automatically performed for morango models in a Django migration. - Adds retry behavior for low-level connection issues not handled by `urllib3` retries diff --git a/morango/__init__.py b/morango/__init__.py index fb551e73..846679f4 100644 --- a/morango/__init__.py +++ b/morango/__init__.py @@ -1 +1 @@ -__version__ = "0.8.14" +__version__ = "0.8.15" diff --git a/morango/api/serializers.py b/morango/api/serializers.py index be92121f..709f2432 100644 --- a/morango/api/serializers.py +++ b/morango/api/serializers.py @@ -1,3 +1,5 @@ +from collections import defaultdict + from rest_framework import exceptions from rest_framework import serializers from rest_framework.fields import ReadOnlyField @@ -158,11 +160,38 @@ class Meta: read_only_fields = fields +class BufferListSerializer(serializers.ListSerializer): + def to_representation(self, data): + buffers = list(data) + if buffers: + rmcb_map = defaultdict(list) + transfer_session_ids = {b.transfer_session_id for b in buffers} + # Morango implementation will only ever call this for one transfer session ID at a time, + # but a loop makes the code straightforward regardless and limits the quantity of + # SQLite variables in use + for transfer_session_id in transfer_session_ids: + # bulk-fetch all RMCB records needed for this batch of buffers in a + # single query, then cache the relevant subset on each buffer, instead + # of letting each buffer's nested rmcb_list serializer issue its own query + rmcb_queryset = RecordMaxCounterBuffer.objects.filter( + transfer_session_id=transfer_session_id, + model_uuid__in={b.model_uuid for b in buffers if b.transfer_session_id == transfer_session_id}, + ) + for rmcb in rmcb_queryset: + rmcb_map[(transfer_session_id, rmcb.model_uuid)].append(rmcb) + for buffer in buffers: + buffer._rmcb_list = rmcb_map[ + (buffer.transfer_session_id, buffer.model_uuid) + ] + return super(BufferListSerializer, self).to_representation(buffers) + + class BufferSerializer(serializers.ModelSerializer): rmcb_list = RecordMaxCounterBufferSerializer(many=True) class Meta: model = Buffer + list_serializer_class = BufferListSerializer fields = ( "serialized", "deleted", diff --git a/morango/models/core.py b/morango/models/core.py index f04d663e..0742c65a 100644 --- a/morango/models/core.py +++ b/morango/models/core.py @@ -553,6 +553,11 @@ class Meta: unique_together = ("transfer_session", "model_uuid") def rmcb_list(self): + # allow callers (e.g. BufferListSerializer) to batch-fetch RMCB records + # for many buffers at once and cache them here, to avoid an N+1 query + # pattern when serializing a large number of buffers + if hasattr(self, "_rmcb_list"): + return self._rmcb_list return RecordMaxCounterBuffer.objects.filter( model_uuid=self.model_uuid, transfer_session_id=self.transfer_session_id ) diff --git a/tests/testapp/tests/test_api.py b/tests/testapp/tests/test_api.py index f0a873aa..660cadb9 100644 --- a/tests/testapp/tests/test_api.py +++ b/tests/testapp/tests/test_api.py @@ -1073,6 +1073,22 @@ def test_buffer_serializer_makes_no_transfer_session_query(self): for q in ctx.captured_queries: self.assertFalse('morango_transfersession' in q['sql']) + def test_buffer_serializer_batches_rmcb_queries_for_many(self): + transfer_session_id = self.create_records_for_pulling(count=10) + buffers = Buffer.objects.filter( + transfer_session_id=transfer_session_id + ).order_by("pk") + + with CaptureQueriesContext(connection) as ctx: + BufferSerializer(buffers, many=True).data + + rmcb_queries = [ + q + for q in ctx.captured_queries + if "recordmaxcounterbuffer" in q["sql"].lower() + ] + self.assertEqual(len(rmcb_queries), 1) + def test_pull_valid_buffer_list(self): transfer_session_id = self.create_records_for_pulling()