Skip to content

Commit 62cbb65

Browse files
authored
Merge pull request #50 from knifecake/fix/issue-48-keep-db-pooling
Keep DB pooling enabled and add pool-size validation
2 parents abf9e1b + 7039f66 commit 62cbb65

6 files changed

Lines changed: 106 additions & 77 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@
77
- Fixed supervisor child-process crash loops (`exit code 11`) seen with
88
Django/PostgreSQL pooling by resetting DB state before forking and clearing
99
Django's class-level psycopg pool cache in the forking path (#48).
10+
- Keep database pooling enabled while resetting fork-inherited connection state
11+
so child processes can establish fresh pools after forking.
12+
- Validate worker thread sizing against PostgreSQL `OPTIONS.pool.max_size`
13+
(when explicitly configured), mirroring Solid Queue's pool sizing check.
1014

1115
## v0.1.8 - 2026-03-08
1216

steady_queue/configuration.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33
from typing import Optional
44

55
from crontab import CronTab
6+
from django.conf import settings
67
from django.core.exceptions import ValidationError
78
from django.utils.module_loading import import_string
89

10+
from steady_queue.db_router import steady_queue_database_alias
911
from steady_queue.processes.base import Base
1012

1113

@@ -167,6 +169,7 @@ def schedulers(self) -> list["Configuration.Process"]:
167169
def is_valid(self):
168170
self.errors = []
169171
self.errors.extend(self.validate_configured_processes())
172+
self.errors.extend(self.validate_database_pool_size())
170173
self.errors.extend(self.validate_recurring_tasks())
171174

172175
return len(self.errors) == 0
@@ -177,6 +180,39 @@ def validate_configured_processes(self) -> list[ValidationError]:
177180

178181
return []
179182

183+
def validate_database_pool_size(self) -> list[ValidationError]:
184+
# Match Solid Queue behavior by validating worker thread count against
185+
# the queue DB connection pool size when a max_size is explicitly set.
186+
if len(self.options.workers) == 0:
187+
return []
188+
189+
db_alias = steady_queue_database_alias()
190+
db_config = settings.DATABASES.get(db_alias, {})
191+
192+
if db_config.get("ENGINE") != "django.db.backends.postgresql":
193+
return []
194+
195+
pool_options = db_config.get("OPTIONS", {}).get("pool")
196+
if not isinstance(pool_options, dict):
197+
return []
198+
199+
pool_max_size = pool_options.get("max_size")
200+
if not isinstance(pool_max_size, int):
201+
return []
202+
203+
if pool_max_size < self.estimated_number_of_threads:
204+
return [
205+
ValidationError(
206+
"Steady Queue is configured to use "
207+
f"{self.estimated_number_of_threads} threads but the "
208+
f"database connection pool max_size for '{db_alias}' is "
209+
f"{pool_max_size}. Increase "
210+
f"DATABASES['{db_alias}']['OPTIONS']['pool']['max_size']."
211+
)
212+
]
213+
214+
return []
215+
180216
def validate_recurring_tasks(self) -> list[ValidationError]:
181217
if self.skip_recurring:
182218
return []
@@ -195,6 +231,13 @@ def validate_recurring_tasks(self) -> list[ValidationError]:
195231

196232
return errors
197233

234+
@property
235+
def estimated_number_of_threads(self) -> int:
236+
# At most `threads` in each worker + 2 additional threads (worker loop
237+
# and heartbeat), mirroring Solid Queue's sizing heuristic.
238+
max_worker_threads = max((w.threads for w in self.options.workers), default=1)
239+
return max_worker_threads + 2
240+
198241
@property
199242
def skip_recurring(self) -> bool:
200243
return self.options.skip_recurring or self.options.only_work

steady_queue/processes/base.py

Lines changed: 2 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -49,45 +49,6 @@ def is_stopped(self) -> bool:
4949
def generate_name(self) -> str:
5050
return "-".join((self.kind, secrets.token_hex(10)))
5151

