経路検索のN+1クエリ解消・IPAキャッシュのArc化などクエリ処理のパフォーマンスを改善 - #1585
Conversation
- get_routes: 経路候補グループごとに路線を取得していたN+1クエリを 一括取得+line_group_cdパーティションに変更。発着駅を含まない 候補グループは路線取得・proto変換の前に除外する - get_routes: TrainTypeに埋め込むlinesを停車駅ごとに再フィルタ+ cloneせず、グループごとに一度だけ構築する - update_station_vec_with_attributes_inner: ループ末尾で get_station_numbersを同一入力で再計算していた冗長処理を削除 - update_station_vec_with_attributes_inner: 会社IDを一意化してから IN句クエリに渡すようにし、路線数分の重複バインドを解消 - get_routesの一括取得・候補除外・種別路線構築を検証するテストを追加 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
キャッシュヒットのたびにIpaResult(TTSセグメントのVecを含む)全体を ディープクローンしていたため、リスト系レスポンスでは駅・路線・種別 ごとに数個のStringアロケーションが発生していた。Arc<IpaResult>を 返す形にし、proto変換時に必要なフィールドだけをクローンする。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
create_table.sqlのDOブロックは拡張が使えない環境向けに例外を NOTICEで握り潰すため、インデックス作成に失敗しても起動ログからは 分からない。実際に稼働DBでtrigramインデックス5本だけが欠落し、 駅名検索が毎回全表スキャンになる事例があった。 必要な拡張はcreate_schema冒頭で必須として作成済みのため、 性能インデックスはスキーマ適用後に明示的に作成し直し、作成後に pg_indexesを検証して欠落があればERRORログで可視化する。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughIPA計算キャッシュがArc共有方式に変更され、DTO変換(line/station/train_type/tts)が参照・クローンベースに更新された。get_routesは経路グループの事前フィルタと路線の一括取得に再構成され、テストが追加された。スキーマ作成時にパフォーマンスインデックスの作成・検証処理が追加された。 ChangesIPAキャッシュ共有化・ルート最適化・インデックス検証
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant get_routes
participant RouteGroups
participant LineRepository
Client->>get_routes: from_station_id, to_station_id
get_routes->>RouteGroups: 発着駅を含むグループのみ抽出
get_routes->>get_routes: line_group_cdを一意化
get_routes->>LineRepository: get_by_line_group_id_vec_for_routesを一括呼び出し
LineRepository-->>get_routes: Line一覧
get_routes->>get_routes: line_group_cdでパーティション
get_routes-->>Client: 経路候補(TrainType/stops付き)
sequenceDiagram
participant Caller
participant compute_ipa_cached
participant cached_lookup
participant STATION_IPA_CACHE
Caller->>compute_ipa_cached: name_katakana, name_roman
compute_ipa_cached->>cached_lookup: 検索要求
cached_lookup->>STATION_IPA_CACHE: 読み取りロックでヒット確認
alt キャッシュヒット
STATION_IPA_CACHE-->>cached_lookup: Arc<IpaResult>クローン
else キャッシュミス
cached_lookup->>cached_lookup: compute()実行
cached_lookup->>STATION_IPA_CACHE: 書き込みロックで挿入
end
cached_lookup-->>Caller: Arc<IpaResult>
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
stationapi/src/import.rs (1)
41-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
PERFORMANCE_INDEXESをcreate_table.sqlと二重管理しないようにする
現状は同内容ですが、ここはcreate_table.sqlの同名DDLを手書きで複製しているため、片方だけ更新されると性能インデックスの定義がずれます。単一ソース化するか、少なくとも差分検出を入れておくと安全です。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stationapi/src/import.rs` around lines 41 - 88, The PERFORMANCE_INDEXES list in import.rs is duplicating the index DDL already defined in create_table.sql, so the two sources can drift. Refactor the index definitions so there is a single source of truth, or add a check in the import/create flow that compares the names and SQL for PERFORMANCE_INDEXES against the SQL file before applying them. Use the PERFORMANCE_INDEXES constant and the create_table.sql-driven table setup path as the main anchors when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@stationapi/src/import.rs`:
- Around line 41-88: The PERFORMANCE_INDEXES list in import.rs is duplicating
the index DDL already defined in create_table.sql, so the two sources can drift.
Refactor the index definitions so there is a single source of truth, or add a
check in the import/create flow that compares the names and SQL for
PERFORMANCE_INDEXES against the SQL file before applying them. Use the
PERFORMANCE_INDEXES constant and the create_table.sql-driven table setup path as
the main anchors when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b30b0298-4ed7-42f6-aa07-59a4b282f3b5
📒 Files selected for processing (7)
stationapi/src/domain/ipa.rsstationapi/src/import.rsstationapi/src/use_case/dto/line.rsstationapi/src/use_case/dto/station.rsstationapi/src/use_case/dto/train_type.rsstationapi/src/use_case/dto/tts.rsstationapi/src/use_case/interactor/query.rs
概要
クエリ処理のパフォーマンス改善。経路検索(
get_routes)のN+1クエリ解消、IPAキャッシュのヒット時ディープクローン排除、稼働DBで欠落していた性能インデックスへの恒久対策を行う。変更の種類
変更内容
get_routes: 経路候補グループごとに路線を取得していたN+1クエリを、全line_group_cdの一括取得+パーティションに変更。発着駅を含まない候補グループは路線取得・proto変換の前に除外get_routes:TrainTypeに埋め込む路線リストを停車駅ごとに再フィルタ+cloneせず、グループごとに一度だけ構築update_station_vec_with_attributes_inner: ループ末尾で同一入力のget_station_numbersを再計算していた冗長処理を削除し、会社IDを一意化してからIN句クエリへ渡すように変更domain/ipa.rs)をArc<IpaResult>返却に変更し、キャッシュヒットのたびにTTSセグメントを含む結果全体をディープクローンしていたのを解消create_schemaで明示的に作成し、作成後にpg_indexesを照合して欠落をERRORログで可視化(create_table.sqlのDOブロックが例外をNOTICEで握り潰すため、稼働DBでtrigramインデックス5本が欠落し駅名検索が全表スキャンになっていた事例への恒久対策)get_routesの一括取得・候補除外・種別路線構築を検証するテストを追加テスト
cargo fmt --all -- --checkが通ることcargo clippy -- -D warningsが通ることcargo test(SQLX_OFFLINE=true)が通ることEXPLAIN ANALYZEで駅名検索がSeq Scan(buffers 625)→Bitmap Index Scan(buffers 36)になることを稼働DBで確認済み。関連Issue
スクリーンショット(任意)
Summary by CodeRabbit
新機能
バグ修正