-
Notifications
You must be signed in to change notification settings - Fork 451
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds automatic reconnection logic for Celery snapshot state handling when the Django database connection is lost.
- Imports
connection
andInterfaceError
from Django. - Wraps worker and task handling in a
try/except
to catch a closed connection. - Closes the connection and logs a message so Django’s connection pooling can reconnect.
Comments suppressed due to low confidence (1)
djcelery/snapshot.py:141
- Ensure that a
logger
instance is defined or imported in this module before usinglogger.info()
, otherwise this will raise aNameError
at runtime.
logger.info(
except InterfaceError as e: | ||
# When connection already closed exception is raised, | ||
# force to close connection and Django will automatically reconnect | ||
if str(e) == 'connection already closed': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comparing exception message strings is brittle; consider catching a more specific exception subclass or inspecting error attributes instead of matching on str(e)
.
if str(e) == 'connection already closed': | |
if getattr(e, 'args', None) and 'connection already closed' in e.args[0]: |
Copilot uses AI. Check for mistakes.
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 str(e) == 'connection already closed': | ||
connection.close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The try/except
block wraps both worker and task handling, which may swallow unrelated errors. Consider narrowing the scope to only the database operations that need reconnection.
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 str(e) == 'connection already closed': | |
connection.close() | |
for worker in state.workers.items(): | |
self.handle_worker(worker) | |
_handle_tasks() | |
try: | |
# Check and handle database reconnection if needed | |
connection.close() | |
except InterfaceError as e: | |
# When connection already closed exception is raised, | |
# force to close connection and Django will automatically reconnect | |
if str(e) == 'connection already closed': |
Copilot uses AI. Check for mistakes.
Currently if the database is not stable and connection is lost, celerycam can not reconnect db connection automatically and it can not work properly anymore. The error
connection already closed
will always be raised.This PR supports to automatically reconnect db connection for celerycam if db connection is lost.