Merge pull request #345 from bjester/buffer-serialization-perf

Improve buffer serialization performance by performing bulk counter lookup

Author: bjester

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

morango/__init__.py

@@ -1 +1 @@
-__version__ = "0.8.14"
+__version__ = "0.8.15"

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",

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
         )

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()