From bc3dd530d0554bb03255d21acfdef436fdbcc037 Mon Sep 17 00:00:00 2001 From: Bram Date: Thu, 16 Jul 2026 22:08:52 +0200 Subject: [PATCH] fix: reduce backend memory usage by releasing each API response memory after request is sent (#5733) * Reduce backend memory usage by releasing each API response memory after request is sent * Fixed feedback --- backend/src/baserow/api/renderers.py | 109 +++++++++++++ backend/src/baserow/config/asgi.py | 8 +- backend/src/baserow/config/helpers.py | 25 +++ backend/src/baserow/config/settings/base.py | 2 +- backend/tests/baserow/api/test_renderers.py | 146 ++++++++++++++++++ .../tests/baserow/config/test_asgi_handler.py | 87 +++++++++++ ...usage_by_releasing_each_api_responses.json | 9 ++ 7 files changed, 383 insertions(+), 3 deletions(-) create mode 100644 backend/src/baserow/api/renderers.py create mode 100644 backend/tests/baserow/api/test_renderers.py create mode 100644 backend/tests/baserow/config/test_asgi_handler.py create mode 100644 changelog/entries/unreleased/bug/reduces_backend_memory_usage_by_releasing_each_api_responses.json diff --git a/backend/src/baserow/api/renderers.py b/backend/src/baserow/api/renderers.py new file mode 100644 index 0000000000..79ecd7be4a --- /dev/null +++ b/backend/src/baserow/api/renderers.py @@ -0,0 +1,109 @@ +from typing import Any, Iterator + +from rest_framework.renderers import JSONRenderer +from rest_framework.serializers import BaseSerializer + + +def _iter_serializer_outputs(data: Any) -> Iterator[BaseSerializer]: + """ + Yields the serializer of every `ReturnList`/`ReturnDict` anywhere in the response + data tree. Serializer outputs can sit at the top level, inside plain lists (e.g. + the job and application APIs), or in deeper mappings (e.g. the kanban and calendar + grouped-row responses). + """ + + stack = [data] + seen = set() + while stack: + node = stack.pop() + if not isinstance(node, (dict, list, tuple)) or id(node) in seen: + continue + seen.add(id(node)) + serializer = getattr(node, "serializer", None) + if serializer is not None: + yield serializer + stack.extend(node.values() if isinstance(node, dict) else node) + + +def _release_serializer_references(serializer: BaseSerializer) -> None: + """ + Severs the references a DRF serializer graph keeps to the serialized payload. The + serializer skeleton itself stays cyclic (that is inherent to DRF and small), but + `instance`, `_args`, `_kwargs`, `_data`, and `_context` pin the fetched model + instances, the serialized output, the request, and arbitrary context payloads, + which is where the actual memory sits. + """ + + current = serializer + seen = set() + while current is not None and id(current) not in seen: + seen.add(id(current)) + instance_dict = getattr(current, "__dict__", None) + if not isinstance(instance_dict, dict): + break + for attribute in ("instance", "initial_data", "_data", "_validated_data"): + if attribute in instance_dict: + instance_dict[attribute] = None + if "_args" in instance_dict: + instance_dict["_args"] = () + for attribute in ("_kwargs", "_context"): + if attribute in instance_dict: + instance_dict[attribute] = {} + current = instance_dict.get("child") + + +class BaserowJSONRenderer(JSONRenderer): + """ + DRF creates several reference cycles for every request (see + https://github.com/encode/django-rest-framework/issues/7250, acknowledged + upstream but never fixed): + + - `Serializer.data` returns a `ReturnList`/`ReturnDict` with a `serializer` + backreference, while the serializer keeps the same object in `_data`. + - `Field.bind` sets `field.parent`, and `BindingDict` keeps a `serializer` + backreference, so a serializer and its fields always form cycles, pinning + `serializer.instance` and the `_args`/`_kwargs` stashed by `Field.__new__` + (which contain the full page of fetched model instances). + - `APIView.dispatch` sets `view.response`, and `finalize_response` puts the + view, request, and the response itself into `response.renderer_context`, + so the response, the rendered content in `response._container`, and the + request always form cycles. + + Because of those cycles, the per-request object graph (fetched rows, serialized + data, rendered bytes) can never be freed by reference counting; it lingers until + Python's cyclic garbage collector runs a full pass, which is allocation-triggered + and therefore never happens on an idle worker. For large responses that looks like + a memory leak. + + The `serializer` backreference and the renderer context exist purely so that + renderers can inspect them while rendering. When this renderer produces the final + response it is their last consumer, so after producing the bytes it severs these + references, making the payload reference-count collectable the moment the handler + drops the response. The cleanup is skipped when another renderer (e.g. the + browsable API) delegates to this render and still needs the context afterwards. + """ + + def render(self, data, accepted_media_type=None, renderer_context=None): + rendered = super().render(data, accepted_media_type, renderer_context) + + response = ( + renderer_context.get("response") + if isinstance(renderer_context, dict) + else None + ) + if response is None or getattr(response, "accepted_renderer", None) is not self: + return rendered + + for serializer in _iter_serializer_outputs(data): + _release_serializer_references(serializer) + + view = renderer_context.get("view") + if view is not None: + view.response = None + view.request = None + view.args = () + view.kwargs = {} + renderer_context.clear() + response.renderer_context = None + + return rendered diff --git a/backend/src/baserow/config/asgi.py b/backend/src/baserow/config/asgi.py index 25fb61a308..cd9409edaa 100644 --- a/backend/src/baserow/config/asgi.py +++ b/backend/src/baserow/config/asgi.py @@ -1,10 +1,11 @@ +import django from django.conf import settings -from django.core.asgi import get_asgi_application from django.urls import re_path from channels.routing import ProtocolTypeRouter, URLRouter from baserow.config.helpers import ( + BaserowASGIHandler, ConcurrencyLimiterASGI, check_lazy_loaded_libraries, log_env_warnings, @@ -16,7 +17,10 @@ # The telemetry instrumentation library setup needs to run prior to django's setup. setup_telemetry(add_django_instrumentation=True) -django_asgi_app = get_asgi_application() +# Same as django.core.asgi.get_asgi_application, but with Baserow's ASGI handler that +# doesn't keep per-request memory alive in reference cycles. +django.setup(set_prefix=False) +django_asgi_app = BaserowASGIHandler() # Check that libraries meant to be lazy-loaded haven't been imported at startup. # This runs after Django is fully loaded, so it catches imports from all apps. diff --git a/backend/src/baserow/config/helpers.py b/backend/src/baserow/config/helpers.py index 3307cb9e3f..d092d860a3 100644 --- a/backend/src/baserow/config/helpers.py +++ b/backend/src/baserow/config/helpers.py @@ -3,6 +3,7 @@ import sys from django.conf import settings +from django.core.handlers.asgi import ASGIHandler from loguru import logger @@ -58,6 +59,30 @@ async def __aexit__(self, exc_type, exc, traceback): pass +class BaserowASGIHandler(ASGIHandler): + """ + Django's disconnect watcher raises `RequestAborted`, whose traceback ends up in a + reference cycle with the `handle` frame that pins the request and response in + memory until a full garbage collection pass. On Django 5.2 completing the watcher + normally is behavior-identical (`handle` ignores the result and cancels the + in-flight request via `asyncio.wait`), and no cycle is created. + + Streaming responses are not covered: on mid-stream disconnect an iterator that + references the view can still keep the response in a cycle. + + TODO: re-evaluate before Django >= 6.1, where only a raising watcher aborts the + in-flight request. `test_handle_cancels_in_flight_request_on_disconnect` fails + loudly in that case. + """ + + async def listen_for_disconnect(self, receive): + message = await receive() + if message["type"] == "http.disconnect": + return + # This should never happen. + assert False, "Invalid ASGI message after request body: %s" % message["type"] + + class ConcurrencyLimiterASGI: """ Helper wrapper on ASGI app to limit the number of requests handled diff --git a/backend/src/baserow/config/settings/base.py b/backend/src/baserow/config/settings/base.py index dff3b052f4..c994e0fe15 100644 --- a/backend/src/baserow/config/settings/base.py +++ b/backend/src/baserow/config/settings/base.py @@ -404,7 +404,7 @@ "baserow.api.user_sources.authentication.UserSourceJSONWebTokenAuthentication", "baserow.api.authentication.JSONWebTokenAuthentication", ), - "DEFAULT_RENDERER_CLASSES": ("rest_framework.renderers.JSONRenderer",), + "DEFAULT_RENDERER_CLASSES": ("baserow.api.renderers.BaserowJSONRenderer",), "DEFAULT_SCHEMA_CLASS": "baserow.api.openapi.AutoSchema", } diff --git a/backend/tests/baserow/api/test_renderers.py b/backend/tests/baserow/api/test_renderers.py new file mode 100644 index 0000000000..6d51a00a0c --- /dev/null +++ b/backend/tests/baserow/api/test_renderers.py @@ -0,0 +1,146 @@ +import gc +import weakref + +from django.shortcuts import reverse + +import pytest +from rest_framework import serializers +from rest_framework.renderers import BrowsableAPIRenderer +from rest_framework.status import HTTP_200_OK + +from baserow.api.renderers import BaserowJSONRenderer + + +@pytest.mark.django_db +def test_rendered_response_graph_is_reference_count_collectable( + api_client, data_fixture +): + """ + The BaserowJSONRenderer must sever the DRF reference cycles after rendering, + so that the whole per-request payload (fetched rows, serialized data, rendered + content) is freed by reference counting alone, without depending on a cyclic + garbage collection pass. This is what keeps the worker memory flat when large + values are listed. + """ + + user, jwt_token = data_fixture.create_user_and_token() + table = data_fixture.create_database_table(user=user) + field = data_fixture.create_text_field(table=table, name="Notes") + table.get_model().objects.create(**{field.db_column: "value"}) + + url = reverse("api:database:rows:list", kwargs={"table_id": table.id}) + + gc.collect() + gc.disable() + try: + response = api_client.get(url, HTTP_AUTHORIZATION=f"JWT {jwt_token}") + assert response.status_code == HTTP_200_OK + + # The renderer must have cleared the cyclic renderer context, while the + # public response API stays usable. + assert response.renderer_context is None + results = response.data["results"] + assert results[0][f"field_{field.id}"] == "value" + + # The serializer graph must no longer pin the fetched rows or the + # serialized output. + serializer = results.serializer + assert serializer.instance is None + assert serializer._data is None + assert serializer.child._args == () + + # The Django test client attaches `response.json = partial(..., + # response)`, a self-cycle that only exists in tests, not in the real + # request path, and would mask the behavior being verified here. + response.__dict__.pop("json", None) + + response_ref = weakref.ref(response) + results_ref = weakref.ref(results) + del response, results, serializer + + # The garbage collector is disabled, so these can only be dead if the + # object graph was freed by pure reference counting. + assert response_ref() is None + assert results_ref() is None + finally: + gc.enable() + + +def test_renderer_skips_cleanup_when_another_renderer_delegates_to_it(): + class FakeView: + response = "sentinel-response" + request = "sentinel-request" + + class FakeResponse: + accepted_renderer = BrowsableAPIRenderer() + + view = FakeView() + response = FakeResponse() + renderer_context = {"view": view, "request": "request", "response": response} + + BaserowJSONRenderer().render({"a": 1}, "application/json", renderer_context) + + assert renderer_context["request"] == "request" + assert view.response == "sentinel-response" + assert view.request == "sentinel-request" + + +class _NameSerializer(serializers.Serializer): + name = serializers.CharField() + + +@pytest.mark.parametrize( + "wrap_data", + [ + # Serializer output inside an ordinary list, e.g. the job and + # application APIs. + lambda data: [data], + # The kanban/calendar grouped-row shape. + lambda data: {"rows": {"1": {"count": 1, "results": data}}}, + ], +) +def test_renderer_releases_nested_serializer_outputs(wrap_data): + serializer = _NameSerializer(instance=[{"name": "a"}], many=True) + data = serializer.data + + renderer = BaserowJSONRenderer() + + class FakeResponse: + accepted_renderer = renderer + + renderer.render( + wrap_data(data), + "application/json", + {"response": FakeResponse()}, + ) + + assert serializer.instance is None + assert serializer._data is None + assert serializer.child._args == () + + +def test_renderer_releases_serializer_context(): + class Marker: + pass + + marker = Marker() + serializer = _NameSerializer( + instance=[{"name": "a"}], many=True, context={"marker": marker} + ) + data = serializer.data + + renderer = BaserowJSONRenderer() + + class FakeResponse: + accepted_renderer = renderer + + renderer.render(data, "application/json", {"response": FakeResponse()}) + + marker_ref = weakref.ref(marker) + gc.disable() + try: + del marker + # Only dead without gc if the serializer no longer holds the context. + assert marker_ref() is None + finally: + gc.enable() diff --git a/backend/tests/baserow/config/test_asgi_handler.py b/backend/tests/baserow/config/test_asgi_handler.py new file mode 100644 index 0000000000..a2e757fdec --- /dev/null +++ b/backend/tests/baserow/config/test_asgi_handler.py @@ -0,0 +1,87 @@ +import asyncio + +import pytest + +from baserow.config.helpers import BaserowASGIHandler + + +@pytest.mark.asyncio +async def test_listen_for_disconnect_returns_instead_of_raising(): + handler = BaserowASGIHandler.__new__(BaserowASGIHandler) + + async def receive(): + return {"type": "http.disconnect"} + + assert await handler.listen_for_disconnect(receive) is None + + +@pytest.mark.asyncio +async def test_listen_for_disconnect_still_asserts_on_invalid_message(): + handler = BaserowASGIHandler.__new__(BaserowASGIHandler) + + async def receive(): + return {"type": "http.unexpected"} + + with pytest.raises(AssertionError): + await handler.listen_for_disconnect(receive) + + +@pytest.mark.asyncio +async def test_handle_cancels_in_flight_request_on_disconnect(): + """ + With a disconnect listener that completes normally instead of raising, `handle()` + must still cancel the in-flight request on disconnect. This holds on Django 5.2 but + not on Django >= 6.1 (TaskGroup based), where only a raising listener aborts the + request. If this test fails after a Django upgrade, the `listen_for_disconnect` + override must be reworked. + """ + + handler = BaserowASGIHandler() + + request_started = asyncio.Event() + request_cancelled = False + + async def fake_run_get_response(request): + nonlocal request_cancelled + request_started.set() + try: + await asyncio.sleep(30) + except asyncio.CancelledError: + request_cancelled = True + raise + + handler.run_get_response = fake_run_get_response + + body_messages = [{"type": "http.request", "body": b"", "more_body": False}] + + async def receive(): + if body_messages: + return body_messages.pop(0) + # Only disconnect once the request is actually in flight, so the cancellation + # path is deterministically exercised. + await request_started.wait() + return {"type": "http.disconnect"} + + sent_messages = [] + + async def send(message): + sent_messages.append(message) + + scope = { + "type": "http", + "http_version": "1.1", + "method": "GET", + "path": "/", + "raw_path": b"/", + "query_string": b"", + "headers": [], + "server": ("testserver", 80), + "client": ("127.0.0.1", 1000), + "scheme": "http", + } + + await asyncio.wait_for(handler(scope, receive, send), timeout=10) + + assert request_cancelled is True + # The request never completed, so nothing must have been sent. + assert sent_messages == [] diff --git a/changelog/entries/unreleased/bug/reduces_backend_memory_usage_by_releasing_each_api_responses.json b/changelog/entries/unreleased/bug/reduces_backend_memory_usage_by_releasing_each_api_responses.json new file mode 100644 index 0000000000..7cbd9b672e --- /dev/null +++ b/changelog/entries/unreleased/bug/reduces_backend_memory_usage_by_releasing_each_api_responses.json @@ -0,0 +1,9 @@ +{ + "type": "bug", + "message": "Reduce backend memory usage by releasing each API response's memory right after it is sent", + "issue_origin": "github", + "issue_number": null, + "domain": "core", + "bullet_points": [], + "created_at": "2026-07-16" +}