mirror of
https://github.com/Hestia-Homes/Model.git
synced 2026-06-30 13:10:47 +00:00
Resolve overrides on the unit's own session, not a second connection
The modelling_e2e Lambda runs on a single-connection pool (pool_size=1, max_overflow=0) so one invocation uses one Postgres connection. But re-hydrating a Property through PostgresUnitOfWork resolved its Landlord Overrides through a PropertyOverridesPostgresReader built from the unit's session *factory* — which opens a brand-new Session per call. While the unit's own read transaction was still open (PropertyPostgresRepository.get_many had checked out the connection), that second Session asked the pool for a second connection, found none, and timed out after 30s: QueuePool limit of size 1 overflow 0 reached, connection timed out, timeout 30.00 The baseline stage (PropertyBaselineOrchestrator.run -> uow.property.get_many -> landlord overrides) hit this on every invocation. Read the overrides on the unit's OWN session instead. property_overrides is committed reference data, so reading it inside the unit's transaction sees the same rows and keeps the invocation on one connection. Extract the query/mapping into a shared helper and add OpenSessionPropertyOverridesReader (reads on a caller-owned, already-open session without closing it) for the unit; the standalone PropertyOverridesPostgresReader still opens its own short session for use outside a unit. Regression test pins the invariant with a real pool_size=1/max_overflow=0 engine: without the fix it reproduces the exact QueuePool timeout. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
17b9ae08eb
commit
de71f9abb6
3 changed files with 108 additions and 24 deletions
|
|
@ -15,7 +15,7 @@ from repositories.product.composite_product_repository import (
|
||||||
catalogue_with_off_catalogue_overrides,
|
catalogue_with_off_catalogue_overrides,
|
||||||
)
|
)
|
||||||
from repositories.property.property_overrides_postgres_reader import (
|
from repositories.property.property_overrides_postgres_reader import (
|
||||||
PropertyOverridesPostgresReader,
|
OpenSessionPropertyOverridesReader,
|
||||||
)
|
)
|
||||||
from repositories.property.property_postgres_repository import (
|
from repositories.property.property_postgres_repository import (
|
||||||
PropertyPostgresRepository,
|
PropertyPostgresRepository,
|
||||||
|
|
@ -46,10 +46,13 @@ class PostgresUnitOfWork(UnitOfWork):
|
||||||
spatial_repo = SpatialPostgresRepository(self._session)
|
spatial_repo = SpatialPostgresRepository(self._session)
|
||||||
# Fold Landlord Overrides onto the Effective EPC on every re-hydration
|
# Fold Landlord Overrides onto the Effective EPC on every re-hydration
|
||||||
# (ADR-0032), so what the Baseline orchestrator scores off ``uow.property``
|
# (ADR-0032), so what the Baseline orchestrator scores off ``uow.property``
|
||||||
# matches what the Plan was modelled from. The reader is uow-independent —
|
# matches what the Plan was modelled from. ``property_overrides`` is
|
||||||
# ``property_overrides`` is committed reference data — so it opens its own
|
# committed reference data, but the reader must read on THIS uow's session
|
||||||
# short read session per call via the same session factory.
|
# — a second session opened concurrently checks out a second connection
|
||||||
overrides_reader = PropertyOverridesPostgresReader(self._session_factory)
|
# and deadlocks the single-connection pool while this uow's transaction is
|
||||||
|
# open. Reading committed data inside the uow's transaction is correct and
|
||||||
|
# keeps the invocation on one connection.
|
||||||
|
overrides_reader = OpenSessionPropertyOverridesReader(self._session)
|
||||||
self.property = PropertyPostgresRepository(
|
self.property = PropertyPostgresRepository(
|
||||||
self._session, epc_repo, spatial_repo, overrides_reader
|
self._session, epc_repo, spatial_repo, overrides_reader
|
||||||
)
|
)
|
||||||
|
|
|
||||||
|
|
@ -2,9 +2,16 @@
|
||||||
|
|
||||||
Read-only and uow-independent: ``property_overrides`` is committed reference
|
Read-only and uow-independent: ``property_overrides`` is committed reference
|
||||||
data the ``bulk_upload_finaliser`` Lambda writes at Finalise, long before First
|
data the ``bulk_upload_finaliser`` Lambda writes at Finalise, long before First
|
||||||
Run executes — there is no transactional coupling to the ingestion run, so this
|
Run executes — there is no transactional coupling to the ingestion run, so the
|
||||||
opens its own short read session per call via the injected session factory
|
standalone reader opens its own short read session per call via the injected
|
||||||
(mirroring the composition root's ``lambda: Session(engine)``).
|
session factory (mirroring the composition root's ``lambda: Session(engine)``).
|
||||||
|
|
||||||
|
Inside a Unit of Work the overrides must instead be read on the UoW's *own*
|
||||||
|
session (``OpenSessionPropertyOverridesReader``): a second session opened
|
||||||
|
concurrently checks out a second connection, which deadlocks the modelling_e2e
|
||||||
|
Lambda's single-connection pool while the UoW's read transaction is still open.
|
||||||
|
Reading committed reference data inside the UoW's transaction is correct — it
|
||||||
|
sees the same committed rows — and keeps the invocation on one connection.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
@ -21,25 +28,43 @@ from repositories.property.property_overrides_reader import (
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _resolve_overrides(session: Session, property_id: int) -> ResolvedPropertyOverrides:
|
||||||
|
rows = session.exec(
|
||||||
|
select(PropertyOverrideRow).where(
|
||||||
|
col(PropertyOverrideRow.property_id) == property_id
|
||||||
|
)
|
||||||
|
).all()
|
||||||
|
return ResolvedPropertyOverrides(
|
||||||
|
rows=tuple(
|
||||||
|
ResolvedPropertyOverride(
|
||||||
|
override_component=row.override_component,
|
||||||
|
building_part=row.building_part,
|
||||||
|
override_value=row.override_value,
|
||||||
|
)
|
||||||
|
for row in rows
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class PropertyOverridesPostgresReader(PropertyOverridesReader):
|
class PropertyOverridesPostgresReader(PropertyOverridesReader):
|
||||||
|
"""Opens its own short read session per call — for standalone use outside a
|
||||||
|
Unit of Work, where there is no shared session/connection to reuse."""
|
||||||
|
|
||||||
def __init__(self, session_factory: Callable[[], Session]) -> None:
|
def __init__(self, session_factory: Callable[[], Session]) -> None:
|
||||||
self._session_factory = session_factory
|
self._session_factory = session_factory
|
||||||
|
|
||||||
def overrides_for(self, property_id: int) -> ResolvedPropertyOverrides:
|
def overrides_for(self, property_id: int) -> ResolvedPropertyOverrides:
|
||||||
with self._session_factory() as session:
|
with self._session_factory() as session:
|
||||||
rows = session.exec(
|
return _resolve_overrides(session, property_id)
|
||||||
select(PropertyOverrideRow).where(
|
|
||||||
col(PropertyOverrideRow.property_id) == property_id
|
|
||||||
)
|
|
||||||
).all()
|
|
||||||
|
|
||||||
return ResolvedPropertyOverrides(
|
|
||||||
rows=tuple(
|
class OpenSessionPropertyOverridesReader(PropertyOverridesReader):
|
||||||
ResolvedPropertyOverride(
|
"""Reads on a caller-owned, already-open session without closing it — for use
|
||||||
override_component=row.override_component,
|
inside a Unit of Work so resolving overrides reuses the UoW's single
|
||||||
building_part=row.building_part,
|
connection instead of checking out a second one."""
|
||||||
override_value=row.override_value,
|
|
||||||
)
|
def __init__(self, session: Session) -> None:
|
||||||
for row in rows
|
self._session = session
|
||||||
)
|
|
||||||
)
|
def overrides_for(self, property_id: int) -> ResolvedPropertyOverrides:
|
||||||
|
return _resolve_overrides(self._session, property_id)
|
||||||
|
|
|
||||||
|
|
@ -7,7 +7,7 @@ from typing import Any
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
from sqlalchemy import Engine
|
from sqlalchemy import Engine
|
||||||
from sqlmodel import Session
|
from sqlmodel import Session, create_engine
|
||||||
|
|
||||||
from datatypes.epc.domain.epc import Epc
|
from datatypes.epc.domain.epc import Epc
|
||||||
from datatypes.epc.domain.epc_property_data import (
|
from datatypes.epc.domain.epc_property_data import (
|
||||||
|
|
@ -152,6 +152,62 @@ def test_unit_hydrates_a_property_with_its_landlord_overrides_folded(
|
||||||
assert main.wall_insulation_type == 3
|
assert main.wall_insulation_type == 3
|
||||||
|
|
||||||
|
|
||||||
|
def test_hydrating_a_property_with_overrides_stays_on_one_connection(
|
||||||
|
db_engine: Engine,
|
||||||
|
) -> None:
|
||||||
|
"""Resolving Landlord Overrides during re-hydration must read on the unit's
|
||||||
|
OWN session, not open a second one. The modelling_e2e Lambda runs on a
|
||||||
|
single-connection pool (``pool_size=1, max_overflow=0``); a second session
|
||||||
|
opened while the unit's read transaction is still open checks out a second
|
||||||
|
connection and deadlocks ("QueuePool limit of size 1 overflow 0 reached,
|
||||||
|
connection timed out"). This pins that the unit holds exactly one connection
|
||||||
|
while it hydrates a Property that has overrides.
|
||||||
|
"""
|
||||||
|
# Arrange — seed a property + EPC + an override (committed reference data).
|
||||||
|
with PostgresUnitOfWork(_session_factory(db_engine)) as uow:
|
||||||
|
row = PropertyRow(portfolio_id=1, postcode="A0 0AA", address="1 St", uprn=1)
|
||||||
|
uow._session.add(row) # pyright: ignore[reportPrivateUsage]
|
||||||
|
uow._session.flush() # pyright: ignore[reportPrivateUsage]
|
||||||
|
property_id = row.id
|
||||||
|
assert property_id is not None
|
||||||
|
EpcPostgresRepository(uow._session).save( # pyright: ignore[reportPrivateUsage]
|
||||||
|
_epc(), property_id=property_id
|
||||||
|
)
|
||||||
|
uow._session.add( # pyright: ignore[reportPrivateUsage]
|
||||||
|
PropertyOverrideRow(
|
||||||
|
property_id=property_id,
|
||||||
|
portfolio_id=1,
|
||||||
|
building_part=0,
|
||||||
|
override_component="wall_type",
|
||||||
|
override_value="Solid brick, with internal insulation",
|
||||||
|
original_spreadsheet_description="solid brick, insulated",
|
||||||
|
)
|
||||||
|
)
|
||||||
|
uow.commit()
|
||||||
|
|
||||||
|
# A pool that admits exactly one connection and fails fast (not after 30s) if
|
||||||
|
# a second is requested — the production modelling_e2e shape.
|
||||||
|
single_connection = create_engine(
|
||||||
|
db_engine.url, pool_size=1, max_overflow=0, pool_timeout=2
|
||||||
|
)
|
||||||
|
try:
|
||||||
|
# Act — hydrate through the unit; this resolves the override.
|
||||||
|
with PostgresUnitOfWork(_session_factory(single_connection)) as uow:
|
||||||
|
prop = uow.property.get(property_id)
|
||||||
|
finally:
|
||||||
|
single_connection.dispose()
|
||||||
|
|
||||||
|
# Assert — reached here without a QueuePool timeout, and the override folded:
|
||||||
|
# cavity (4) → solid brick (3) / internal (3).
|
||||||
|
main = next(
|
||||||
|
part
|
||||||
|
for part in prop.effective_epc.sap_building_parts
|
||||||
|
if part.identifier is BuildingPartIdentifier.MAIN
|
||||||
|
)
|
||||||
|
assert main.wall_construction == 3
|
||||||
|
assert main.wall_insulation_type == 3
|
||||||
|
|
||||||
|
|
||||||
def test_leaving_the_block_without_commit_persists_nothing(db_engine: Engine) -> None:
|
def test_leaving_the_block_without_commit_persists_nothing(db_engine: Engine) -> None:
|
||||||
# Arrange
|
# Arrange
|
||||||
new_unit = lambda: PostgresUnitOfWork(_session_factory(db_engine))
|
new_unit = lambda: PostgresUnitOfWork(_session_factory(db_engine))
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue