Improve database backend retry handling for stale SQLAlchemy connections by harshang03 · Pull Request #10122 · celery/celery

harshang03 · 2026-02-14T13:48:38Z

Summary

set safer SQLAlchemy result-backend defaults by enabling pool_pre_ping and a bounded pool_recycle window for database_engine_options
add retry-aware backend hooks so recoverable backend errors can trigger connection cleanup before retry sleeps
make DatabaseBackend treat SQLAlchemy connection/state errors as retryable and invalidate cached engine/session state on each retry path
cover the new behavior with backend retry tests and database session invalidation tests

Files changed

celery/app/defaults.py: set default database_engine_options pool health options
celery/backends/base.py: add on_backend_retryable_error() hook and call it in retry loops
celery/backends/database/__init__.py: classify retry-safe DB exceptions, invalidate backend sessions on retry, and merge engine options with safe defaults
celery/backends/database/session.py: add SessionManager.invalidate() to dispose cached engine/session entries
t/unit/backends/test_base.py: assert retry hook invocation for backend retry loops
t/unit/backends/test_database.py: add tests for default/override engine options, retry classification, and session invalidation

Test plan

.venv/bin/python -m pytest t/unit/backends/test_database.py t/unit/backends/test_base.py

Enable pool health defaults for the SQLAlchemy result backend and invalidate cached engines/sessions whenever retryable database errors occur so retries reconnect cleanly instead of reusing dead connections.

Copilot

Pull request overview

This PR hardens Celery’s SQLAlchemy database result backend against stale/invalid connections by setting safer engine defaults, adding a retry hook to clean up backend state before retry sleeps, and adding unit tests to cover the new behavior.

Changes:

Enable SQLAlchemy pool health options by default (pool_pre_ping=True, pool_recycle=3600) for the database backend configuration.
Add a generic on_backend_retryable_error() hook in BaseBackend and invoke it from retry loops to allow backends to clean up state before sleeping/retrying.
Extend the SQLAlchemy database backend to classify common SQLAlchemy connection/state errors as retryable and invalidate cached engine/session state on retry, with unit tests for these behaviors.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`celery/app/defaults.py`	Changes default `database_engine_options` to include pool health settings.
`celery/backends/base.py`	Adds and invokes `on_backend_retryable_error()` from backend retry loops.
`celery/backends/database/__init__.py`	Defines retryable SQLAlchemy errors, applies engine-option defaults/merging, and invalidates session state on retry.
`celery/backends/database/session.py`	Adds `SessionManager.invalidate()` to drop cached engine/session state and dispose the engine.
`t/unit/backends/test_base.py`	Adds assertions that the new retry hook is invoked during retry loops.
`t/unit/backends/test_database.py`	Adds tests for engine-option defaults/overrides, retry classification, and session invalidation.

Copilot · 2026-02-14T14:01:55Z

        engine_options=Option(
+            {
+                'pool_pre_ping': True,
+                'pool_recycle': 3600,
+            },
            type='dict', old={'celery_result_engine_options'},
        ),


database_engine_options default is now non-empty, but the configuration docs still state the default is {} (e.g. docs/userguide/configuration.rst under database_engine_options). Can we update the docs (and add a versionchanged:: note) so the documented default matches the new behavior?

I've fixed this in commit f1e5cb4

Copilot · 2026-02-14T14:01:56Z

+DEFAULT_DATABASE_ENGINE_OPTIONS = {
+    'pool_pre_ping': True,
+    'pool_recycle': 3600,
+}


DEFAULT_DATABASE_ENGINE_OPTIONS duplicates the default set in app/defaults.py for database_engine_options. Keeping two sources of truth increases the risk they drift; can we centralize this default (e.g., derive it from config defaults or reuse a shared constant) so it only needs to be updated in one place?

can you please cross check this?

I've removed the default value and extend and fix this in 25f4697

Co-authored-by: Copilot <[email protected]>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Copilot · 2026-02-15T16:21:20Z

                if self.always_retry and self.exception_safe_to_retry(exc):
                    if retries < self.max_retries:
                        retries += 1
+                        self.on_backend_retryable_error(exc)



Same as above: an exception raised by on_backend_retryable_error() here would replace the original backend error and prevent the intended retry. Making this hook best-effort (catch + log) would keep retry behavior reliable.

Fix in 4a4e6d7

Copilot · 2026-02-15T16:21:21Z

+        # Start with the default engine options, but allow explicit empty dicts
+        # from configuration or constructor arguments to disable all defaults.
+        engine_options_base = dict(DEFAULT_DATABASE_ENGINE_OPTIONS)
+
+        conf_engine_options = conf.database_engine_options
+        if conf_engine_options is not None:
+            if not conf_engine_options:
+                engine_options_base = {}
+            else:


