Skip to content

Conversation

@Schweigi
Copy link
Contributor

@Schweigi Schweigi commented Sep 27, 2017

Fixes #4116

Celery doesn't re-use DB connections when the setting CONN_MAX_AGE is set in Django. Instead connections are closed immediately. This leads to a high load on the database when there are a lot of Celery workers. It also affects query performance negatively.

This PR uses the Django close_if_unusable_or_obsolete() method to close a DB connection instead of close().

close_if_unusable_or_obsolete() is available at least since Django 1.8 (see django/django@2830807) which is the minimum supported version for Celery 4.0.

Let me know if you need anything else to merge this.

Copy link
Contributor

@georgepsarakis georgepsarakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes seem valid to me, see also here and here.

@Schweigi
Copy link
Contributor Author

I would like to add that we use this fix successfully in production and hand no issues so far.

@thedrow thedrow merged commit c086cfd into celery:master Oct 1, 2017
@ograycode
Copy link

Are there any plans for a patch fix (4.1.x) that will include this?

@dralley
Copy link

dralley commented Mar 1, 2018

@thedrow Is a 4.1.1 coming soon? Are there any PRs blocking it?

@auvipy
Copy link
Member

auvipy commented Mar 1, 2018

4.2 is coming one blocker remain. check the milestone

snopoke added a commit to dimagi/commcare-cloud that referenced this pull request Nov 21, 2018
* setting is not respected by celery < v4.2
  * celery/celery#4292
* not compatible with 'gevent' gunicorn worker class
  * benoitc/gunicorn#996 (comment)
@auvipy
Copy link
Member

auvipy commented Nov 22, 2018

this change is creating regression as per reports, should we revert this?

@georgepsarakis
Copy link
Contributor

georgepsarakis commented Nov 24, 2018

@thedrow I read the issues linking here, I think that they mostly concern finding a correct value for CONN_MAX_AGE. I don't think we should revert, but I agree on the setting or better documentation.

@Chronial
Copy link

Chronial commented Apr 24, 2019

This PR actually broke celery's DB handling (see #5483). I would suggest a revert.

@auvipy
Copy link
Member

auvipy commented May 11, 2019

celery 4.x doesn't support Django-celery package

@auvipy
Copy link
Member

auvipy commented May 11, 2019

Please review this patch witch django-celery-beats and results

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Django celery fixup doesn't respect Django settings for PostgreSQL connections

6 participants