Opened 4 years ago

Closed 3 years ago

#15782 closed Bug (fixed)

Runserver/runfcgi/startup with MySQL is unlike any other database

Reported by: toofishes Owned by: nobody
Component: Database layer (models, ORM) Version: 1.3
Severity: Normal Keywords:
Cc: georgedorn@… 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

Django provides a BaseDatabaseValidation class. The only database engine to subclass this is MySQL.

To reproduce:

$ django-admin.py foobar
$ cd foobar
<edit settings.py to use ENGINE "django.db.backends.mysql">
$ python2 manage.py runserver
Validating models...

Unhandled exception in thread started by <bound method Command.inner_run of <django.contrib.staticfiles.management.commands.runserver.Command object at 0x27f5950>>
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/django/core/management/commands/runserver.py", line 88, in inner_run
    self.validate(display_num_errors=True)
  File "/usr/lib/python2.7/site-packages/django/core/management/base.py", line 249, in validate
    num_errors = get_validation_errors(s, app)
  File "/usr/lib/python2.7/site-packages/django/core/management/validation.py", line 103, in get_validation_errors
    connection.validation.validate_field(e, opts, f)
  File "/usr/lib/python2.7/site-packages/django/db/backends/mysql/validation.py", line 14, in validate_field
    db_version = self.connection.get_server_version()
  File "/usr/lib/python2.7/site-packages/django/db/backends/mysql/base.py", line 338, in get_server_version
    self.cursor()
  File "/usr/lib/python2.7/site-packages/django/db/backends/__init__.py", line 250, in cursor
    cursor = self.make_debug_cursor(self._cursor())
  File "/usr/lib/python2.7/site-packages/django/db/backends/mysql/base.py", line 322, in _cursor
    self.connection = Database.connect(**kwargs)
  File "/usr/lib/python2.7/site-packages/MySQLdb/__init__.py", line 81, in Connect
    return Connection(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/MySQLdb/connections.py", line 187, in __init__
    super(Connection, self).__init__(*args, **kwargs2)
_mysql_exceptions.OperationalError: (2002, "Can't connect to local MySQL server through socket '/var/run/mysqld/mysqld.sock' (2)")

Expected result:

$ django-admin.py foobar
$ cd foobar
<edit settings.py to use ENGINE "django.db.backends.sqlite3">
$ python2 manage.py runserver
Validating models...

0 errors found
Django version 1.3, using settings 'foobar.settings'
Development server is running at http://127.0.0.1:8000/
Quit the server with CONTROL-C.

Why this is a problem:

  1. The number one reason this is bad- no other database engine completely halts the Django project load if it is not up. It simply cannot service requests at all. All other database engines at least show a helpful 500 page on each failed request.
  2. Validation like this doesn't come early enough for someone that develops on one database engine and then switches to MySQL.
  3. The validation that needs a valid connection is testing an extreme edge case- those using versions before 5.0.3. From http://www.mysql.com/support/eol-notice.html, Extended Support for 4.1 and Active Support for 5.0 ended ended 2009-12-31.
  4. We are likely missing validation cases for other databases; we should be consistent in trying to do it right or not do it at all.
  5. The validation here isn't even correct for unique fields. max_length can be greater than 255, in reality the unique indexes are constrained to 1000 bytes. For that matter, this also does nothing to keep one from shooting themselves in the foot with a combo unique index produced by unique_together.

Potential fixes:

  1. Drop this database field validation completely, as only one database is even providing it and not even fully checking.
  2. Remove the bits referencing the connection; this will solve the most severe problem and those using old database versions should be well aware of the restrictions.
  3. Make the validation module do the right thing more often, fixing MySQL and adding it for other databases.

I'd be happy to provide a patch for #1 or #2, but wanted some feedback first.

Tangentially related to #7040.

Attachments (1)

skip-5-0-3-validation-if-db-down.patch (1.6 KB) - added by toofishes 3 years ago.
Updated to set db_version = None

Download all attachments as: .zip

Change History (9)

comment:1 Changed 4 years ago by aaugustin

  • Easy pickings unset
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

See #9431 which has a lengthy discussion of why and how the checks are performed.

comment:2 Changed 4 years ago by jacob

  • Triage Stage changed from Design decision needed to Accepted

Yeah, this shouldn't blow up. It should either display a nice error, or fail consistently (i.e. in the same way that other databases fail).

comment:3 Changed 4 years ago by toofishes

  • Has patch set

Since it wasn't clear whether you thought option 1, 2, 3, or none of the above was the best way to resolve this, I'll propose this patch which basically implements suggestion #2. If we can get a connection, we will continue to check the < 5.0.3 bits; else we will just skip it, do what we can without a connection, and make sure we start up. The first page you hit will then show a 500 error saying the DB connection could not be established, but one will not have to restart Django once the DB comes up.

comment:4 Changed 4 years ago by GDorn

  • Cc georgedorn@… added
  • UI/UX unset

comment:5 Changed 4 years ago by mtredinnick

  • Triage Stage changed from Accepted to Ready for checkin

This patch looks pretty much correct, although as stylistic point, I'd use db_version = None in the case where we can't determine it.

I apparently need to un-hose the MySQL install on my laptop, so I can't test this right now (I'd like to at least confirm it doesn't break existing recent MySQL versions) but it's effectively ready to commit.

comment:6 Changed 4 years ago by toofishes

It works with 5.5.15 locally, if that is recent enough.

I believe I did the odd (None * 3) triplet because of the error message formation, which can contain a database version, and I didn't feel like reworking that bit or something, as it was already quite messy. I'm fine with whatever you decide as long as it works!

comment:7 Changed 3 years ago by toofishes

Ugh, sorry about that last one- I forget to update the error message forming logic. The next version will have that fixed.

Changed 3 years ago by toofishes

Updated to set db_version = None

comment:8 Changed 3 years ago by claudep

  • Resolution set to fixed
  • Status changed from new to closed

In [17868]:

Fixed #15782 -- Prevented MySQL backend to crash on runserver when db server is down. Thanks toofishes for the report and patch.

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