Improve database check performance: replace introspection.table_names() by a simple cursor query

[cristianemoyano]

Summary

The introspection.table_names() method in Django could produce a poor performance in large databases.

Take a look at the different backends implementations:

• https://github.com/django/django/blob/0b95a96ee10d3e12aef01d449467bcf4641286b4/django/db/backends/mysql/introspection.py#L67
• https://github.com/django/django/blob/0b95a96ee10d3e12aef01d449467bcf4641286b4/django/db/backends/sqlite3/introspection.py#L70
• https://github.com/django/django/blob/0b95a96ee10d3e12aef01d449467bcf4641286b4/django/db/backends/postgresql/introspection.py#L47
• https://github.com/django/django/blob/0b95a96ee10d3e12aef01d449467bcf4641286b4/django/db/backends/oracle/introspection.py#L72

That's why I suggest executing a cheap query: SELECT 1 to test the database connection.

Testing

python runtests.py tests/test_decorators.py:TestDBConnection

(django-watchman) ➜  django-watchman git:(improve-database-check-performance) ✗ python runtests.py
nosetests tests -s --verbosity=1
Creating test database for alias 'default'...
...............................S............................
----------------------------------------------------------------------
Ran 60 tests in 0.158s

OK (SKIP=1)
Destroying test database for alias 'default'...

[mwarkentin]

@cristianemoyano Thanks for opening this PR! Interesting, I hadn't realized this could be an issue. Is it something you've run into yourself? Just curious what sort of performance you were seeing on the table_names call?

[cristianemoyano]

@cristianemoyano Thanks for opening this PR! Interesting, I hadn't realized this could be an issue. Is it something you've run into yourself? Just curious what sort of performance you were seeing on the table_names call?

Hey @mwarkentin,

You're welcome. Yes, we realized that running this health check many times in production results in database degradation. Clearly running "select 1" is much cheaper. Unfortunately, I can't show you metrics. I also did a "python timeit" comparing both approaches and my approach turned out to be 6x faster.

Another reason is that the purpose of this health check is to establish a connection with the database and instead the method is also returning the name of the tables which is unnecessary.

[mwarkentin]

@cristianemoyano I'm doing some work to modernize the testing and release processes for django-watchman at the moment - hope to get this in and released when that stabilizes.

[cristianemoyano]

@cristianemoyano I'm doing some work to modernize the testing and release processes for django-watchman at the moment - hope to get this in and released when that stabilizes.

@mwarkentin that sounds Great! let me know if you need anything from my side.
Happy Holidays!

[mwarkentin]

@cristianemoyano looks like the added test is failing - can you take a look? Was it working for you locally?

https://github.com/mwarkentin/django-watchman/runs/4619579103?check_suite_focus=true

[cristianemoyano]

@cristianemoyano looks like the added test is failing - can you take a look? Was it working for you locally?

https://github.com/mwarkentin/django-watchman/runs/4619579103?check_suite_focus=true

That's weird. I will try again rebasing my code later this week.

GitHub

black + isort fmt · mwarkentin/django-watchman@217a80d

django-watchman exposes a status endpoint for your backing services like databases, caches, etc. - black + isort fmt · 217a80d

[cristianemoyano]

Hey @mwarkentin, can you try again, please? Thanks!

[cristianemoyano]

All checks are green! 🎉

[mwarkentin]

@cristianemoyano thanks! I'll take a look next week. 😊

[mwarkentin]

LGTM - thanks again! I'll update on this PR when I've released to pypi.