fix django.db.backends import in patch_thread_ident

[mx14]

BaseDatabaseWrapper and DatabaseError were moved to django.db.backends.base.base package since django 1.8.

django/django@2830807#diff-53fcf3ac0535307033e0cfabb85c5301

[vytisb]

I am somewhat worried about this fix, since the original fix from gunicorn is still there, unmodified. And I assume gunicorn has more users than django-celery.

Have you actually reproduced the error and then seen it fixed after this PR?

As a side-note, I fail to understand what this monkey-patching actually does. It seems to replace thread.get_ident with thread.get_ident...

[mx14]

yes, actually reproduced, and it works for me. see the issue #402

[vytisb]

This import doesn't work on Python 3. You should also try to import _thread for this patch to work on py3.

[mx14]

fixed. thanks.

[hheimbuerger]

I've had the same issue with djcelery in Django 1.9.x and manually applying these changes locally has fixed it for me.

As for gunicorn, benoitc/gunicorn#927 looks related (still open as of right now).

[hheimbuerger]

Really strange. This works fine on Windows, but as soon as I deploy it to Linux (Heroku), the original exception is back.

[hheimbuerger]

So the problem still occurs with this patch with the pool class celery.concurrency.prefork:TaskPool. (It seems to fix the issue for celery.concurrency.threads:TaskPool, which is what previously led me to believe that everything works.)

[vytisb]

@hheimbuerger, can you check if the problem persists when running without django-celery? If so, I would suggest filing a bug with Celery.

[hheimbuerger]

@vytisb I cannot really check, unfortunately, as my project heavily depends on django-celery.

Or do you mean just without the Django ORM queue?

[hheimbuerger]

@mx14 @vytisb Can one of you add some more detail to what's going on here? I've been trying to understand what this code does and what causes the problems with Django 1.9, but I couldn't really find any statement of intent besides this cryptic note:

This patch make sure that we use real threads to get the ident which is going to happen if we are using gevent or eventlet.

What are 'real threads'? Which 'unreal threads' is Django using when it isn't patched?

It seems to want to overwrite the BaseDatabaseWrapper's constructor, but I don't really understand why. The local variable _thread_ident is overwritten, but the original constructor would have already set it anyway (in exactly the same way, it appears), so why do that?

For mysterious reasons, it also seems to overwrite the method validate_thread_sharing() with an implementation that is almost identical (the boolean condition is slightly rephrased, but should be functionally identical).

I've also done some debugging, and in my case (prefork), BaseDatabaseWrapper is always constructed even before the patch is applied. So then the patched constructor is never run. That's probably why I'm having this problem in the first place.

The reason is that the patch is applied as part of the manage.py celery command code (the import in djcelery/management/command/celery.py:6), but the BaseDatabaseWrapper is constructed by the call to django.setup() much earlier (during the first model's initialization).

[vytisb]

@hheimbuerger, I've looked into this patch and I believe I can explain it.

First, gevent/eventlet use an event based execution model where a bunch of micro threads are running in a single real OS thread. These libs also monkey patch various python system utilities to make code that was not explicitly adapted work with their model. thread.get_ident() gets patched to return the micro thread id instead of the real thread id since that would be the same for all micro threads.

Now, patch_thread_ident() takes the real get_ident function, and monkey-patches BaseDatabaseWrapper.validate_thread_sharing to use that instead of the gevent/eventlet monkey-patched one. This basically disables the thread sharing validation. In doing so, it doesn't solve the problem, but rather hides the symptoms.

Therefore, I think that this PR should not be merged and instead patch_thread_ident should be completely removed.
The real problem may lie somewhere in the way Celery handles workers.

All that said, this should have no effect when not using gevent/eventlet.

@vytisb I cannot really check, unfortunately, as my project heavily depends on django-celery.

Or do you mean just without the Django ORM queue?
I wanted to try to completely eliminate django-celery from the equation.

Are you using DjangoLoader (djcelery.setup_loader()) or a Celery app object (with automatic DjangoFixup)? I've seen some code differences between the two that might be relevant.

[hheimbuerger]

@vytisb Thanks a lot for your detailled explanation!

So basically, you're saying this problem shouldn't even occur when using the prefork model? When I was debugging, I thought I had seen the BaseDatabaseWrapper be created on the main thread and then attempted to be reused after the workers are forked off, raising this exception. Then again, I don't claim to fully understand how my debugger (PyCharm) displays all of these concepts. In fact, I don't see any new (OS-level) processes even with prefork.

I'm using a Celery app object, which I configure with config_from_object('django.conf:settings') and autodiscover_tasks(lambda: settings.INSTALLED_APPS). I wasn't aware of any fixups, so I must assume that they are being done as normally, if they are automatic.

Is there any documentation or other reading material to better understand all of these fixups: what they do and why, and how they are initialized in the different cases and the various versions of Django they support?

[vytisb]

@hheimbuerger

So basically, you're saying this problem shouldn't even occur when using the prefork model?

No, patch_thread_ident doesn't do anything with prefork model. The underlying problem of mishandled BaseDatabaseWrapper still exists.

Is there any documentation or other reading material to better understand all of these fixups

There isn't any documentation, AFAIK. I found out about DjangoFixup by reading the code. It does the Django and Celery integration that DjangoLoader used to do. It is applied automatically unless you explicitly disable it, so no worries there.

I'm using a Celery app object

With that, I think, the discussion should move to celery/celery#3520 since the problem should be outside django-celery.

[hheimbuerger]

@vytisb You're right, thanks so much for your responses! I've since identified another library as the culprit (which ran even more gevent patching in my non-gevent environment), so my apologies for bringing confusing details to this unrelated PR.

I'll go into more detail on celery/celery#3520.