The comment says an explicit empty mapping disables the defaults, but the current logic resets engine_options_base = {} when engine_options == {}, which also discards any non-default options coming from conf.database_engine_options. If the intent is “empty disables everything”, the comment should reflect that; if the intent is “empty disables only Celery’s defaults”, the merge logic likely needs adjusting.

I've updated the merge logic in 25f4697

Co-authored-by: Copilot <[email protected]>

codecov · 2026-02-15T16:32:41Z

Codecov Report

❌ Patch coverage is 71.42857% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.50%. Comparing base (67b328a) to head (3e4111b).
⚠️ Report is 104 commits behind head on main.

Files with missing lines	Patch %	Lines
celery/backends/database/__init__.py	69.56%	3 Missing and 4 partials ⚠️
celery/backends/base.py	71.42%	2 Missing ⚠️
celery/backends/database/session.py	80.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10122      +/-   ##
==========================================
- Coverage   87.53%   87.50%   -0.04%     
==========================================
  Files         153      153              
  Lines       19385    19417      +32     
  Branches     2229     2235       +6     
==========================================
+ Hits        16968    16990      +22     
- Misses       2121     2126       +5     
- Partials      296      301       +5

Flag	Coverage Δ
unittests	`87.48% <71.42%> (-0.04%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ChickenBenny · 2026-02-16T09:09:58Z

✅ All Copilot Review Comments Addressed

I've made some modifications to address all the Copilot review comments. Since I don't have write access to this branch, I've opened a new PR that references this one:

New PR: #10124

Summary of Fixes:

Issue	Status	Commit
📚 Documentation outdated	✅ Fixed	`f1e5cb4`
🔄 Duplicate `DEFAULT_DATABASE_ENGINE_OPTIONS`	✅ Fixed	`25f4697`
🛡️ Hook exception protection	✅ Fixed	`4a4e6d7`
🧪 Test coverage	✅ Added	`8ed0e6`

Changes Made:

Documentation - Updated docs/userguide/configuration.rst with correct defaults and .. versionchanged:: 5.6 directive
Centralized Configuration - Removed duplicate constant; engine options now merge from conf.database_engine_options only
Hook Protection - Added try-except around on_backend_retryable_error() to ensure hook failures don't interrupt retry loops
Test Coverage - Added comprehensive tests for retry hooks and failure scenarios (coverage improved from 71.4% to 92.3%)

Codecov Comparison:

This PR (Improve database backend retry handling for stale SQLAlchemy connections #10122): 71.42% patch coverage
New PR (Fix 10109 db backend connection health #10124): 92.30% patch coverage ✅

@auvipy @harshang03 Please review PR #10124 which contains all the fixes. Thanks to @harshang03 for the initial implementation!

auvipy · 2026-02-16T11:10:19Z

thanks. continuing with new pr

Harden database backend retries against stale SQLAlchemy connections.

bdb4480

Enable pool health defaults for the SQLAlchemy result backend and invalidate cached engines/sessions whenever retryable database errors occur so retries reconnect cleanly instead of reusing dead connections.

auvipy requested review from auvipy and Copilot February 14, 2026 13:57

Copilot started reviewing on behalf of auvipy February 14, 2026 13:57 View session

Copilot AI reviewed Feb 14, 2026

View reviewed changes

auvipy and others added 2 commits February 15, 2026 19:58

Merge branch 'main' into fix-10109-db-backend-connection-health

9edaf3a

Update celery/backends/database/__init__.py

e53b705

Co-authored-by: Copilot <[email protected]>

auvipy requested review from Copilot and removed request for auvipy February 15, 2026 16:16

Copilot started reviewing on behalf of auvipy February 15, 2026 16:17 View session

Copilot AI reviewed Feb 15, 2026

View reviewed changes

auvipy and others added 2 commits February 15, 2026 22:28

Update celery/backends/base.py

e10bd8f

Co-authored-by: Copilot <[email protected]>

Merge branch 'main' into fix-10109-db-backend-connection-health

3e4111b

auvipy mentioned this pull request Feb 15, 2026

Database Backend: Add missing index on date_done (Fixes #10097) #10098

Merged

ChickenBenny mentioned this pull request Feb 16, 2026

Fix 10109 db backend connection health #10124

Merged

auvipy closed this Feb 16, 2026

Uh oh!

Uh oh!

Conversation

harshang03 commented Feb 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Files changed

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

auvipy Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

ChickenBenny Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

auvipy Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

ChickenBenny Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

ChickenBenny Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

ChickenBenny Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov Bot commented Feb 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ChickenBenny commented Feb 16, 2026

✅ All Copilot Review Comments Addressed

Summary of Fixes:

Changes Made:

Codecov Comparison:

Uh oh!

auvipy commented Feb 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

harshang03 commented Feb 14, 2026 •

edited

Loading

ChickenBenny Feb 16, 2026 •

edited

Loading

codecov Bot commented Feb 15, 2026 •

edited

Loading