Threading and database cursors

[svenseeberg]

In https://github.com/digitalfabrik/integreat-cms we're currently facing an issue with saving objects that contain links that should be checked. Users "frequently" run into response timeouts (after 1800s). At the same time we see quite a lot of long running linkcheck queries running in the database (>30s). This only happens when tests_running == False in the listeners.py. The long running queries disappear as soon as we set tests_running = True.

We concluded that we're facing some kind of issue with threading. As Apache2 mod_wsgi does have a GIL deadlock detection which is not jumping into action, we think that there must be another cause. Our next suspicion is a potentially shared database cursor between the WSGI process and the linkcheck worker thread.

The queries that are stuck are these:

SELECT "linkcheck_link"."id", "linkcheck_link"."content_type_id", "linkcheck_link"."object_id", "linkcheck_link"."field", "linkcheck_link"."url_id", "linkcheck_link"."text", "linkcheck_link"."ignore" FROM "linkcheck_link" WHERE ("linkcheck_link"."content_type_id" = 9 AND "linkcheck_link"."object_id" = 175527)

The query seems to originate in https://github.com/DjangoAdminHackers/django-linkcheck/blob/master/linkcheck/listeners.py#L80

https://www.psycopg.org/psycopg3/docs/advanced/async.html states the following:

Cursor objects are not thread-safe, and are not designed to be used by several threads at the same time.

We were not yet able to positively validate that the linkcheck thread uses its own dedicated database cursor. And a resource contention with the database cursor would fully explain the issues we're facing.

[svenseeberg]

We're now quite confident that the worker thread uses the same database connection as the WSGI process. The worker is started in https://github.com/DjangoAdminHackers/django-linkcheck/blob/master/linkcheck/listeners.py#L121 and does not have any dedicated database cursor handling. It is therefore likely that it uses the existing one as would be the case when simply importing models.

[svenseeberg]

We would like to create a pull request that optionally moves the execution of do_check_instance_links() into a Celery worker, if that is welcome.

[claudep]

It is surely welcome! It should not bring a hard dependency on Celery, though.

[svenseeberg]

It should not bring a hard dependency on Celery,

Fully agreed 👍

[svenseeberg]

We suggest the following implementation:

• Add a file linkcheck_celery.py which has a do_check_instance_link_ids() method with a @shared_task decorator (to be clarified: is the decorator actually needed?)
• Add a settings.py option LINKCHECK_IN_CELERY
• If LINKCHECK_IN_CELERY is set to true, do_check_instance_link_ids() will be imported in the listeners.py
• If LINKCHECK_IN_CELERY == True, add an else if statement to https://github.com/DjangoAdminHackers/django-linkcheck/blob/master/linkcheck/listeners.py#L113-L121 wich does call do_check_instance_link_ids() with apply_async()
• Move the do_check_instance_links() out of the check_instance_links() to make it reusable for the celery shared task (it seems there would be no harm done doing this)

However, the basic problem with threading would continue to exist and should also be addressed as well at some point. Sadly, we did not (yet?) find a good solution to execute the threaded function with a dedicated database cursor with the Django ORM.