Skip to content

Support celerycam for reconnecting db connenction if db connection is lost accidentally #550

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions djcelery/snapshot.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
from datetime import timedelta

from django.conf import settings
from django.db import connection
from django.db.utils import InterfaceError

from celery import states
from celery.events.state import Task
Expand Down Expand Up @@ -127,9 +129,18 @@ def _handle_tasks():
for i, task in enumerate(state.tasks.items()):
self.handle_task(task)

for worker in state.workers.items():
self.handle_worker(worker)
_handle_tasks()
try:
for worker in state.workers.items():
self.handle_worker(worker)
_handle_tasks()
except InterfaceError as e:
# When connection already closed exception is raised,
# force to close connection and Django will automatically reconnect
if getattr(e, 'args', None) and 'connection already closed' in e.args[0]:
connection.close()
Comment on lines +132 to +140
Copy link
Preview

Copilot AI May 20, 2025

Choose a reason for hiding this comment

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

[nitpick] The try block currently wraps both worker and task loops, which may swallow unrelated InterfaceErrors. Consider narrowing its scope to only the database-close operation or isolating the catch to the specific call that can raise due to a closed connection.

Suggested change
try:
for worker in state.workers.items():
self.handle_worker(worker)
_handle_tasks()
except InterfaceError as e:
# When connection already closed exception is raised,
# force to close connection and Django will automatically reconnect
if getattr(e, 'args', None) and 'connection already closed' in e.args[0]:
connection.close()
for worker in state.workers.items():
self.handle_worker(worker)
_handle_tasks()
try:
# Force to close connection if "connection already closed" exception is raised
connection.close()
except InterfaceError as e:
if getattr(e, 'args', None) and 'connection already closed' in e.args[0]:

Copilot uses AI. Check for mistakes.

Copy link
Preview

Copilot AI May 20, 2025

Choose a reason for hiding this comment

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

Instead of calling connection.close() directly, use django.db.close_old_connections() to properly handle connection pooling and ensure thread safety according to Django's recommendations.

Suggested change
connection.close()
close_old_connections()

Copilot uses AI. Check for mistakes.

logger.info(
'Django db connection is closed and will reconnect'
)

def on_cleanup(self):
expired = (self.TaskState.objects.expire_by_states(states, expires)
Expand Down
Loading