52-
def disable_connection_pooling(self):
53-
"""
54-
Disable connection pooling for steady_queue processes.
55-
56-
Connection pooling with psycopg doesn't work with forked processes.
57-
This method removes pool configuration from database settings and from
58-
already-instantiated Django connection wrappers.
59-
"""
60-
from django.conf import settings
61-
62-
if hasattr(settings, "DATABASES"):
63-
for alias, db_config in settings.DATABASES.items():
64-
if db_config.get("ENGINE") != "django.db.backends.postgresql":
65-
continue
66-
67-
options = db_config.setdefault("OPTIONS", {})
68-
if "pool" in options:
69-
logger.info(
70-
"%(name)s disabling connection pooling for database '%(alias)s'",
71-
{"name": self.name, "alias": alias},
72-
)
73-
del options["pool"]
74-
75-
for alias in connections:
76-
connection = connections[alias]
77-
if (
78-
connection.settings_dict.get("ENGINE")
79-
!= "django.db.backends.postgresql"
80-
):
81-
continue
82-
83-
options = connection.settings_dict.setdefault("OPTIONS", {})
84-
if "pool" in options:
85-
logger.debug(
86-
"%(name)s removing pool option from instantiated connection '%(alias)s'",
87-
{"name": self.name, "alias": alias},
88-
)
89-
del options["pool"]
90-
9152
def close_postgresql_connection_pools(self):
9253
"""
9354
Close and clear Django's class-level psycopg pool cache.
@@ -130,9 +91,8 @@ def reset_database_connections(self):
13091
"""
13192
Reset database connections for forked processes.
13293
133-
This disables connection pooling and resets connection state to prevent
134-
issues with shared connections between parent and child processes.
94+
This closes all current connections and clears Django's class-level
95+
psycopg pool cache so child processes don't inherit parent pool state.
13596
"""
136-
self.disable_connection_pooling()
13797
connections.close_all()
13898
self.close_postgresql_connection_pools()

tests/settings.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
DATABASE_ROUTERS = ["steady_queue.db_router.SteadyQueueRouter"]
4848

4949
if DATABASES["queue"]["ENGINE"] == "django.db.backends.postgresql":
50-
DATABASES["queue"]["OPTIONS"] = {"pool": {"min_size": 2, "max_size": 4}}
50+
DATABASES["queue"]["OPTIONS"] = {"pool": {"min_size": 2, "max_size": 8}}
5151
DATABASES["queue"]["TEST"] = {"NAME": "test_queue"}
5252
DATABASES["default"]["TEST"] = {"NAME": "test_default"}
5353

tests/test_configuration.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import warnings
12
from datetime import timedelta
23

34
from django.core.exceptions import ValidationError
@@ -153,6 +154,54 @@ def test_configuration_with_no_processes_fails_validation(self):
153154
self.assertGreater(len(config.errors), 0)
154155
self.assertIn("No processes configured", str(config.errors[0]))
155156

157+
def test_small_postgres_pool_fails_validation(self):
158+
"""Configured worker threads must fit in postgres pool max_size."""
159+
options = Configuration.Options(workers=[Configuration.Worker(threads=3)])
160+
config = Configuration(options)
161+
162+
with warnings.catch_warnings():
163+
warnings.filterwarnings(
164+
"ignore",
165+
message="Overriding setting DATABASES can lead to unexpected behavior.",
166+
category=UserWarning,
167+
)
168+
169+
with self.settings(
170+
DATABASES={
171+
"queue": {
172+
"ENGINE": "django.db.backends.postgresql",
173+
"OPTIONS": {"pool": {"min_size": 1, "max_size": 4}},
174+
}
175+
}
176+
):
177+
self.assertFalse(config.is_valid)
178+
179+
self.assertTrue(
180+
any("pool max_size" in error.message for error in config.errors)
181+
)
182+
183+
def test_sufficient_postgres_pool_passes_validation(self):
184+
"""Validation passes when postgres pool max_size is large enough."""
185+
options = Configuration.Options(workers=[Configuration.Worker(threads=3)])
186+
config = Configuration(options)
187+
188+
with warnings.catch_warnings():
189+
warnings.filterwarnings(
190+
"ignore",
191+
message="Overriding setting DATABASES can lead to unexpected behavior.",
192+
category=UserWarning,
193+
)
194+
195+
with self.settings(
196+
DATABASES={
197+
"queue": {
198+
"ENGINE": "django.db.backends.postgresql",
199+
"OPTIONS": {"pool": {"min_size": 1, "max_size": 5}},
200+
}
201+
}
202+
):
203+
self.assertTrue(config.is_valid)
204+
156205
def test_invalid_recurring_task_schedule_fails_validation(self):
157206
"""Invalid cron schedule in recurring task fails validation."""
158207
invalid_task = Configuration.RecurringTask(

tests/test_fork_safety.py

Lines changed: 7 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
1-
import warnings
21
from unittest.mock import patch
32

4-
from django.conf import settings
53
from django.test import SimpleTestCase
64

75
from steady_queue.configuration import Configuration
@@ -44,7 +42,9 @@ def test_supervisor_start_resets_connections_before_forking(self):
4442

4543

4644
class ResetDatabaseConnectionsTest(SimpleTestCase):
47-
def test_reset_connections_disables_and_clears_psycopg_pool_cache(self):
45+
def test_reset_connections_clears_psycopg_pool_cache_without_disabling_pooling(
46+
self,
47+
):
4848
class FakePool:
4949
def __init__(self):
5050
self.closed = False
@@ -85,38 +85,11 @@ def close_all(self):
8585
}
8686
)
8787

88-
with warnings.catch_warnings():
89-
warnings.filterwarnings(
90-
"ignore",
91-
message="Overriding setting DATABASES can lead to unexpected behavior.",
92-
category=UserWarning,
93-
)
94-
95-
with self.settings(
96-
DATABASES={
97-
"default": {
98-
"ENGINE": "django.db.backends.postgresql",
99-
"OPTIONS": {"pool": {"min_size": 1, "max_size": 4}},
100-
},
101-
"queue": {
102-
"ENGINE": "django.db.backends.postgresql",
103-
"OPTIONS": {"pool": {"min_size": 1, "max_size": 4}},
104-
},
105-
"sqlite": {
106-
"ENGINE": "django.db.backends.sqlite3",
107-
"OPTIONS": {"pool": {"min_size": 1, "max_size": 4}},
108-
},
109-
}
110-
):
111-
with patch("steady_queue.processes.base.connections", fake_connections):
112-
Base().reset_database_connections()
113-
114-
self.assertNotIn("pool", settings.DATABASES["default"]["OPTIONS"])
115-
self.assertNotIn("pool", settings.DATABASES["queue"]["OPTIONS"])
116-
self.assertIn("pool", settings.DATABASES["sqlite"]["OPTIONS"])
88+
with patch("steady_queue.processes.base.connections", fake_connections):
89+
Base().reset_database_connections()
11790

118-
self.assertNotIn("pool", fake_connections["default"].settings_dict["OPTIONS"])
119-
self.assertNotIn("pool", fake_connections["queue"].settings_dict["OPTIONS"])
91+
self.assertIn("pool", fake_connections["default"].settings_dict["OPTIONS"])
92+
self.assertIn("pool", fake_connections["queue"].settings_dict["OPTIONS"])
12093
self.assertIn("pool", fake_connections["sqlite"].settings_dict["OPTIONS"])
12194

12295
self.assertTrue(fake_connections.close_all_called)

0 commit comments

Comments
 (0)