Opened 9 years ago

Closed 9 years ago

#25329 closed Bug (fixed)

_nodb_connection is open for an entire test suite run

Reported by: Matthew Taylor Owned by: nobody
Component: Testing framework Version: dev
Severity: Normal Keywords: nodb, testing, test runner
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I was running into an issue with my test suite in which I would get occasionally get an OperationalError: (2006, 'MySQL server has gone away' error during test database teardown – i.e. during connection.creation.destroy_test_db(old_name, self.verbosity, self.keepdb) in the test runner.

I would often run into this when running the entire test suite, or tests against multiple apps in my project – running tests against just one app, or one unit test, would not cause this tear down issue. Upon investigation, I discovered that by setting wait_timeout on my MySQL instance high (it had been set to 180 seconds, so I tried setting it back to the default 28800 and running tests again), I was able to prevent the error from occurring on runs of the entire test suite, or multi-app tests.

After a bit of investigation, it appeared to me that part of the issue I was experiencing was that the _nodb_connection property on the BaseDatabaseCreation object was being set on instantiation, and then kept alive for the duration of the tests. If my understanding is indeed the case, I was wondering if it might be possible/better to have this connection be closed after test database creation (i.e. in create_test_db()), and reopened again for tear down, rather than being kept open for the entire test duration.

Change History (7)

comment:1 by Josh Smeaton, 9 years ago

Triage Stage: UnreviewedAccepted
Type: UncategorizedBug
Version: 1.8master

I've run into this same issue when running the test suite. It usually happens when I'm debugging and have a set_trace waiting around for awhile. I assumed it was the regular database connections used throughout the tests that were going away though.

How certain are you that the nodb connection is to blame?

I think that if the nodb connection can be safely closed, then it should be closed. It can be created again to tear down the test database if it's needed. Is the nodb connection used after test database creation other than for tear down?

in reply to:  1 comment:2 by Matthew Taylor, 9 years ago

Replying to jarshwah:

How certain are you that the nodb connection is to blame?

I'm relatively certain – I also ran a trace on the tear down method to verify that it was looking at the right name for the test database, and it was. When I investigated the source of BaseDatabaseCreation further I saw that both _create_test_db() and _destroy_test_db() use a nodb connection – for create, it uses the line with self._nodb_connection.cursor() as cursor, whereas for destroy, it uses the line with self.connection._nodb_connection.cursor() as cursor.

I believe they are retrieving the same connection each time, because even though create uses the property on BaseDatabaseCreation itself, that property appears to return self.connection._nodb_connection (see here). And, since self.connection is set on init, that was what made me think it was possible that the connection was being kept open for the entire time. Also it appears to be a cached_property on the connection object too.

I think that if the nodb connection can be safely closed, then it should be closed. It can be created again to tear down the test database if it's needed. Is the nodb connection used after test database creation other than for tear down?

I'm not 100% sure. Inside of BaseDatabaseCreation it doesn't appear to be used anywhere else. A cursory search of the repo didn't reveal it being used elsewhere, though.

comment:3 by Adam Johnson, 9 years ago

I can replicate this on 1.8.4:

$ mysql -uroot
...
mysql> set @@global.wait_timeout=10;
Query OK, 0 rows affected (0.00 sec)

mysql> Bye
$ runtests  # 30 seconds 
...
  File "/opt/yplan/venv/local/lib/python2.7/site-packages/django/db/backends/base/creation.py", line 503, in destroy_test_db
    self._destroy_test_db(test_database_name, verbosity)
  File "/opt/yplan/venv/local/lib/python2.7/site-packages/django/db/backends/base/creation.py", line 521, in _destroy_test_db
    % self.connection.ops.quote_name(test_database_name))
  File "/opt/yplan/venv/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "/opt/yplan/venv/local/lib/python2.7/site-packages/django/db/utils.py", line 97, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/opt/yplan/venv/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 62, in execute
    return self.cursor.execute(sql)
  File "/opt/yplan/venv/local/lib/python2.7/site-packages/django_mysql/monkey_patches.py", line 42, in execute
    return orig_execute(self, sql, args)
  File "/opt/yplan/venv/local/lib/python2.7/site-packages/django/db/backends/mysql/base.py", line 124, in execute
    return self.cursor.execute(query, args)
  File "/opt/yplan/venv/local/lib/python2.7/site-packages/MySQLdb/cursors.py", line 220, in execute
    self.errorhandler(self, exc, value)
  File "/opt/yplan/venv/local/lib/python2.7/site-packages/MySQLdb/connections.py", line 36, in defaulterrorhandler
    raise errorvalue
django.db.utils.OperationalError: (2006, 'MySQL server has gone away')

It definitely is _nodb_connection, since _destroy_test_db only connects to that database:

def _destroy_test_db(self, test_database_name, verbosity):
        """
        Internal implementation - remove the test db tables.
        """
        # Remove the test database to clean up after
        # ourselves. Connect to the previous database (not the test database)
        # to do so, because it's not allowed to delete a database while being
        # connected to it.
        with self.connection._nodb_connection.cursor() as cursor:

I think _nodb_connection should reconnect everytime you access it, rather than it being a cached_property, which seems like a premature optimization to me.

Version 0, edited 9 years ago by Adam Johnson (next)

comment:4 by Adam Johnson, 9 years ago

Has patch: set

comment:5 by Tim Graham, 9 years ago

Patch needs improvement: set

Still a test failure on the patch.

comment:6 by Tim Graham, 9 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

Claude, any concerns?

comment:7 by Claude Paroz <claude@…>, 9 years ago

Resolution: fixed
Status: newclosed

In b2f6e42:

Fixed #25329 -- Prevented _nodb_connection from being left open

Note: See TracTickets for help on using tickets.
Back to Top