From cfcca7ccce3dc527d16757ff6dc978e50c4a2e61 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Sat, 7 Jun 2014 14:09:27 +0200 Subject: [PATCH] Fixed #3711, #6734, #12581 -- Bounded connection.queries. Prevented unlimited memory consumption when running background tasks with DEBUG=True. Thanks Rob, Alex, Baptiste, and others. --- django/db/__init__.py | 2 +- django/db/backends/__init__.py | 17 ++++++++-- django/db/backends/utils.py | 4 +-- django/test/utils.py | 4 +-- docs/faq/models.txt | 25 ++++---------- docs/releases/1.8.txt | 5 ++- tests/backends/tests.py | 62 ++++++++++++++++++++++++++++++++-- tests/bulk_create/tests.py | 8 ++--- 8 files changed, 94 insertions(+), 33 deletions(-) diff --git a/django/db/__init__.py b/django/db/__init__.py index 84ef6e1400..a303a2c150 100644 --- a/django/db/__init__.py +++ b/django/db/__init__.py @@ -52,7 +52,7 @@ connection = DefaultConnectionProxy() # Register an event to reset saved queries when a Django request is started. def reset_queries(**kwargs): for conn in connections.all(): - conn.queries = [] + conn.queries_log.clear() signals.request_started.connect(reset_queries) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index bcc3979e2b..587cd874d9 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -1,3 +1,4 @@ +from collections import deque import datetime import time import warnings @@ -30,17 +31,21 @@ class BaseDatabaseWrapper(object): ops = None vendor = 'unknown' + queries_limit = 9000 + def __init__(self, settings_dict, alias=DEFAULT_DB_ALIAS, allow_thread_sharing=False): # Connection related attributes. + # The underlying database connection. self.connection = None - self.queries = [] # `settings_dict` should be a dictionary containing keys such as # NAME, USER, etc. It's called `settings_dict` instead of `settings` # to disambiguate it from Django settings modules. self.settings_dict = settings_dict self.alias = alias + # Query logging in debug mode. self.use_debug_cursor = None + self.queries_log = deque(maxlen=self.queries_limit) # Transaction related attributes. # Tracks if the connection is in autocommit mode. Per PEP 249, by @@ -79,6 +84,14 @@ class BaseDatabaseWrapper(object): def __hash__(self): return hash(self.alias) + @property + def queries(self): + if len(self.queries_log) == self.queries_log.maxlen: + warnings.warn( + "Limit for query logging exceeded, only the last {} queries " + "will be returned.".format(self.queries_log.maxlen)) + return list(self.queries_log) + ##### Backend-specific methods for creating connections and cursors ##### def get_connection_params(self): @@ -429,7 +442,7 @@ class BaseDatabaseWrapper(object): def make_debug_cursor(self, cursor): """ - Creates a cursor that logs all queries in self.queries. + Creates a cursor that logs all queries in self.queries_log. """ return utils.CursorDebugWrapper(cursor, self) diff --git a/django/db/backends/utils.py b/django/db/backends/utils.py index 8078af9f95..77132e3f25 100644 --- a/django/db/backends/utils.py +++ b/django/db/backends/utils.py @@ -80,7 +80,7 @@ class CursorDebugWrapper(CursorWrapper): stop = time() duration = stop - start sql = self.db.ops.last_executed_query(self.cursor, sql, params) - self.db.queries.append({ + self.db.queries_log.append({ 'sql': sql, 'time': "%.3f" % duration, }) @@ -99,7 +99,7 @@ class CursorDebugWrapper(CursorWrapper): times = len(param_list) except TypeError: # param_list could be an iterator times = '?' - self.db.queries.append({ + self.db.queries_log.append({ 'sql': '%s times: %s' % (times, sql), 'time': "%.3f" % duration, }) diff --git a/django/test/utils.py b/django/test/utils.py index fdbf920868..9a0d7a494c 100644 --- a/django/test/utils.py +++ b/django/test/utils.py @@ -509,7 +509,7 @@ class CaptureQueriesContext(object): def __enter__(self): self.use_debug_cursor = self.connection.use_debug_cursor self.connection.use_debug_cursor = True - self.initial_queries = len(self.connection.queries) + self.initial_queries = len(self.connection.queries_log) self.final_queries = None request_started.disconnect(reset_queries) return self @@ -519,7 +519,7 @@ class CaptureQueriesContext(object): request_started.connect(reset_queries) if exc_type is not None: return - self.final_queries = len(self.connection.queries) + self.final_queries = len(self.connection.queries_log) class IgnoreDeprecationWarningsMixin(object): diff --git a/docs/faq/models.txt b/docs/faq/models.txt index 94011d254f..57e3c8077f 100644 --- a/docs/faq/models.txt +++ b/docs/faq/models.txt @@ -32,6 +32,12 @@ same interface on each member of the ``connections`` dictionary:: >>> from django.db import connections >>> connections['my_db_alias'].queries +If you need to clear the query list manually at any point in your functions, +just call ``reset_queries()``, like this:: + + from django.db import reset_queries + reset_queries() + Can I use Django with a pre-existing database? ---------------------------------------------- @@ -76,22 +82,3 @@ type, create an initial data file and put something like this in it:: As explained in the :ref:`SQL initial data file ` documentation, this SQL file can contain arbitrary SQL, so you can make any sorts of changes you need to make. - -Why is Django leaking memory? ------------------------------ - -Django isn't known to leak memory. If you find your Django processes are -allocating more and more memory, with no sign of releasing it, check to make -sure your :setting:`DEBUG` setting is set to ``False``. If :setting:`DEBUG` -is ``True``, then Django saves a copy of every SQL statement it has executed. - -(The queries are saved in ``django.db.connection.queries``. See -:ref:`faq-see-raw-sql-queries`.) - -To fix the problem, set :setting:`DEBUG` to ``False``. - -If you need to clear the query list manually at any point in your functions, -just call ``reset_queries()``, like this:: - - from django import db - db.reset_queries() diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt index 1345e55a9a..3e241aff27 100644 --- a/docs/releases/1.8.txt +++ b/docs/releases/1.8.txt @@ -170,7 +170,8 @@ Management Commands Models ^^^^^^ -* ... +* Django now logs at most 9000 queries in ``connections.queries``, in order + to prevent excessive memory usage in long-running processes in debug mode. Signals ^^^^^^^ @@ -263,6 +264,8 @@ Now, an error will be raised to prevent data loss:: Miscellaneous ~~~~~~~~~~~~~ +* ``connections.queries`` is now a read-only attribute. + * ``URLField.to_python`` no longer adds a trailing slash to pathless URLs. * ``django.contrib.gis`` dropped support for GEOS 3.1 and GDAL 1.6. diff --git a/tests/backends/tests.py b/tests/backends/tests.py index 85ca0bcf16..5ac0d53cb8 100644 --- a/tests/backends/tests.py +++ b/tests/backends/tests.py @@ -8,11 +8,13 @@ from decimal import Decimal import re import threading import unittest +import warnings from django.conf import settings from django.core.management.color import no_style from django.db import (connection, connections, DEFAULT_DB_ALIAS, - DatabaseError, IntegrityError, transaction) + DatabaseError, IntegrityError, reset_queries, transaction) +from django.db.backends import BaseDatabaseWrapper from django.db.backends.signals import connection_created from django.db.backends.postgresql_psycopg2 import version as pg_version from django.db.backends.utils import format_number, CursorWrapper @@ -630,7 +632,7 @@ class BackendTestCase(TestCase): # to use ProgrammingError). with self.assertRaises(connection.features.closed_cursor_error_class): # cursor should be closed, so no queries should be possible. - cursor.execute("select 1") + cursor.execute("SELECT 1" + connection.features.bare_select_suffix) @unittest.skipUnless(connection.vendor == 'postgresql', "Psycopg2 specific cursor.closed attribute needed") @@ -666,6 +668,62 @@ class BackendTestCase(TestCase): except Exception: pass + @override_settings(DEBUG=True) + def test_queries(self): + """ + Test the documented API of connection.queries. + """ + reset_queries() + + with connection.cursor() as cursor: + cursor.execute("SELECT 1" + connection.features.bare_select_suffix) + self.assertEqual(1, len(connection.queries)) + + self.assertIsInstance(connection.queries, list) + self.assertIsInstance(connection.queries[0], dict) + self.assertItemsEqual(connection.queries[0].keys(), ['sql', 'time']) + + reset_queries() + self.assertEqual(0, len(connection.queries)) + + + # Unfortunately with sqlite3 the in-memory test database cannot be closed. + @skipUnlessDBFeature('test_db_allows_multiple_connections') + @override_settings(DEBUG=True) + def test_queries_limit(self): + """ + Test that the backend doesn't store an unlimited number of queries. + + Regression for #12581. + """ + old_queries_limit = BaseDatabaseWrapper.queries_limit + BaseDatabaseWrapper.queries_limit = 3 + new_connections = ConnectionHandler(settings.DATABASES) + new_connection = new_connections[DEFAULT_DB_ALIAS] + + try: + with new_connection.cursor() as cursor: + cursor.execute("SELECT 1" + new_connection.features.bare_select_suffix) + cursor.execute("SELECT 2" + new_connection.features.bare_select_suffix) + + with warnings.catch_warnings(record=True) as w: + self.assertEqual(2, len(new_connection.queries)) + self.assertEqual(0, len(w)) + + with new_connection.cursor() as cursor: + cursor.execute("SELECT 3" + new_connection.features.bare_select_suffix) + cursor.execute("SELECT 4" + new_connection.features.bare_select_suffix) + + with warnings.catch_warnings(record=True) as w: + self.assertEqual(3, len(new_connection.queries)) + self.assertEqual(1, len(w)) + self.assertEqual(str(w[0].message), "Limit for query logging " + "exceeded, only the last 3 queries will be returned.") + + finally: + BaseDatabaseWrapper.queries_limit = old_queries_limit + new_connection.close() + # We don't make these tests conditional because that means we would need to # check and differentiate between: diff --git a/tests/bulk_create/tests.py b/tests/bulk_create/tests.py index 8648184223..f49fe4e54e 100644 --- a/tests/bulk_create/tests.py +++ b/tests/bulk_create/tests.py @@ -91,7 +91,7 @@ class BulkCreateTests(TestCase): def test_large_batch(self): with override_settings(DEBUG=True): - connection.queries = [] + connection.queries_log.clear() TwoFields.objects.bulk_create([ TwoFields(f1=i, f2=i + 1) for i in range(0, 1001) ]) @@ -112,7 +112,7 @@ class BulkCreateTests(TestCase): @skipUnlessDBFeature('has_bulk_insert') def test_large_batch_efficiency(self): with override_settings(DEBUG=True): - connection.queries = [] + connection.queries_log.clear() TwoFields.objects.bulk_create([ TwoFields(f1=i, f2=i + 1) for i in range(0, 1001) ]) @@ -124,7 +124,7 @@ class BulkCreateTests(TestCase): mixed together with objects without PK set. """ with override_settings(DEBUG=True): - connection.queries = [] + connection.queries_log.clear() TwoFields.objects.bulk_create([ TwoFields(id=i if i % 2 == 0 else None, f1=i, f2=i + 1) for i in range(100000, 101000)]) @@ -142,7 +142,7 @@ class BulkCreateTests(TestCase): mixed together with objects without PK set. """ with override_settings(DEBUG=True): - connection.queries = [] + connection.queries_log.clear() TwoFields.objects.bulk_create([ TwoFields(id=i if i % 2 == 0 else None, f1=i, f2=i + 1) for i in range(100000, 101000)])