From 307793f43b91b3e8139ce3c4734bac77bd44c697 Mon Sep 17 00:00:00 2001 From: Kohtaroh Sakaue Date: Thu, 28 May 2026 23:31:25 +0900 Subject: [PATCH 1/3] =?UTF-8?q?perf(#124):=20=E3=82=AF=E3=82=A8=E3=83=AA?= =?UTF-8?q?=E6=9C=80=E9=81=A9=E5=8C=96=20(joinedload=20+=20=E8=AA=8D?= =?UTF-8?q?=E5=8F=AF=E7=B5=B1=E5=90=88=20+=20pool=20=E8=A8=AD=E5=AE=9A?= =?UTF-8?q?=E8=A6=8B=E7=9B=B4=E3=81=97)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #124 のレイテンシーをさらに削るため、1 リクエスト中の SQL ラウンドトリップ数を 減らす 3 点の最適化をまとめる。Singapore 移行で短縮した RTT (≈ 160ms × 3-5 クエリ) をクエリ数を 1-2 に絞ることでさらに削減し、ゴール p95 < 500ms を狙う。 - selectinload → joinedload 化 (cruds/trips, pages, blocks) - Trip → Pages → Blocks → Locations の 4 段連鎖クエリを 1 つの LEFT JOIN に集約 - 1-to-many の重複行除去のため呼び出し側で `result.unique()` を挟む - Block の location / destination_location は 1-to-1 なので重複は増えない - GET 系の認可と本体取得を 1 クエリに統合 (routers/blocks, pages) - 従来の require_block_access / require_page_access は 認可用 SELECT を 1 本発行 していたが、本体取得の joinedload に乗せて trip_id を取り出す形に変更 - GET /blocks/{block_id}: blocks_cruds.get_block_with_page で Block + Page + locations を 1 クエリ取得 → page.trip_id を Cookie で検証 - GET /pages/{page_id}: pages_cruds.get_page で Page + blocks + locations を 1 クエリ取得 → page.trip_id を Cookie で検証 - GET /pages/{page_id}/blocks: 上と同じ get_page を再利用し page.blocks を返す (旧 find_blocks 経由は廃止) - PUT / DELETE 系は副作用ありで境界が複雑なので require_*_access のまま据え置き - pool_pre_ping=False (db_connection.py) - チェックアウト毎の SELECT 1 (1 RTT) を省略 - 代わりに pool_recycle=600 を追加し、10 分以上経った接続は次回 checkout 時に 破棄して再確立。Neon Pooler 側の切断やネットワーク中断を握り潰さない保険 Co-Authored-By: Claude Opus 4.7 (1M context) --- .vscode/settings.json | 1 + server/app/cruds/blocks.py | 41 +++++++++++++++++++++++++++++------- server/app/cruds/pages.py | 20 ++++++++++-------- server/app/cruds/trips.py | 21 ++++++++++++------ server/app/db_connection.py | 13 ++++++++---- server/app/routers/blocks.py | 31 ++++++++++++++++++++------- server/app/routers/pages.py | 17 ++++++++++----- 7 files changed, 103 insertions(+), 41 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 859b9149..46ba6341 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -43,6 +43,7 @@ ], "cSpell.words": [ "embla", + "joinedload", "shadcn", "shinkansen" ], diff --git a/server/app/cruds/blocks.py b/server/app/cruds/blocks.py index ce0e35cd..a57ea3ad 100644 --- a/server/app/cruds/blocks.py +++ b/server/app/cruds/blocks.py @@ -1,6 +1,6 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession -from sqlalchemy.orm import selectinload +from sqlalchemy.orm import joinedload from app.cruds import locations as locations_cruds from app.models import Block @@ -9,9 +9,25 @@ def _block_with_relations(): + """Block を location / destination_location と一緒に 1 クエリで取る。 + + どちらも 1-to-1 なので joinedload で重複が増えない。 + """ return select(Block).options( - selectinload(Block.location), - selectinload(Block.destination_location), + joinedload(Block.location), + joinedload(Block.destination_location), + ) + + +def _block_with_page_and_relations(): + """Block + Page (trip_id 取得用) + location / destination_location を 1 クエリで取る。 + + 認可 (Page.trip_id が必要) と本体取得を 1 ラウンドトリップに統合するため。 + """ + return select(Block).options( + joinedload(Block.page), + joinedload(Block.location), + joinedload(Block.destination_location), ) @@ -62,8 +78,8 @@ async def create_block(db: AsyncSession, block: BlockCreate, page_id: int) -> Bl stmt = ( select(Block) .options( - selectinload(Block.location), - selectinload(Block.destination_location) + joinedload(Block.location), + joinedload(Block.destination_location), ) .where(Block.id == db_block.id) ) @@ -83,9 +99,7 @@ async def find_blocks(db: AsyncSession, page_id: int) -> list[Block]: Returns: list[Block]: ブロックリスト """ - result = await db.execute( - _block_with_relations().where(Block.page_id == page_id) - ) + result = await db.execute(_block_with_relations().where(Block.page_id == page_id)) return list(result.scalars().all()) @@ -104,6 +118,17 @@ async def get_block(db: AsyncSession, block_id: int) -> Block | None: return result.scalar_one_or_none() +async def get_block_with_page(db: AsyncSession, block_id: int) -> Block | None: + """ブロックを Page (trip_id 用) ごと 1 クエリで取得する。 + + ルーターで認可 + 本体取得を 1 ラウンドトリップで済ませるためのヘルパー。 + """ + result = await db.execute( + _block_with_page_and_relations().where(Block.id == block_id) + ) + return result.scalar_one_or_none() + + async def _replace_block_location( db: AsyncSession, db_block: Block, diff --git a/server/app/cruds/pages.py b/server/app/cruds/pages.py index 7bdbe11c..b9e24254 100644 --- a/server/app/cruds/pages.py +++ b/server/app/cruds/pages.py @@ -1,16 +1,20 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession -from sqlalchemy.orm import selectinload +from sqlalchemy.orm import joinedload from app.models import Block, Page from app.schemas.page import PageCreate, PageUpdate def _page_with_relations(): + """Page → Blocks → Locations を 1 クエリの LEFT JOIN で取得する。 + + 1-to-many の連鎖により重複行が出るので、呼び出し側で `result.unique()` を挟む。 + """ return select(Page).options( - selectinload(Page.blocks).options( - selectinload(Block.location), - selectinload(Block.destination_location), + joinedload(Page.blocks).options( + joinedload(Block.location), + joinedload(Block.destination_location), ) ) @@ -44,10 +48,8 @@ async def find_pages(db: AsyncSession, trip_id: int) -> list[Page]: Returns: list[Page]: ページリスト """ - result = await db.execute( - _page_with_relations().where(Page.trip_id == trip_id) - ) - return list(result.scalars().all()) + result = await db.execute(_page_with_relations().where(Page.trip_id == trip_id)) + return list(result.unique().scalars().all()) async def get_page(db: AsyncSession, page_id: int) -> Page | None: @@ -62,7 +64,7 @@ async def get_page(db: AsyncSession, page_id: int) -> Page | None: Page | None: 特定のページ、見つからない場合はNone """ result = await db.execute(_page_with_relations().where(Page.id == page_id)) - return result.scalar_one_or_none() + return result.unique().scalar_one_or_none() async def update_page(db: AsyncSession, page_id: int, page: PageUpdate) -> Page | None: diff --git a/server/app/cruds/trips.py b/server/app/cruds/trips.py index 2b8d0ecc..9aadf778 100644 --- a/server/app/cruds/trips.py +++ b/server/app/cruds/trips.py @@ -1,16 +1,23 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession -from sqlalchemy.orm import selectinload +from sqlalchemy.orm import joinedload from app.models import Block, Page, Trip from app.schemas.trip import TripCreateIn, TripUpdate def _trip_with_relations(): + """Trip → Pages → Blocks → Locations を 1 クエリの LEFT JOIN で取得する。 + + 1-to-many を joinedload で連鎖させるため重複行が発生する。呼び出し側で + `result.unique()` を挟むこと。クエリ数を 4 から 1 に減らす目的。 + """ return select(Trip).options( - selectinload(Trip.pages).selectinload(Page.blocks).options( - selectinload(Block.location), - selectinload(Block.destination_location), + joinedload(Trip.pages) + .joinedload(Page.blocks) + .options( + joinedload(Block.location), + joinedload(Block.destination_location), ) ) @@ -44,7 +51,7 @@ async def find_trips(db: AsyncSession) -> list[Trip]: list[Trip]: すべての旅行プラン """ result = await db.execute(_trip_with_relations()) - return list(result.scalars().all()) + return list(result.unique().scalars().all()) async def get_trip(db: AsyncSession, trip_id: int) -> Trip | None: @@ -59,7 +66,7 @@ async def get_trip(db: AsyncSession, trip_id: int) -> Trip | None: Trip | None: 特定の旅行プラン、見つからない場合はNone """ result = await db.execute(_trip_with_relations().where(Trip.id == trip_id)) - return result.scalar_one_or_none() + return result.unique().scalar_one_or_none() async def get_trip_by_url_id(db: AsyncSession, url_id: str) -> Trip | None: @@ -74,7 +81,7 @@ async def get_trip_by_url_id(db: AsyncSession, url_id: str) -> Trip | None: Trip | None: 特定の旅行プラン、見つからない場合はNone """ result = await db.execute(_trip_with_relations().where(Trip.url_id == url_id)) - return result.scalar_one_or_none() + return result.unique().scalar_one_or_none() async def update_trip(db: AsyncSession, trip_id: int, trip: TripUpdate) -> Trip | None: diff --git a/server/app/db_connection.py b/server/app/db_connection.py index 6170ee2c..ea2a9644 100644 --- a/server/app/db_connection.py +++ b/server/app/db_connection.py @@ -21,10 +21,15 @@ # 非同期エンジンの作成 engine: AsyncEngine = create_async_engine( settings.get_database_url(), - echo=False, # 本番環境では False - pool_pre_ping=True, # 接続の健全性チェック - pool_size=5, # 接続プールサイズ - max_overflow=10, # 最大オーバーフロー接続数 + echo=False, + # チェックアウト時の SELECT 1 を省略 (1 RTT 削減)。代わりに pool_recycle で + # 古い接続を破棄し、まれな切断は SQLAlchemy の自動 reconnect に任せる。 + pool_pre_ping=False, + pool_size=5, + max_overflow=10, + # 10 分以上経った接続は次回 checkout 時に破棄して再確立。Neon Pooler 側の + # 接続切断やネットワーク中断を握り潰さないための保険。 + pool_recycle=600, connect_args={"ssl": True} if settings.ssl_required else {}, ) diff --git a/server/app/routers/blocks.py b/server/app/routers/blocks.py index 34b9cc2d..64eac210 100644 --- a/server/app/routers/blocks.py +++ b/server/app/routers/blocks.py @@ -1,10 +1,15 @@ -from fastapi import APIRouter, Depends +from fastapi import APIRouter, Depends, Request from sqlalchemy.ext.asyncio import AsyncSession -from app.auth import require_block_access, require_page_access +from app.auth import ( + get_allowed_trip_ids, + require_block_access, + require_page_access, +) from app.cruds import blocks as blocks_cruds +from app.cruds import pages as pages_cruds from app.db_connection import get_db_session -from app.errors import NotFound +from app.errors import Forbidden, NotFound from app.schemas.block import Block, BlockCreate, BlockUpdate router = APIRouter(tags=["Blocks"]) @@ -39,15 +44,22 @@ async def create_block( ) async def get_blocks( page_id: int, - _: int = Depends(require_page_access), + request: Request, db: AsyncSession = Depends(get_db_session), ) -> list[Block]: """ 説明: - 特定のページに関連するすべてのブロックを取得する + - 認可と本体取得を 1 クエリで済ませるため、Page を blocks 込みで取得し + その page.trip_id を Cookie で検証する """ - return await blocks_cruds.find_blocks(db=db, page_id=page_id) + db_page = await pages_cruds.get_page(db=db, page_id=page_id) + if db_page is None: + raise NotFound(message="Page not found") + if db_page.trip_id not in get_allowed_trip_ids(request): + raise Forbidden() + return db_page.blocks @router.get( @@ -58,18 +70,21 @@ async def get_blocks( ) async def get_block( block_id: int, - _: int = Depends(require_block_access), + request: Request, db: AsyncSession = Depends(get_db_session), ) -> Block: """ 説明: - IDで指定された単一のブロックを取得する + - 認可と本体取得を 1 クエリで済ませるため、Block を Page (trip_id 取得用) と + locations 込みで取得し、その page.trip_id を Cookie で検証する """ - db_block = await blocks_cruds.get_block(db, block_id=block_id) + db_block = await blocks_cruds.get_block_with_page(db=db, block_id=block_id) if db_block is None: raise NotFound(message="Block not found") - + if db_block.page.trip_id not in get_allowed_trip_ids(request): + raise Forbidden() return db_block diff --git a/server/app/routers/pages.py b/server/app/routers/pages.py index cfe4ac28..c22abb74 100644 --- a/server/app/routers/pages.py +++ b/server/app/routers/pages.py @@ -1,10 +1,14 @@ -from fastapi import APIRouter, Depends +from fastapi import APIRouter, Depends, Request from sqlalchemy.ext.asyncio import AsyncSession -from app.auth import require_trip_access, require_page_access +from app.auth import ( + get_allowed_trip_ids, + require_page_access, + require_trip_access, +) from app.cruds import pages as pages_cruds from app.db_connection import get_db_session -from app.errors import NotFound +from app.errors import Forbidden, NotFound from app.schemas.page import Page, PageCreate, PageCreateResponse, PageUpdate # /trips/{trip_id}/pages で作成と一覧取得 @@ -59,18 +63,21 @@ async def get_pages( ) async def get_page( page_id: int, - _: int = Depends(require_page_access), + request: Request, db: AsyncSession = Depends(get_db_session), ) -> Page: """ 説明: - IDで指定された単一のページを取得する + - 認可と本体取得を 1 クエリで済ませるため、Page を blocks / locations 込みで + 取得し、その page.trip_id を Cookie で検証する """ db_page = await pages_cruds.get_page(db, page_id=page_id) if db_page is None: raise NotFound(message="Page not found") - + if db_page.trip_id not in get_allowed_trip_ids(request): + raise Forbidden() return db_page From 6313bdf83fd294755928477a475948564bfc54ca Mon Sep 17 00:00:00 2001 From: Kohtaroh Sakaue Date: Thu, 28 May 2026 23:39:35 +0900 Subject: [PATCH 2/3] =?UTF-8?q?fix(#124):=20joinedload=20=E5=8C=96?= =?UTF-8?q?=E3=81=AB=E4=BC=B4=E3=81=86=E7=B5=90=E6=9E=9C=E9=A0=86=E5=BA=8F?= =?UTF-8?q?=E3=81=AE=E4=B8=8D=E5=AE=89=E5=AE=9A=E3=81=95=E3=82=92=E8=A7=A3?= =?UTF-8?q?=E6=B6=88?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 選択句に明示的な ORDER BY が無い状態で `result.unique().scalars()` を回すと PostgreSQL の返却順がオプティマイザ依存になり、joinedload では LEFT JOIN の ハッシュ結果順序が selectinload 時代の id 昇順と一致しなくなった。 test_find_trips が trips[0] = 'trip 2' になって失敗していたのを修正する。 - models.py: Trip.pages と Page.blocks に order_by="..." を追加し、relationship 経由のアクセスでも子要素を id 昇順で揃える - cruds/trips.py: find_trips() の select に order_by(Trip.id) を追加 - cruds/pages.py: find_pages() の select に order_by(Page.id) を追加 - cruds/blocks.py: find_blocks() の select に order_by(Block.id) を追加 ローカル (Docker Postgres 16) で `pytest tests/` 97 件すべて PASS を確認。 Co-Authored-By: Claude Opus 4.7 (1M context) --- server/app/cruds/blocks.py | 4 +++- server/app/cruds/pages.py | 4 +++- server/app/cruds/trips.py | 2 +- server/app/models.py | 2 ++ 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/server/app/cruds/blocks.py b/server/app/cruds/blocks.py index a57ea3ad..bdcfc62b 100644 --- a/server/app/cruds/blocks.py +++ b/server/app/cruds/blocks.py @@ -99,7 +99,9 @@ async def find_blocks(db: AsyncSession, page_id: int) -> list[Block]: Returns: list[Block]: ブロックリスト """ - result = await db.execute(_block_with_relations().where(Block.page_id == page_id)) + result = await db.execute( + _block_with_relations().where(Block.page_id == page_id).order_by(Block.id) + ) return list(result.scalars().all()) diff --git a/server/app/cruds/pages.py b/server/app/cruds/pages.py index b9e24254..68fa7648 100644 --- a/server/app/cruds/pages.py +++ b/server/app/cruds/pages.py @@ -48,7 +48,9 @@ async def find_pages(db: AsyncSession, trip_id: int) -> list[Page]: Returns: list[Page]: ページリスト """ - result = await db.execute(_page_with_relations().where(Page.trip_id == trip_id)) + result = await db.execute( + _page_with_relations().where(Page.trip_id == trip_id).order_by(Page.id) + ) return list(result.unique().scalars().all()) diff --git a/server/app/cruds/trips.py b/server/app/cruds/trips.py index 9aadf778..422433a2 100644 --- a/server/app/cruds/trips.py +++ b/server/app/cruds/trips.py @@ -50,7 +50,7 @@ async def find_trips(db: AsyncSession) -> list[Trip]: Returns: list[Trip]: すべての旅行プラン """ - result = await db.execute(_trip_with_relations()) + result = await db.execute(_trip_with_relations().order_by(Trip.id)) return list(result.unique().scalars().all()) diff --git a/server/app/models.py b/server/app/models.py index 6637c468..01d2957a 100644 --- a/server/app/models.py +++ b/server/app/models.py @@ -87,6 +87,7 @@ class Trip(Base): cascade="all, delete-orphan", lazy="raise", passive_deletes=True, + order_by="Page.id", ) @@ -117,6 +118,7 @@ class Page(Base): cascade="all, delete-orphan", lazy="raise", passive_deletes=True, + order_by="Block.id", ) From 07e329d02f1e6b670ae6ba9ff7ffb836f8d1dbf8 Mon Sep 17 00:00:00 2001 From: Kohtaroh Sakaue Date: Fri, 29 May 2026 00:04:15 +0900 Subject: [PATCH 3/3] =?UTF-8?q?fix(#124):=20pool=5Fpre=5Fping=3DTrue=20?= =?UTF-8?q?=E3=81=AB=E6=88=BB=E3=81=97=E3=81=A6=20Neon=20=E3=82=B5?= =?UTF-8?q?=E3=82=B9=E3=83=9A=E3=83=B3=E3=82=B9=E5=BE=A9=E5=B8=B0=E6=99=82?= =?UTF-8?q?=E3=81=AE=20dead=20connection=20=E3=82=92=E9=98=B2=E3=81=90?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gemini code assist から指摘あり: Neon Free は 5 分アイドルで compute がサスペンス されるため、`pool_pre_ping=False` + `pool_recycle=600` ではプール内に残った dead connection を最初のリクエストが掴んで 500 になる可能性が高い。 - pool_pre_ping を True に戻して checkout 時の SELECT 1 で死活確認する - pool_recycle は 1800 (30 分) に延長。pool_pre_ping との二重防御として残す - 1 RTT のオーバーヘッドが戻るが、joinedload 化 (3 RTT 削減) と認可統合 (1 RTT 削減) の効果は維持される ローカル (Docker Postgres 16) で `pytest tests/` 97 件すべて PASS を確認。 Co-Authored-By: Claude Opus 4.7 (1M context) --- server/app/db_connection.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/server/app/db_connection.py b/server/app/db_connection.py index ea2a9644..2b6b9dd0 100644 --- a/server/app/db_connection.py +++ b/server/app/db_connection.py @@ -22,14 +22,17 @@ engine: AsyncEngine = create_async_engine( settings.get_database_url(), echo=False, - # チェックアウト時の SELECT 1 を省略 (1 RTT 削減)。代わりに pool_recycle で - # 古い接続を破棄し、まれな切断は SQLAlchemy の自動 reconnect に任せる。 - pool_pre_ping=False, + # Neon Free は 5 分アイドルで compute がサスペンドされ、復帰時に + # アプリ側のプール内コネクションが dead 化することがある。checkout 時の + # SELECT 1 (Pessimistic Disconnect Handling) で死活確認するのが安全。 + # 1 RTT 分のコストが発生するが、クエリ最適化で減らせる RTT 数より接続 + # 失敗による 500 のほうがコスト高なので有効のままにする。 + pool_pre_ping=True, pool_size=5, max_overflow=10, - # 10 分以上経った接続は次回 checkout 時に破棄して再確立。Neon Pooler 側の - # 接続切断やネットワーク中断を握り潰さないための保険。 - pool_recycle=600, + # 30 分超過した接続は次回 checkout 時にリサイクル。pool_pre_ping と + # 二重防御。短すぎると無駄な再確立が増え、長すぎると古い接続を掴むリスク。 + pool_recycle=1800, connect_args={"ssl": True} if settings.ssl_required else {}, )